Closed Bug 442706 Opened 16 years ago Closed 15 years ago

Build NSPR for Symbian targets with autoconf and (gnu)make

Categories

(NSPR :: NSPR, defect)

ARM
Symbian
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: me, Assigned: me)

References

Details

(Keywords: mobile)

Attachments

(3 files, 14 obsolete files)

7.04 KB, patch
Details | Diff | Splinter Review
1.09 KB, patch
christian.bugzilla
: review+
Details | Diff | Splinter Review
945 bytes, patch
ted
: review+
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008052912 Firefox/3.0
Build Identifier: 

In this bug we want to find a way for building binaries which run on Symbian target OS. Symbian application and library binaries are usually built by Symbian proprietary make tools "makmake". But projects from Mozilla all use GNU make and autoconf to build all targets for all platforms. So we have to patch the configure.in and related makefiles to get them working with Symbian compilers including for emulator(x86, windows executable) and for device (ARM, cross compile) as well as linker.


Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Blocks: 432430
Keywords: mobile
This is the first static Makefile which could build NSPR(libnspr4.lib) for Symbian emulator in MSYS environment. In this patch we could utilize MSYS and Gnu Make to build target binary. The patch requires installation of Nokia Carbide.C++ Express (free of charge) IDE tool which contains the command line compiler and linker. Also Nokia S60 SDK 3rd Edition installation is also required.

The purpose of this patch submission is for your roughly review and determine if we should continue with building Mozilla platform with static Makefile(the symbian.mk) instead of the merge them into whole configure.in/Makefile.in tool chain. The latter choice might take more time to finish, and Symbian target executable always requires a dual cross compiling processes: one is for emulator(host os:win, host arch:x86, target os:win, target arch:x86) and the other is for device(host os:win, host arch:x86, target os:symbian, target arch:arm). So I hope this patch will let us making our evaluation about in which way we should build our Mozilla targets for Symbian.
Attachment #327614 - Flags: review?(wtc)
Comment on attachment 327614 [details] [diff] [review]
static makefile for NSPR on Symbian, emulator build only

a new patch for configure.in and related files will be submitted soon, and this will not be used for production anymore.
Attachment #327614 - Attachment is obsolete: true
Attachment #327614 - Flags: review?(wtc)
After patching the configure.in, the original build system could build the nsprpub(nspr,plc,plds) for emulator target. Test cases are not built or performed. There are something to note:

1, Prerequisite tools are :
  a) Nokia "S60 SDK 3rd Edition MR" or later SDK.
  b) Nokia Carbide.c++ IDE, although we won't use the IDE, still we need the compilers in this product. Express edition is free for download.
  c) MSYS or Mozilla-Build.

2, configure in the patch is generated from configure.in by autoconf, but I observed a tiny difference of the original configure production process. Then I used a tiny shell script to prevent this tiny difference from running autoconf:

#!/usr/bin/bash
/opt/autoconf2.13/bin/autoconf
sed -e "s/ '\\\012'/ '\\\012' ' ' | tr '\\\015'/" configure >> configure.new
rm ./configure
mv ./configure.new ./configure

3, Although Symbian/S60 SDKs after v7.0 hide the EPOCROOT environment variable from user space, here we use this to detect SDK installation path. If you install your Symbian SDK at D:\Symbian\9.1\S60_3rd_MR, then you should get something like this in MSYS:
EPOCROOT=/d/Symbian/9.1/S60_3rd_MR/
(There are both heading and trailing "/", this is the EPOCROOT E.V. tradition)

4, Because Carbide.c++ IDE sets some environment variables inside for its compilers, so we have to copy them out, they will look like these in your MSYS:

export MWCSYM2INCLUDES="D:\\Program Files\\Nokia\\Carbide.c++ v1.3\\x86Build\\Symbian_Support\\MSL\\MSL_C\\MSL_Common\\Include;D:\\Program Files\\Nokia\\Carbide.c++ v1.3\\x86Build\\Symbian_Support\\MSL\\MSL_C\\MSL_Win32\\Include;D:\\Program Files\\Nokia\\Carbide.c++ v1.3\\x86Build\\Symbian_Support\\MSL\\MSL_C\\MSL_X86;D:\\Program Files\\Nokia\\Carbide.c++ v1.3\\x86Build\\Symbian_Support\\MSL\\MSL_C++\\MSL_Common\\Include;+D:\\Program Files\\Nokia\\Carbide.c++ v1.3\\x86Build\\Symbian_Support\\MSL\\MSL_Extras\\MSL_Common\\Include;D:\\Program Files\\Nokia\\Carbide.c++ v1.3\\x86Build\\Symbian_Support\\MSL\\MSL_Extras\\MSL_Win32\\Include;D:\\Program Files\\Nokia\\Carbide.c++ v1.3\\x86Build\\Symbian_Support\\Win32-x86 Support\\Headers\\Win32 SDK;"
export MWSYM2LIBRARIES="D:\\Program Files\\Nokia\\Carbide.c++ v1.3\\x86Build\\Symbian_Support\\Win32-x86 Support\\Libraries\\Win32 SDK;D:\\Program Files\\Nokia\\Carbide.c++ v1.3\\x86Build\\Symbian_Support\\Runtime\\Runtime_x86\\Runtime_Win32\\Libs"
export MWSYM2LIBRARYFILES="MSL_All_MSE_Symbian_D.lib;gdi32.lib;user32.lib;kernel32.lib;"

5, invoke the configure like this:

../mozilla-central/nsprpub/configure --target=arm-none-symbianelf --enable-symbian-target=WINSCW

