configure.in and build scripts for Symbian OS

RESOLVED WONTFIX

Status

Firefox Build System
General
RESOLVED WONTFIX
9 years ago
2 months ago

People

(Reporter: Harry Li, Assigned: Harry Li)

Tracking

({mobile})

Trunk
ARM
Symbian
mobile

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 4 obsolete attachments)

(Assignee)

Description

9 years ago
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3) Gecko/20090305 Firefox/3.1b3 GTB5
Build Identifier: 

At present, the method for obtaining $OS_TARGET on Symbian platform is still not confirmed by related peers. Discussion happened at the end of bug 442706, but that is a resolved bug now. We could continue the discussion and patching in this new bug.

Reproducible: Always
(Assignee)

Updated

9 years ago
Blocks: 432606
Keywords: mobile
OS: Other → Symbian
Hardware: Other → ARM

Updated

9 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 1

9 years ago
For what it's worth, simply omitting the hunk that adds the --enable-symbian-target option from the patch in bug 479585, attachment 368662 [details] [diff] [review] lets you set OS_TARGET to whatever you like: doing 'configure --target=CPU-COMPANY-symbianSYSTEM' sets OS_TARGET to 'symbianSYSTEM'.  So I'm not sure what the option allows you to do that you can't do already.
(Assignee)

Comment 2

9 years ago
Now I am working on patching three configure.in files (located in "/","/nsprpub","/js/src") with only --target parameter to set $OS_TARGET.
And I changed the summary of this bug to make it not only discussing $OS_TARGET but also addressing build system issues.
Summary: Method for obtaining OS_TARGET on Symbian platform → configure.in and build scripts for Symbian OS
(Assignee)

Comment 3

9 years ago
Created attachment 371111 [details] [diff] [review]
xulrunner-symbian.apr05

--enable-symbian-target is no longer needed. Before building, $CROSS_COMPILE environment variable should be set to 1. Then build with arguments :

../hg/src/configure --target=arm-none-symbianelf --disable-debug --with-symbian-sdk=/c/Symbian/9.2/S60_3rd_FP1 --enable-application=xulrunner --enable-default-toolkit=cairo-symbian --disable-javaxpcom --disable-jit

NSPRPUB/ and JS/SRC/ building have been tested.
(Assignee)

Comment 4

9 years ago
Created attachment 371112 [details] [diff] [review]
nsprpub-symbian.apr05
(Assignee)

Comment 5

9 years ago
Created attachment 371113 [details] [diff] [review]
js-symbian.apr05
Attachment #371113 - Flags: review?(jim)
(Assignee)

Comment 6

9 years ago
Created attachment 371444 [details] [diff] [review]
xulrunner-symbian-build.apr07
Attachment #371111 - Attachment is obsolete: true
(Assignee)

Comment 7

9 years ago
Created attachment 371446 [details] [diff] [review]
js-symbian-build.apr07
Attachment #371113 - Attachment is obsolete: true
Attachment #371446 - Flags: review?(jim)
Attachment #371113 - Flags: review?(jim)
(Assignee)

Comment 8

9 years ago
with these three patches, now we could build NSPR, JS, and parse/expat successfully.
Make sure there is the environment variable $CROSS_COMPILE=1
invoking configure.in in the obj dir: 
../hg/src/configure --target=arm-none-symbianelf --disable-debug
--with-symbian-sdk=/c/Symbian/9.2/S60_3rd_FP1 --enable-application=xulrunner
--enable-default-toolkit=cairo-symbian --disable-javaxpcom --disable-jit

Updated

9 years ago
Attachment #371446 - Flags: review?(jim) → review+

Comment 9

9 years ago
Harry, do you have permission to land patches on the trunk yourself, or do you need me to land this one for you?
(Assignee)

Comment 10