6, Device target rule are not written at this time. It will be ready soon, and please make some comments before the device target rule patching go too far with some error.
Attachment #328247 - Flags: review?(wtc)
Comment on attachment 328247 [details] [diff] [review]
patch for configure.in, emalator builds OK, tests are not made and run

new patch for emulator builds with test cases built and run will be submitted soon.
Attachment #328247 - Attachment is obsolete: true
Attachment #328247 - Flags: review?(wtc)
Please read comment #3 before this,
here are the updates:

1, Addition comment for point 1 in Comment#3: We use the "nsinstall.exe" binary executable in Mozilla-Build instead of building it with Windows SDK ourself.

2, There are two prerequisite environment parameters:
SYMBIAN_SDK_PATH=/d/Symbian/9.1/S60_3rd_MR/
ACTIVE_PERL=/d/perl/bin/perl.exe
We use the first one instead of EPOCROOT EV because some of the Symbian build perl scripts will check if EPOCROOT EV satisfy its requirements: begins and ends with "\", which are not good for our MSYS and compiler argument. And ACTIVE_PERL EV is set for replacing the perl which comes with Mozilla-Build suite because the original will be incompatible.

3, Only some test cases are run, because emulator speed is very slow and some features are different from device target. The test cases run are only used to verify our build result rather than NSPR functionality.

4, Make sure in your PATH EV, "c/Program Files/Common Files/Symbian/Tool" is BEHIND "/d/Symbian/9.1/S60_3rd_MR/Epoc32/tools" if you install your S60 SDK at "d:\Symbian\9.1\S60_3rd_MR\". If not, some perl scripts during build process will fail.
Attachment #329221 - Flags: review?(wtc)
Comment on attachment 329221 [details] [diff] [review]
patch for build system, emalator only, tests are built and partly tested

A new patch for both emulator(WINSCW) and device target(GCCE) builds done successfully will be submitted soon. New patch will be tested on device.
Attachment #329221 - Attachment is obsolete: true
Attachment #329221 - Flags: review?(wtc)
This patch is made against CVS HEAD, NSPR 4.7.2_BETA. Both two Symbian targets (WINSCW,GCCE) could be built successfully, and all test cases are performed on a target phone. One additional test cases caller--run_nspr_test.cpp/run_nspr_test_reg.rsc is added, it could be built with other test cases together.

Prerequisites are: 
1-MSYS on a Windows OS
2-MozillaBuild pacakge(nsinstall.exe)
3-Nokia S60 SDK
4-Nokia Carbide.c++ Express(only emulator compiler is used)
5-ActivePerl v5.6.1.638 (used as Symbian resource file builder, this is also a prerequisite of S60 SDK, newer ones may not work)

When build for emulator, use
configure --target=arm-none-symbianelf --enable-symbian-target=WINSCW
When build for device, use
configure --target=arm-none-symbianelf --enable-symbian-target=GCCE --disable-debug
the last switch "disable-debug" is required.

The test cases packaging file is not included due to directory layout still need to be decided. But this won't bring any inconvenient to this patch.
Attachment #331733 - Flags: review?(wtc)
Attached file installation package source file (obsolete) —
Symbian OS uses its own Software Installation System(SIS) for third party application install. A .pkg is used to define source binaries and target path, along with other information. This file should be located one level upper to GCCE build path, assuming the build path named "gobj". Also the result installation package has to be signed (even self-sign) before transferred into a phone for testing. Detail about Symbian OS installation package can be found at : http://www.symbian.com/developer/techlib/v9.1docs/doc_source/N10356/Installing/index.html#Installing.Toplevel.toc
Hi Harry,

+	    CC=arm-none-symbianelf-g++.exe
+        CXX=arm-none-symbianelf-g++.exe
+	    LD=arm-none-symbianelf-ld.exe
+        AR=arm-none-symbianelf-ar.exe

So you should not define this due to GCC.  It will be defined with ./configure --target=arm-none-symbianelf, so we should use configure options.

Also, I believe that SYMBIAN_SDK_PATH is moved from environment value to configure option such as MAC OS SDK PATH.
I hope you guys remember that Symbian is not only S60 (Nokia) but also UIQ (SonyEricsson, Motorola).
Attached patch continue woking on portings (obsolete) — Splinter Review
I'm very sorry for the delay of the porting works because of my busy Oct and
Sept. In Aug and Sept I tried to verify patched ptio.c, and it was good. I also
did some test cases run on more devices during last two months. This patch is
made against the latest trunk (4.7.4 beta). It is tested on a N80 device.
Attachment #331733 - Attachment is obsolete: true
Attachment #332007 - Attachment is obsolete: true
Attachment #349593 - Flags: review+
Attachment #331733 - Flags: review?(wtc)
Assignee: wtc → harry.li
Status: UNCONFIRMED → NEW
Ever confirmed: true
set platform to symbiab
OS: Other → Symbian
Hardware: Other → All
Severity: enhancement → normal
Hardware: All → ARM
To simplify things I have made this patch that only compiles and links on Symbian. I will then open a new bug for getting the tests running.
Attachment #349593 - Attachment is obsolete: true
I think we should change the line 

+        DEFINES="$DEFINES -D__PRODUCT_INCLUDE__=\$(SYMBIAN_SDK_PATH)Epoc32/include/variant/Symbian_OS_v9.1.hrh"

into 

+        DEFINES="$DEFINES -D__PRODUCT_INCLUDE__=\$(SYMBIAN_SDK_PATH)Epoc32/include/variant/Symbian_OS.hrh"

Then the configure.in script will be Symbian OS version independent if we ask users of the script to copy/move "Symbian _OS_v9.1/2/3.hrh" into "Symbian_OS.hrh". If you have other consideration to stick on v9.1(MR) or v9.2(FP1), please let me known. Rest of the patch is OK.
(In reply to comment #14)
> I think we should change the line 
> 
> +        DEFINES="$DEFINES
> -D__PRODUCT_INCLUDE__=\$(SYMBIAN_SDK_PATH)Epoc32/include/variant/Symbian_OS_v9.1.hrh"
> 
> into 
> 
> +        DEFINES="$DEFINES
> -D__PRODUCT_INCLUDE__=\$(SYMBIAN_SDK_PATH)Epoc32/include/variant/Symbian_OS.hrh"
> 
> Then the configure.in script will be Symbian OS version independent if we ask
> users of the script to copy/move "Symbian _OS_v9.1/2/3.hrh" into
> "Symbian_OS.hrh". If you have other consideration to stick on v9.1(MR) or
> v9.2(FP1), please let me known. Rest of the patch is OK.

You don't think it is better to leave the version number for the exact version we are supporting? And in that case, shouldn't it be 9.2 instead of 9.1 actually?
If we are going to specify the SDK version we are using, then we could leave the version number in filename. By changing 
+        SYMBIAN_SYS_INCLUDE="$SYMBIAN_SYS_INCLUDE -include Symbian_OS.hrh"
into
+        SYMBIAN_SYS_INCLUDE="$SYMBIAN_SYS_INCLUDE -include Symbian_OS_v9.2.hrh"
and changing
+        DEFINES="$DEFINES -D__PRODUCT_INCLUDE__=\$(SYMBIAN_SDK_PATH)Epoc32/include/variant/Symbian_OS_v9.1.hrh"
into
+        DEFINES="$DEFINES -D__PRODUCT_INCLUDE__=\$(SYMBIAN_SDK_PATH)Epoc32/include/variant/Symbian_OS_v9.2.hrh"

My previous sugguestion only helps back/forward porting Fennec for Symbian to 3rd MR (v3.0, v9.1.hrh)/3rd FP2(v3.2, v9.3.hrh) SDK versions for wider devices supporting. I am not sure if 3rd MR(3.0) devices can't run Fennec. BTW, in theory v3.2 devices can run binaries built with v3.1 SDK, compatibility problems happen rarely.
Where is $(SYMBIAN_SDK_PATH) coming from anyway? Typically we don't hardcode stuff like that into the build system, we require your INCLUDE paths etc are set properly in the environment. MozillaBuild has batch files that set up the build environment properly for Windows, for example. For the WinCE build, we've added a --with-wince-sdk configure option to point at the WinCE SDK (because we do some tricky overriding of bits in the header files).
It is set in the environment as per the instructions here: https://wiki.mozilla.org/Mobile/Symbian/NSPR

What in the "*-symbian*)" section do you think should be moved out?
Christian,

>> CC=arm-none-symbianelf-g++.exe
>> CXX=arm-none-symbianelf-g++.exe
>> LD=arm-none-symbianelf-ld.exe
>> AR=arm-none-symbianelf-ar.exe

If you set "--target=arm-none-symbianelf", this is not necessary.  And GCC_VERSION macro can detect gcc version.   So you should not use hardcored 3.4.3.

Also, symbian os version can detect to parse SYMBIAN_SDK_PATH value.
(In reply to comment #19)
> Christian,
> 
> >> CC=arm-none-symbianelf-g++.exe
> >> CXX=arm-none-symbianelf-g++.exe
> >> LD=arm-none-symbianelf-ld.exe
> >> AR=arm-none-symbianelf-ar.exe
> 
> If you set "--target=arm-none-symbianelf", this is not necessary.  And
> GCC_VERSION macro can detect gcc version.   So you should not use hardcored
> 3.4.3.

I will try to investigate what you suggest. however, the hardcoded 3.4.3 is found at Symbian build script, e.g. \Symbian\9.1\S60_3rd_MR\Epoc32\tools\cl_gcce.pm

> 
> Also, symbian os version can detect to parse SYMBIAN_SDK_PATH value.

Please be noted that SYMBIAN_SDK_PATH is not required to be something like "X:\Symbian\9.1\", I just install my symbian sdks at d:\30 d:\31 d:\50.
(In reply to comment #19)
> Christian,
> 
> >> CC=arm-none-symbianelf-g++.exe
> >> CXX=arm-none-symbianelf-g++.exe
> >> LD=arm-none-symbianelf-ld.exe
> >> AR=arm-none-symbianelf-ar.exe
> 
> If you set "--target=arm-none-symbianelf", this is not necessary.  And
> GCC_VERSION macro can detect gcc version.   So you should not use hardcored
> 3.4.3.

I will update the patch.

> Also, symbian os version can detect to parse SYMBIAN_SDK_PATH value.

I am not sure I understand this. Are you saying SYMBIAN_SDK_PATH doesn't have to be used in the configure file?
Updated patch according to comment #19.
Attachment #360295 - Attachment is obsolete: true
(In reply to comment #22)
> Created an attachment (id=360511) [details]
> Updated patch for building NSPR on Symbian
> 
> Updated patch according to comment #19.

The new patch still has the inconsistent .hrh filenames. I think you could choose either stick on specific version or using a general name according to Mozilla style, although I suggest the latter one.
Modified the Symbian headers to be 9.2 (as included in the Symbian SDK).
Attachment #360511 - Attachment is obsolete: true
(In reply to comment #21)

> I am not sure I understand this. Are you saying SYMBIAN_SDK_PATH doesn't have
> to be used in the configure file?

I thought we can use it if environment value has version string.  But correct way is like MacOS X.  (See --enable-macos-target option)

Also, now, current patch uses environment value for SDK PATH etc.  Other platforms such as MacOS X (--with-macos-sdk) and Windows CE (--with-wince-sdk) uses configure options.  I think configure options is better than environment path for SDK PATH.

And I have some question.

> AC_DEFINE(__arm__)
> CPU_ARCH=ARM

This should not define on WINSCW.  WINSCW is i386 arch.
With this patch you don't need to set any environment variables, but can instead build e.g. with these configure arguments for the emulator:

--target=arm-none-symbianelf --enable-symbian-target=WINSCW --with-symbian-sdk=/c/symbian/9.2/S60_3rd_FP1

And for the device like this:

--target=arm-none-symbianelf --enable-symbian-target=GCCE --with-symbian-sdk=/c/symbian/9.2/S60_3rd_FP1 --with-symbian-arm-toolchain=/c/Program\ Files/CSL\ Arm\ Toolchain
Attachment #360566 - Attachment is obsolete: true
Attachment #361528 - Flags: review?(wtc)
Comment on attachment 361528 [details] [diff] [review]
Patch without need for environment variables

1. nsprpub/config/Makefile.in

> ifeq ($(OS_ARCH),WINCE)
> TARGETS = $(PROGS)
> else
>+ifneq ($(OS_ARCH),SYMBIAN)
> PROGS	+= $(OBJDIR)/nsinstall$(PROG_SUFFIX)
>+endif
> TARGETS = $(PROGS) $(PLSRCS:.pl=)
> endif
> endif

Does it work if you rewrite the above like this?

>-ifeq ($(OS_ARCH),WINCE)
>+ifeq (,$(filter-out SYMBIAN WINCE,$(OS_ARCH)))
> TARGETS = $(PROGS)
> else
> PROGS	+= $(OBJDIR)/nsinstall$(PROG_SUFFIX)+endif
> TARGETS = $(PROGS) $(PLSRCS:.pl=)
> endif
> endif

2. nsprpub/configure.in

Do you still need those debug echo statements?

>+    echo -----------------------------------------------------------------------------
>+    echo Building with Symbian SDK in: $SYMBIAN_SDK_DIR
>+	echo Building with Symbian ARM toolchain in: $SYMBIAN_ARM_TOOLCHAIN
>+    echo -----------------------------------------------------------------------------

The third echo statement seems to start with a tab character.
In nsprpub/configure.in, the tab stop is 8, not 4.  So that
tab should be replaced by 4 spaces.

>+    AC_DEFINE(__arm__)
>+    AC_DEFINE(__SERIES60_30__)
>+    AC_DEFINE(__SERIES60_3X__)
>+    AC_DEFINE(__SYMBIAN32__)
...
>+    AC_DEFINE(__SUPPORT_CPP_EXCEPTIONS__)

These __XXX__ macros look like compiler predefined macros.
Are you sure we still need to define them?

>+#    PERL="\$(ACTIVE_PERL)"

Please remove this commented-out line.

>+        CFLAGS="$CFLAGS -O0 -inline off -wchar_t off -align 4 -warnings on -w nohidevirtual,nounusedexpr -msgstyle gcc -enum int -str pool -exc ms -trigraphs on -nostderr -gccdep -cwd source -i- -I\$(VPATH)"

Are you sure you want to use -O0?

>+        AC_DEFINE(__CW32__)
>+        AC_DEFINE(__WINS__)
>+        AC_DEFINE(__WINSCW__)

See above.  __XXX__ macros are usually compiler predefined
macros.  Do we still need to define them?

>+        DEFINES="$DEFINES -U_WIN32 -U__i386__"

I know we should undefine _WIN32 to avoid confusion with
Windows, but do we need to undefine __i386__?  Doesn't
the WINSCW target runs inside an emulator on Windows x86?
So it is correct for __i386__ to defined.

3. nsprpub/pr/src/pthreads/ptthread.c

>+#ifdef SYMBIAN
>+/* There is a _POSIX_THREAD_PRIORITY_SCHEDULING in unixtd.h in Symbian OS
>+ * include header files, so undef it here 
>+ */
>+#undef _POSIX_THREAD_PRIORITY_SCHEDULING
>+#endif

The comment should explain what happens if we don't
undefine _POSIX_THREAD_PRIORITY_SCHEDULING.  Does this
mean unixtd.h in Symbian OS should not define
_POSIX_THREAD_PRIORITY_SCHEDULING?  Or is NSPR using
_POSIX_THREAD_PRIORITY_SCHEDULING incorrectly?
Attached patch nsprpub_symbian5.patch (obsolete) — Splinter Review
(In reply to comment #27)
> (From update of attachment 361528 [details] [diff] [review])
> 1. nsprpub/config/Makefile.in
> 
> > ifeq ($(OS_ARCH),WINCE)
> > TARGETS = $(PROGS)
> > else
> >+ifneq ($(OS_ARCH),SYMBIAN)
> > PROGS	+= $(OBJDIR)/nsinstall$(PROG_SUFFIX)
> >+endif
> > TARGETS = $(PROGS) $(PLSRCS:.pl=)
> > endif
> > endif
> 
> Does it work if you rewrite the above like this?
> 
> >-ifeq ($(OS_ARCH),WINCE)
> >+ifeq (,$(filter-out SYMBIAN WINCE,$(OS_ARCH)))
> > TARGETS = $(PROGS)
> > else
> > PROGS	+= $(OBJDIR)/nsinstall$(PROG_SUFFIX)+endif
> > TARGETS = $(PROGS) $(PLSRCS:.pl=)
> > endif
> > endif
> 

Done.

> 2. nsprpub/configure.in
> 
> Do you still need those debug echo statements?
> 
> >+    echo -----------------------------------------------------------------------------
> >+    echo Building with Symbian SDK in: $SYMBIAN_SDK_DIR
> >+	echo Building with Symbian ARM toolchain in: $SYMBIAN_ARM_TOOLCHAIN
> >+    echo -----------------------------------------------------------------------------
> 
> The third echo statement seems to start with a tab character.
> In nsprpub/configure.in, the tab stop is 8, not 4.  So that
> tab should be replaced by 4 spaces.
> 

Changed tab into spaces. Information display is kept for development use in this submit.

> >+    AC_DEFINE(__arm__)
> >+    AC_DEFINE(__SERIES60_30__)
> >+    AC_DEFINE(__SERIES60_3X__)
> >+    AC_DEFINE(__SYMBIAN32__)
> ...
> >+    AC_DEFINE(__SUPPORT_CPP_EXCEPTIONS__)
> 
> These __XXX__ macros look like compiler predefined macros.
> Are you sure we still need to define them?
> 

These macros are added by Nokia/Symbian makmake build scripts rather than compiler predefined when building a project in Nokia tools. Here we are replacing their tools so we have to write them by hand.

> >+#    PERL="\$(ACTIVE_PERL)"
> 
> Please remove this commented-out line.
> 

Done.

> >+        CFLAGS="$CFLAGS -O0 -inline off -wchar_t off -align 4 -warnings on -w nohidevirtual,nounusedexpr -msgstyle gcc -enum int -str pool -exc ms -trigraphs on -nostderr -gccdep -cwd source -i- -I\$(VPATH)"
> 
> Are you sure you want to use -O0?
> 

I think before everything is proved to work, we'd better set optimization to none.

> >+        AC_DEFINE(__CW32__)
> >+        AC_DEFINE(__WINS__)
> >+        AC_DEFINE(__WINSCW__)
> 
> See above.  __XXX__ macros are usually compiler predefined
> macros.  Do we still need to define them?
> 

The same as above.

> >+        DEFINES="$DEFINES -U_WIN32 -U__i386__"
> 
> I know we should undefine _WIN32 to avoid confusion with
> Windows, but do we need to undefine __i386__?  Doesn't
> the WINSCW target runs inside an emulator on Windows x86?
> So it is correct for __i386__ to defined.
> 

This macro doesn't mess up anything now. I have removed it.

> 3. nsprpub/pr/src/pthreads/ptthread.c
> 
> >+#ifdef SYMBIAN
> >+/* There is a _POSIX_THREAD_PRIORITY_SCHEDULING in unixtd.h in Symbian OS
> >+ * include header files, so undef it here 
> >+ */
> >+#undef _POSIX_THREAD_PRIORITY_SCHEDULING
> >+#endif
> 
> The comment should explain what happens if we don't
> undefine _POSIX_THREAD_PRIORITY_SCHEDULING.  Does this
> mean unixtd.h in Symbian OS should not define
> _POSIX_THREAD_PRIORITY_SCHEDULING?  Or is NSPR using
> _POSIX_THREAD_PRIORITY_SCHEDULING incorrectly?

I have updated the comment:

/*
Because some Nokia Open C limitations, we will disable _POSIX_THREAD_PRIORITY_SCHEDULING here.
*/

Reference: http://www.forum.nokia.com/document/CDL_Extension_S60_3rd_Ed_FP2/GUID-719955DA-415B-420E-9F9B-F6DB37615EC5/html/s60_openc_api_specification4.html

However, I haven't get a clear proof yet whether thread priority scheduling is totally unavailable or not.
Attachment #361747 - Flags: review?
Attachment #361747 - Flags: review? → review?(wtc)
Attachment #361528 - Attachment is obsolete: true
Attachment #361528 - Flags: review?(wtc)
Attachment #361747 - Flags: review?(wtc)
Attachment #361747 - Flags: review?(ted.mielczarek)
Attachment #361747 - Flags: review+
Comment on attachment 361747 [details] [diff] [review]
nsprpub_symbian5.patch

r=wtc.  Ted, you may want to take a quick look at this
patch, too.

>+    AC_DEFINE(__arm__)
>+    AC_DEFINE(__SERIES60_30__)
>+    AC_DEFINE(__SERIES60_3X__)
>+    AC_DEFINE(__SYMBIAN32__)

Please add a comment to explain that you're defining
these __XXX__ macros to match what the Nokia/Symbian
makmake build scripts do.  By the way, is "makmake"
a typo?

I think we should only define __arm__ for OS_TARGET=GCCE.
For OS_TARGET=WINSCW, we should just use the predefined
__i386__.

>+#ifdef SYMBIAN
>+/* Because some Nokia Open C limitations, we will disable 
>+ * _POSIX_THREAD_PRIORITY_SCHEDULING here.
>+ */
>+#undef _POSIX_THREAD_PRIORITY_SCHEDULING
>+#endif

This comment should say what the limitations are.  For
example, is it because pthread_setschedparam supports
only SCHED_RR policy?
Comment on attachment 361747 [details] [diff] [review]
nsprpub_symbian5.patch

Ted, one thing I'd specifically like your comment on is
whether the --enable-symbian-target option is the right
way to do cross-compilation for Symbian OS.
(In reply to comment #29)
> I think we should only define __arm__ for OS_TARGET=GCCE.
> For OS_TARGET=WINSCW, we should just use the predefined
> __i386__.

I think this is used because in pr/include/md/_symbian.h the only supported architecture is __arm__

> 
> >+#ifdef SYMBIAN
> >+/* Because some Nokia Open C limitations, we will disable 
> >+ * _POSIX_THREAD_PRIORITY_SCHEDULING here.
> >+ */
> >+#undef _POSIX_THREAD_PRIORITY_SCHEDULING
> >+#endif
> 
> This comment should say what the limitations are.  For
> example, is it because pthread_setschedparam supports
> only SCHED_RR policy?

_POSIX_THREAD_PRIORITY_SCHEDULING is already defined in the Symbian include file unistd.h as:

#define        _POSIX_THREAD_PRIORITY_SCHEDULING 200112L
We should fix pr/include/md/_symbian.h and _symbian.cfg to
also support the __i386__ architecture used by OS_TARGET=WINSCW.

Re: _POSIX_THREAD_PRIORITY_SCHEDULING: I'm sorry that I'm being
stubborn, but it's important to note why we want to contradict
what the system header says.  It's fine to say: "we don't have
time to port the thread priority related code, so we undefine
_POSIX_THREAD_PRIORITY_SCHEDULING to turn that code off for now."
I'm curious what the limitations are exactly.

I suspect that you're running into problems because of this
code in nsprpub/pr/include/md/_pth.h:

237 #elif defined(LINUX) || defined(__GNU__) || defined(__GLIBC__) \
238     || defined(FREEBSD) || defined(SYMBIAN)
239 #define PT_PRIO_MIN            sched_get_priority_min(SCHED_OTHER)
240 #define PT_PRIO_MAX            sched_get_priority_max(SCHED_OTHER)

According to the reference
http://www.forum.nokia.com/document/CDL_Extension_S60_3rd_Ed_FP2/GUID-719955DA-415B-420E-9F9B-F6DB37615EC5/html/s60_openc_api_specification4.html
Open C only supports the SCHED_RR policy, so perhaps the
sched_get_priority_min(SCHED_OTHER) and
sched_get_priority_max(SCHED_OTHER) calls are failing.
On some systems, you need special privilege to use the
SCHED_RR policy.  If Symbian has that restriction, that'd
be a good reason to turn off _POSIX_THREAD_PRIORITY_SCHEDULING.
> Re: _POSIX_THREAD_PRIORITY_SCHEDULING: I'm sorry that I'm being
> stubborn, but it's important to note why we want to contradict
> what the system header says.  It's fine to say: "we don't have
> time to port the thread priority related code, so we undefine
> _POSIX_THREAD_PRIORITY_SCHEDULING to turn that code off for now."
> I'm curious what the limitations are exactly.
> 
> I suspect that you're running into problems because of this
> code in nsprpub/pr/include/md/_pth.h:
> 
> 237 #elif defined(LINUX) || defined(__GNU__) || defined(__GLIBC__) \
> 238     || defined(FREEBSD) || defined(SYMBIAN)
> 239 #define PT_PRIO_MIN            sched_get_priority_min(SCHED_OTHER)
> 240 #define PT_PRIO_MAX            sched_get_priority_max(SCHED_OTHER)
> 
> According to the reference
> http://www.forum.nokia.com/document/CDL_Extension_S60_3rd_Ed_FP2/GUID-719955DA-415B-420E-9F9B-F6DB37615EC5/html/s60_openc_api_specification4.html
> Open C only supports the SCHED_RR policy, so perhaps the
> sched_get_priority_min(SCHED_OTHER) and
> sched_get_priority_max(SCHED_OTHER) calls are failing.
> On some systems, you need special privilege to use the
> SCHED_RR policy.  If Symbian has that restriction, that'd
> be a good reason to turn off _POSIX_THREAD_PRIORITY_SCHEDULING.

In Symbian sched_get_priority_min/max is not supported even for SCHED_RR. The caller will get a value of 255 for max & 0 as min which is just a random value. The call should be moved to RThread::GetPriority if this min/max value is becomming critical for the execution.
Hi everyone. I am currently working in a project where we port an OSS
POSIX-based component to Symbian OS and thought I could share some comments
with you. I am relatively inexperienced with autoconf and the Mozilla build system, so please bear with me if I suggest something unfeasible.

1)
(In reply to comment #16)
> My previous sugguestion only helps back/forward porting Fennec for Symbian to
> 3rd MR (v3.0, v9.1.hrh)/3rd FP2(v3.2, v9.3.hrh) SDK versions for wider devices
> supporting. I am not sure if 3rd MR(3.0) devices can't run Fennec. BTW, in
> theory v3.2 devices can run binaries built with v3.1 SDK, compatibility
> problems happen rarely.

I agree that using the v9.1 .hrh would provide better portability and support also to S60 3rd ed. I suggest that the v9.1 header would be used, unless only S60 3rd. ed. FP1 / Symbian OS v9.2 and above  are specifically targeted.

2)
I fully understand the reasons of not using the Symbian OS build system to build Mozilla projects. I  am still slightly concerned of the maintenance effort involved in replicating the toolchain options to the Mozilla build system. Also different S60 SDK version may have differences in the toolchain options. I admit it might get elaborate, but would it be possible to make a specifically crafted Symbian OS project to capture the toolchain command-lines during the Mozilla build configuration? I am thinking of replacing the toolchain executables with command-line capturing stubs and "building" a dummy Symbian OS project during this configuration step 

3)
I noticed that you are currently supporting only the GCCE compiler for HW builds. I suggest you also provide support for the ARM RVCT compiler as it is preferred by OEMs to build device firmwares. The GCCE version supported by Symbian also has bugs in it, which I noticed you have already bumped into.

4)
The Symbian OS build system uses a wealth of other tools in addition to compilers and linkers. It seems that you have currently put invocations of those tools (e.g. elf2e32) directly into component Makefile.in files. This means that the verbose tool command-lines will be repeated for all components with Makefiles.

A longer-term solution could be to add new configuration macros to top-level make config that would contain the tool invocation details, reducing the Makefile contents to only the component-specific option values, such as component UIDs, capabilities etc. I think the top-level macros would also reduce the need for checking for the Symbian platform in Makefiles, as the macros could be ignored on other platforms.
(In reply to comment #34)
> 1)
> (In reply to comment #16)
> > My previous sugguestion only helps back/forward porting Fennec for Symbian to
> > 3rd MR (v3.0, v9.1.hrh)/3rd FP2(v3.2, v9.3.hrh) SDK versions for wider devices
> > supporting. I am not sure if 3rd MR(3.0) devices can't run Fennec. BTW, in
> > theory v3.2 devices can run binaries built with v3.1 SDK, compatibility
> > problems happen rarely.
> 
> I agree that using the v9.1 .hrh would provide better portability and support
> also to S60 3rd ed. I suggest that the v9.1 header would be used, unless only
> S60 3rd. ed. FP1 / Symbian OS v9.2 and above  are specifically targeted.

I seem to have lost a bit from this comment from my previous post: Could this platform .hrh file version problem be settled by the configure script taking a peek at the Symbian SDK's [--with-symbian-sdk]/epoc32/variant directory during configuration time?
(In reply to comment #35)
> (In reply to comment #34)
> > 1)
> > (In reply to comment #16)
> > > My previous sugguestion only helps back/forward porting Fennec for Symbian to
> > > 3rd MR (v3.0, v9.1.hrh)/3rd FP2(v3.2, v9.3.hrh) SDK versions for wider devices
> > > supporting. I am not sure if 3rd MR(3.0) devices can't run Fennec. BTW, in
> > > theory v3.2 devices can run binaries built with v3.1 SDK, compatibility
> > > problems happen rarely.
> > 
> > I agree that using the v9.1 .hrh would provide better portability and support
> > also to S60 3rd ed. I suggest that the v9.1 header would be used, unless only
> > S60 3rd. ed. FP1 / Symbian OS v9.2 and above  are specifically targeted.
> 
> I seem to have lost a bit from this comment from my previous post: Could this
> platform .hrh file version problem be settled by the configure script taking a
> peek at the Symbian SDK's [--with-symbian-sdk]/epoc32/variant directory during
> configuration time?

yes, with the latest patch SDK path can be selected by --with-symbian-sdk , but the variant file name "symbian_os_v9.x.hrh" still has to be hard coded in configure.in.
Attachment #361747 - Flags: review?(ted.mielczarek) → review-
Comment on attachment 361747 [details] [diff] [review]
nsprpub_symbian5.patch

AC_MSG_ERROR([Missing OS_TARGET for ${target}. Set --enable-symbian-target to with 'WINSCW' or 'GCCE'.])

I think you should just pick a default flavor and go with it, and allow overriding the default with the option. Bonus points if you can detect that the selected target won't work with the provided toolchain.

+	CC=mwccsym2.exe
+	CXX=mwccsym2.exe
+        LD=mwldsym2.exe
+        AR=mwldsym2.exe

Your indentation here is inconsistent, please clean it up.

+        SYMBIAN_SYS_INCLUDE="$SYMBIAN_SYS_INCLUDE -include $SYMBIAN_SDK_DIR/EPOC32/INCLUDE/GCCE/GCCE.h -I\"$SYMBIAN_ARM_TOOLCHAIN/lib/gcc/arm-none-symbianelf/3.4.3/include\""

I don't understand this, does the symbian GCC really not know about its own standard include paths?
Remove 3 MACROs which might be not used in Mozilla.
Fix default $OS_TARGET to GCCE.
Remove unused system include path ($SYMBIAN_ARM_TOOLCHAIN/lib/gcc/arm-none-symbianelf/3.4.3/include). This was added in original purpose of simulating Symbian's build scripts. According to the README in that path, it's only used for Symbian's build system. At this moment the build is OK without it.
Also the diff in ptthread.c is removed cause I think it's better to put that in source patch bug 432430.

Thanks for reviewing.
Attachment #361747 - Attachment is obsolete: true
Attachment #364024 - Flags: review?(ted.mielczarek)
Summary: Build for Symbian targets with autoconf and (gnu)make → Build NSPR for Symbian targets with autoconf and (gnu)make
(In reply to comment #38)
> Also the diff in ptthread.c is removed cause I think it's better to put that in
> source patch bug 432430.
> 
Without that change you can't actually build. So it would be good to get it into this patch, so we have a point where we knows it builds.
Attached patch Updated patch (obsolete) — Splinter Review
After talking to Harry on irc, I have made an updated patch to comment #38 with the change to ptthread.c included as per comment #39 plus added SYMBIAN_SDK_DIR and SYMBIAN_ARM_TOOLCHAIN to nsprpub/config/autoconf.mk.in in order for them to be visible when doing build e.g. when building in nsprpub/pr/tests/
Attachment #364024 - Attachment is obsolete: true
Attachment #364397 - Flags: review?(ted.mielczarek)
Attachment #364024 - Flags: review?(ted.mielczarek)
(In reply to comment #38)
> Remove unused system include path
> ($SYMBIAN_ARM_TOOLCHAIN/lib/gcc/arm-none-symbianelf/3.4.3/include). This was
> added in original purpose of simulating Symbian's build scripts. According to
> the README in that path, it's only used for Symbian's build system. At this
> moment the build is OK without it.

Christian: it looks like your new patch still includes the --with-arm-toolchain despite not using the variable anywhere. I think you should just remove it.
Attached patch Updated patch (obsolete) — Splinter Review
Doh! Removed the --with-arm-toolchain.
Attachment #364397 - Attachment is obsolete: true
Attachment #364998 - Flags: review?(ted.mielczarek)
Attachment #364397 - Flags: review?(ted.mielczarek)
Comment on attachment 364998 [details] [diff] [review]
Updated patch

+SYMBIAN_ARM_TOOLCHAIN = @SYMBIAN_ARM_TOOLCHAIN@

Please remove this line too.

+AC_SUBST(SYMBIAN_ARM_TOOLCHAIN)

As well as this one.

r=me with those lines removed. If you attach an updated patch against NSPR CVS HEAD, wtc or I can check it in for you.
Attachment #364998 - Flags: review?(ted.mielczarek) → review+
Updated the patch according to comment 43 and verified it builds on NSPR CVS head.
Attachment #364998 - Attachment is obsolete: true
Checked in on NSPR HEAD:
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.253; previous revision: 1.252
done
Checking in config/Makefile.in;
/cvsroot/mozilla/nsprpub/config/Makefile.in,v  <--  Makefile.in
new revision: 1.24; previous revision: 1.23
done
Checking in config/autoconf.mk.in;
/cvsroot/mozilla/nsprpub/config/autoconf.mk.in,v  <--  autoconf.mk.in
new revision: 1.38; previous revision: 1.37
done
Checking in pr/src/pthreads/ptthread.c;
/cvsroot/mozilla/nsprpub/pr/src/pthreads/ptthread.c,v  <--  ptthread.c
new revision: 3.80; previous revision: 3.79
done
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Ted,

Thanks a lot for reviewing and checking in the patch.

mozilla/nsprpub/configure is checked in to CVS.  So
whenever you change mozilla/nsprpub/configure.in, you
also need to regenerate mozilla/nsprpub/configure and
check it in.

There is one more twist: mozilla/nsprpub/configure
contains a manual change for MKS Korn Shell support.
(MKS Toolkit is a set of Unix command-line tools for
Windows, similar to Cygwin and MSYS.)  I just checked
in a new mozilla/nsprpub/configure, so if you do

% cvs -q co -A NSPR
% cd mozilla/nsprpub
% autoconf-2.13
% cvs diff -u configure

You will see the manual change I made.  I'd appreciate
it if you could make the same change every time you change
mozilla/nsprpub/configure.  Thanks.

The request for an automatic way to add the change to
mozilla/nsprpub/configure for MKS support is bug 442706.
If you know how to do that with autoconf and m4, that'll
be great.  Otherwise we can create a shell script to apply
the change.
I cited the wrong bug.  It should be bug 480680.
Oops, sorry! I've gotten spoiled by all this time of checking into mozilla CVS/mozilla Hg without having to commit configure. I'll keep that in mind in the future.
Christian, please review this patch.  On Linux, I
found that OS_TARGET was incorrectly set to GCCE.

In the interest of time, I've checked in this patch.

Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.254; previous revision: 1.253
done
Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.249; previous revision: 1.248
done
Attachment #365330 - Flags: review?(christian.bugzilla)
This build script cannot generate e32image format. So, I file a new bug as bug 481329 to generate e32image using elf2e32.
Comment on attachment 365330 [details] [diff] [review]
Don't set OS_TARGET to GCCE

Looks good.
Attachment #365330 - Flags: review?(christian.bugzilla) → review+
We need to preserve the existing value of OS_TARGET if the
--enable-win32-target or --enable-symbian-target is not
specified.

If I specify --enable-win32-target=WIN95 only, OS_TARGET
is set to WIN95 but then immediately cleared because
--enable-symbian-target is not specified.  This patch fixes
that problem.

Since this is a serious problem, I've checked in this patch
on the NSPR trunk (NSPR 4.8) with TBR=ted.

Checking in configure;
/cvsroot/mozilla/nsprpub/configure,v  <--  configure
new revision: 1.250; previous revision: 1.249
done
Checking in configure.in;
/cvsroot/mozilla/nsprpub/configure.in,v  <--  configure.in
new revision: 1.255; previous revision: 1.254
done
Attachment #368463 - Flags: review?(ted.mielczarek)
Comment on attachment 368463 [details] [diff] [review]
Don't clear OS_TARGET if --enable-win32-target or --enable-symbian-target is not specified

Ted,

This patch clearly shows --enable-win32-target and --enable-symbian-target
do exactly the same thing.  Should we consolidate them into one option, say
--enable-os-target or --enable-target?

We'll need to keep --enable-win32-target for backward compatibility, but
use the new option for both Win32 and Symbian.

Alternatively, should we use the "standard" way of specifying a cross-compilation
target, rather than a --enable-os-target option?
Comment on attachment 368463 [details] [diff] [review]
Don't clear OS_TARGET if --enable-win32-target or --enable-symbian-target is not specified

This is obviously fine as a bustage fix. It does seem silly to have parallel options like this. I'm not sure what the --target options would look like to auto-specify this, since autoconf doesn't know anything about Symbian.
Attachment #368463 - Flags: review?(ted.mielczarek) → review+
It seems to me we should be driving these decisions by looking at the target triplet (i.e. --target).  Adding options like --enable-symbian-target, as suggested in the patch for bug 479585, is not the right way to go.

Target triplets are pretty flexible: you can make up your own, as long as you try to be consistent with existing practice.  Looking at GCC, the GNU binutils, and GDB, I see the following patterns being checked for:

arm*-*-symbianelf* --- GCC, binutils, gdb
sh*-*-symbianelf*  --- GNU ld, gas

In bug 479585, attachment 368662 [details] [diff] [review], Harry Li is suggesting WINSCW and GCCE as possible values for --enable-symbian-target.  I assume 'GCCE' means GCC/ELF; we could use a --target of *-*-symbianelf* for that.  WINSCW is CodeWarrior; we could call that *-*-symbian-winscw.  Then, the target triplet would tell us what we needed to know.
You need to log in before you can comment on or make changes to this bug.