9 years ago
(In reply to comment #9)
> Harry, do you have permission to land patches on the trunk yourself, or do you
> need me to land this one for you?

I don't have permission to land patches, so I'd appreciated you do this for me. Thanks.
(Assignee)

Updated

9 years ago
Attachment #371444 - Flags: review?
(Assignee)

Updated

9 years ago
Attachment #371112 - Flags: review?

Comment 11

9 years ago
Harry, I think you should set nelson for review for the nspr patch and ted for the xulrunner one.
(Assignee)

Updated

9 years ago
Attachment #371444 - Flags: review? → review?(ted.mielczarek)
(Assignee)

Updated

9 years ago
Attachment #371112 - Flags: review? → review?(nelson)

Comment 12

9 years ago
Comment on attachment 371446 [details] [diff] [review]
js-symbian-build.apr07

Actually, I screwed up here:

 ifneq (,$(CROSS_COMPILE)$(filter-out WINNT OS2,$(OS_ARCH)))
-ifneq ($(OS_ARCH), WINCE)
+ifeq (,$(filter-out SYMBIAN WINCE,$(OS_ARCH)))
 # nsinstall doesn't get built until we enter config/ in the exports phase,
 # so we'll have to manually ensure it gets built here if we want to use
 # $(EXPORTS)

This can't be right: the new condition will run the enclosed code only if OS_ARCH *is* SYMBIAN or WINCE, whereas I think the intention of the patch was to exclude it on SYMBIAN as well as WINCE --- ifneq.

I would have gone ahead and fixed this and landed it... but how can this patch be working for you as written?
Attachment #371446 - Flags: review+ → review-
Comment on attachment 371112 [details] [diff] [review]
nsprpub-symbian.apr05

Harry, please answer these questions about your patch.

It appears that you are removing support for one of the two Symbian 
"flavors", namely WINSCW.  From contextual comments, I gather that this
flavor is for developing code to run on a Symbian simulator on Windows.

Q1: Why do you want to remove that?
Q2: Is there agreement among the Symbian developers that that should be 
removed?

It appears that, in the case of a symbian OS, the makefile relies on two 
different but similarly named make variables: OS_TARGET and target_os.
Please explain the difference between them.
(Assignee)

Comment 14

9 years ago
Sorry for the inactivity of recent weeks cause I was in a work city relocation.

A1: Because I think supporting for emulator flavor will take us some time, and even with a working emulator build, the speed of test cases execution is too slow to accept. So this become impractical in my opinion. While the build scripts were changing and I haven't tested these changes on the emulator to ensure they are working, so I removed support for this branch.
A2: I haven't had agreement with other Symbian developers. However, Christian suggested leaving choice for other people to complete this in future. You could see bug 483816 was created with this purpose.

I should state that I am a very beginner on GNU autoconf and make. In the beginning of porting, I used --enable-symbian-target=GCCE/WINSCW to specify $OS_TARGET , and in latter parts of configure script and Makefile I detect build flavor by this variable. When these build scripts patches got reviewed, Mozilla peers suggests taking build flavors from --target argument, then we should set --target=arm-none-symbianelf for device build and --target=arm-none-symbianwinscw for emulator build. So I could only get build flavor from $target_os variable in configure script, then this is used to set $OS_TARGET in autoconf.mk to help Makefile to know which flavor should be built.

Because my work city relocation is still not needed, maybe I couldn't do code related works until 1st week in May, sorry for the inconvenience.
(Assignee)

Comment 15

9 years ago
(In reply to comment #12)
> (From update of attachment 371446 [details] [diff] [review])
> Actually, I screwed up here:
> 
>  ifneq (,$(CROSS_COMPILE)$(filter-out WINNT OS2,$(OS_ARCH)))
> -ifneq ($(OS_ARCH), WINCE)
> +ifeq (,$(filter-out SYMBIAN WINCE,$(OS_ARCH)))
>  # nsinstall doesn't get built until we enter config/ in the exports phase,
>  # so we'll have to manually ensure it gets built here if we want to use
>  # $(EXPORTS)
> 
> This can't be right: the new condition will run the enclosed code only if
> OS_ARCH *is* SYMBIAN or WINCE, whereas I think the intention of the patch was
> to exclude it on SYMBIAN as well as WINCE --- ifneq.
> 
> I would have gone ahead and fixed this and landed it... but how can this patch
> be working for you as written?

Hi Jim,
Sorry for the late response.

After re-check the patch and run the build scripts again, I think this line is correct. To my understanding, "$(filter-out SYMBIAN WINCE,$(OS_ARCH)" will result "SYMBIAN" if $(OS_ARCH) is "SYMBIAN", "WINCE" if $(OS_ARCH) is "WINCE" or
"" if $(OS_ARCH) is "MAC". Then we take this expression into "$ifeq (,$(filter-out SYMBIAN WINCE,$(OS_ARCH))". Cause the first argument is "", a null, so if $(OS_ARCH) is SYMBIAN or WINCE, the null and the second non-null express will result a false, then it will be lead to not compiling nsinstall.

Comment 16

9 years ago
(In reply to comment #15)
> To my understanding, "$(filter-out SYMBIAN WINCE,$(OS_ARCH)" will
> result "SYMBIAN" if $(OS_ARCH) is "SYMBIAN", "WINCE" if $(OS_ARCH) is "WINCE"
> or
> "" if $(OS_ARCH) is "MAC".

Actually, filter-out does the opposite: it removes all occurrences of SYMBIAN and WINCE from the list of words in OS_ARCH.  Since OS_ARCH is always a single word, the whole filter-out call expands to the empty string when OS_ARCH is SYMBIAN or WINCE, or something non-empty otherwise.  I believe the function you're looking for is 'filter'.

For debugging makefiles, you can add calls to make's $(info) function to the 'ifeq' body to see whether it's being triggered or not.  I also find it very helpful to run 'make -qp' and search through the output, which shows the rules, variable values, and source locations that set them.  See the descriptions of 'filter' and 'filter-out' here:
http://www.gnu.org/software/make/manual/html_node/Text-Functions.html#Text-Functions

I'm at a loss to explain why your build works, but I'm pretty sure that 'make' conditional is not correct.

Comment 17

9 years ago
(In reply to comment #14)
> I should state that I am a very beginner on GNU autoconf and make.

If you join the #jsapi IRC channel on irc.mozilla.org, I'd be happy to answer your beginner questions on autoconf and make.
(Assignee)

Comment 18

9 years ago
Created attachment 376366 [details] [diff] [review]
js_src-symbian-build.may08

After double recheck, I found the previous configure gave "OS_ARCH=symbianelf" in autoconf.mk file, that's the reason why I got a valid build. Now I move this line (and OS_RELEASE, CPU_ARCH) to the place where configure will actually put it into "autoconf.mk". With this patch I could build js successfully.
Attachment #371446 - Attachment is obsolete: true
Attachment #376366 - Flags: review?(jim)
(Assignee)

Comment 19

9 years ago
Created attachment 376367 [details] [diff] [review]
xulrunner-symbian-build.may08

This patch contains the same fix as stated in comment #18 .
Attachment #371444 - Attachment is obsolete: true
Attachment #376367 - Flags: review?(ted.mielczarek)
Attachment #371444 - Flags: review?(ted.mielczarek)

Updated

9 years ago
Attachment #376366 - Flags: review?(jim) → review+

Comment 20

9 years ago
(In reply to comment #18)
> After double recheck, I found the previous configure gave "OS_ARCH=symbianelf"
> in autoconf.mk file, that's the reason why I got a valid build. Now I move this
> line (and OS_RELEASE, CPU_ARCH) to the place where configure will actually put
> it into "autoconf.mk". With this patch I could build js successfully.

Thanks for sorting this out.  Should I land this now?
(Assignee)

Comment 21

9 years ago
(In reply to comment #20)
> (In reply to comment #18)
> > After double recheck, I found the previous configure gave "OS_ARCH=symbianelf"
> > in autoconf.mk file, that's the reason why I got a valid build. Now I move this
> > line (and OS_RELEASE, CPU_ARCH) to the place where configure will actually put
> > it into "autoconf.mk". With this patch I could build js successfully.
> 
> Thanks for sorting this out.  Should I land this now?

This is ready for landing.
Thanks :)

Comment 22

9 years ago
js_src-symbian-build.may08 passes the try server; will push tomorrow morning, when I'm able to stay and watch the tree for consequences.

Comment 23

9 years ago
Got distracted Friday; plan to land today.
Comment on attachment 376367 [details] [diff] [review]
xulrunner-symbian-build.may08

+    HOST_CC=gcc
+    HOST_CXX=g++

You shouldn't need to set these, configure should automatically look for gcc for the host compiler:
http://mxr.mozilla.org/mozilla-central/source/configure.in#297

+    HOST_CFLAGS=-D_X86_
+    HOST_CXXFLAGS=-D_X86_

Why is this necessary?

+    CFLAGS="-MD -nostdinc"

You should always do: CFLAGS="$CFLAGS ..." in configure, so the user can pass in their own CFLAGS on the commandline.

+        CFLAGS="$CFLAGS -c -Wall -Wno-unknown-pragmas -fexceptions -march=armv5t -mapcs -pipe -msoft-float"

I don't think you want -c in there.

Also, -Wall should get set in this block here, unless the symbian compiler doesn't trip the GNU_CC variable:
http://mxr.mozilla.org/mozilla-central/source/configure.in#1450

Any reason you set CFLAGS/CXXFLAGS in two steps, instead of just all at once?

+    *-symbianwinscw)
+        dnl TODO: add emulator build code
+        OS_TARGET=WINSCW


If this isn't going to work yet, could you put in something like AC_MSG_ERROR([Symbian emulator build is not yet supported]) ?

+    AC_DEFINE(NDEBUG)

I don't think you should be defining this unilaterally, we set this when you --disable-debug:
http://mxr.mozilla.org/mozilla-central/source/configure.in#6423

+    AC_DEFINE(HAVE_FCNTL_FILE_LOCKING)
+    AC_DEFINE(HAVE_SOCKLEN_T)

It looks like these defines are only used in NSPR, so you shouldn't need them here.

+cairo-symbian)
+    dnl TODO: add section for cairo port of symbian
+    ;;
+    

Do you have a cairo port yet? I don't think I'm happy leaving this as a TODO in configure.

 if test "$result" = "no"; then
-    AC_MSG_ERROR([Your compiler does not follow the C++ specification for temporary object destruction order.])
+    AC_MSG_WARN([Your compiler does not follow the C++ specification for temporary object destruction order.])
 fi

I don't think this is going to work. That check is there for a reason, the comment references bug 235381 which talks about crashes on Solaris.

r-, need some answers to the questions I've posed here.
Attachment #376367 - Flags: review?(ted.mielczarek) → review-
Assignee: nobody → harry.li
Status: NEW → ASSIGNED
Component: General → Build Config
Product: Fennec → Core
QA Contact: general → build-config

Comment 26

9 years ago
(In reply to comment #25)

Hmm, some of Ted's comments here apply also to the patch for js/src.  Could you put together a follow-up patch for that, too?  In particular:

        CFLAGS="-MD -nostdinc"

You should add to CFLAGS, not replace its value,

        CFLAGS="$CFLAGS -c -Wall -Wno-unknown-pragmas -fexceptions -march=armv5t -mapcs -pipe -msoft-float"

This shouldn't include -c.

            CXXFLAGS="$CFLAGS -Wno-ctor-dtor-privacy"

This should be extending CXXFLAGS, not CFLAGS.  If you need to repeat those flags, perhaps you could put them in a variable first, and then add them to CFLAGS and CXXFLAGS.

            CFLAGS="$CFLAGS ${GCCE_INCLUDE} -x c"
            CXXFLAGS="$CXXFLAGS ${GCCE_INCLUDE} -x c++"

Why do you need to specify '-x c' and '-x c++' here?  Are you compiling files that have non-standard filename extensions?  If so, could we get the -x flags for those targets someplace more specific?

And --- my apologies.  I took the attitude that whatever was in a Symbian conditional was the Symbian developers' concern, and didn't look too critically, but that's bogus.  Anyone trying to work on those scripts or makefiles will be trying to avoid breaking other targets, and odd stuff will cause them a lot of confusion and headache.  For example, we don't want people wondering, "Gee, is CFLAGS something I should be able to set or not?  It's usually preserved, but then in some cases (say, this Symbian code here) the inherited value is just wiped out..."  Thanks to Ted for keeping standards high.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → WONTFIX

Updated

2 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.