912371 - ICU cross compiling support

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Makoto Kato [:m_kato]]

Although Android (and B2G) needs more hacks, we should add general cross compile code for ICU.

Comment 1

[Makoto Kato [:m_kato]]

Attachment: Add cross support

Comment 2

[Jan Beich]

Comment on attachment 799331 [details] [diff] [review]
Add cross support

Review of attachment 799331 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/configure.in
@@ +4298,2 @@
>              ;;
> +        *bsd*)

DragonFly doesn't have *bsd* in config.guess string from which
HOST_OS_ARCH is derived in native builds on unlisted platforms.

Comment 3

[Makoto Kato [:m_kato]]

Attachment: Add cross build support for ICU

Comment 4

[Makoto Kato [:m_kato]]

Comment on attachment 806543 [details] [diff] [review]
Add cross build support for ICU

Since ICU tree is out of spidermonkey tree, we cannot use AC_OUTPUT_SUBDIRS for configure of ICU.  Because objdir root of SpiderMonkey only build is just $objdir instead of $objdir/js/src.  If using AC_OUTPUT_SUBDIRS(../../intl/icu/source), objdir of ICU becomes $objdir/../../intl/icu/source.

Also, should I use configure for host tools instead of runConfigureICU?

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 806543 [details] [diff] [review]
Add cross build support for ICU

Review of attachment 806543 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/Makefile.in
@@ +257,5 @@
>  # - Force ICU to use the standard suffix for object files because expandlibs
>  #   will discard all files with a non-standard suffix (bug 857450).
>  # - Options for genrb: -k strict parsing; -R omit collation tailoring rules.
>  buildicu:
> +ifneq (,$(CROSS_COMPILE))

ifdef CROSS_COMPILE

@@ +258,5 @@
>  #   will discard all files with a non-standard suffix (bug 857450).
>  # - Options for genrb: -k strict parsing; -R omit collation tailoring rules.
>  buildicu:
> +ifneq (,$(CROSS_COMPILE))
> +	$(GMAKE) $(ICU_GMAKE_OPTIONS) -C intl/icu/host STATIC_O=$(OBJ_SUFFIX) GENRBOPTS='-k -R'

Is it possible to make it build the strict subset that is required for the target build?

@@ +265,4 @@
>  	$(ICU_LIB_RENAME)
>  
>  distclean clean::
> +ifneq (,$(CROSS_COMPILE))

ifdef

::: js/src/configure.in
@@ +4252,5 @@
>          case "$OS_TARGET" in
>              WINNT)
>                  ICU_LIB_NAMES="icuin icuuc icudt"
>                  ;;
> +            *)

Better to keep an explicit list.

@@ +4350,5 @@
> +        ICU_CXXFLGAS="$ICU_CXXFLAGS $MOZ_DEBUG_FLAGS"
> +        ICU_LDFLAGS="$MOZ_DEBUG_LDFLAGS"
> +        if test -z "$MOZ_DEBUG"; then
> +            # To generate debug symbols, it require MOZ_DEBUG_FLAGS.  But not debug build.
> +            ICU_CFLGAS="$ICU_CFLAGS -UDEBUG -DNDEBUG"

typo

@@ +4351,5 @@
> +        ICU_LDFLAGS="$MOZ_DEBUG_LDFLAGS"
> +        if test -z "$MOZ_DEBUG"; then
> +            # To generate debug symbols, it require MOZ_DEBUG_FLAGS.  But not debug build.
> +            ICU_CFLGAS="$ICU_CFLAGS -UDEBUG -DNDEBUG"
> +            ICU_CXXFLGAS="$ICU_CXXFLAGS -UDEBUG -DNDEBUG"

typo

Comment 6

[Makoto Kato [:m_kato]]

Attachment: patch v2

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 829143 [details] [diff] [review]
patch v2

Review of attachment 829143 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/Makefile.in
@@ +254,5 @@
>  # - Options for genrb: -k strict parsing; -R omit collation tailoring rules.
>  buildicu:
>  # ICU's build system is full of races, so force non-parallel build.
> +ifdef CROSS_COMPILE
> +	+$(ICU_MAKE) -j1 -C intl/icu/host STATIC_O=$(OBJ_SUFFIX) GENRBOPTS='-k -R'

I'm pretty sure we can find better options for this, to limit what it builds.

@@ +263,5 @@
>  distclean clean::
> +ifdef CROSS_COMPILE
> +	$(call SUBMAKE,$@,intl/icu/host)
> +endif
> +	$(call SUBMAKE,$@,intl/icu/source)

s/source/target/, maybe.

::: js/src/build/autoconf/codeset.m4
@@ +1,1 @@
> +# codeset.m4 serial AM1 (gettext-0.10.40)

This ought to be a hg cp.

::: js/src/configure.in
@@ +4253,5 @@
> +        # ICU requires RTTI
> +        if test "$GNU_CC"; then
> +            HOST_ICU_CXXFLAGS=`echo $HOST_ICU_CXXFLAGS | sed 's|-fno-rtti|-frtti|g'`
> +        else
> +            if test "$_MSC_VER"; then

elif

@@ +4263,5 @@
> +        if test -n "$MOZ_DEBUG"; then
> +            HOST_ICU_BUILD_OPTS="$HOST_ICU_BUILD_OPTS --enable-debug"
> +        fi
> +
> +        abs_srcdir=`(cd $srcdir; pwd)`

why do you need abs_srcdir?

@@ +4278,5 @@
> +        dnl Shell quoting is fun.
> +                ${ICU_SRCDIR+"$ICU_SRCDIR"} \
> +                --enable-static --disable-shared \
> +                --enable-extras=no --enable-icuio=no --enable-layout=no \
> +                --enable-tests=no --enable-samples=no || exit 1

--disable-* instead of --enable-*=no?

@@ +4331,5 @@
> +            ICU_CXXFLAGS=`echo $ICU_CXXFLAGS | sed 's|-GR-|-GR|g'`
> +        fi
> +    fi
> +
> +    # We cannout use AC_OUTPUT_SUBDIRS since ICU tree is out of spidermonkey.

cannot

@@ +4335,5 @@
> +    # We cannout use AC_OUTPUT_SUBDIRS since ICU tree is out of spidermonkey.
> +    # When using AC_OUTPUT_SUBDIRS, objdir of ICU is out of objdir
> +    # due to relative path.
> +    # If building ICU moves into root of mozilla tree, we can use
> +    # AC_OUTPUT_SUBDIR instead of.

instead.

Comment 8

[Makoto Kato [:m_kato]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4110a8986a2a

> I'm pretty sure we can find better options for this, to limit what it builds.

I added -C option (-C or --noBinaryCollation  do not generate binary collation image)

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/4110a8986a2a

Comment 10

[Makoto Kato [:m_kato]]

backed out
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f64519c330f

Comment 11

[Makoto Kato [:m_kato]]

Attachment: v3

not test yet on mac x86

Comment 12

[Makoto Kato [:m_kato]]

Comment on attachment 8335145 [details] [diff] [review]
v3

- Need -Od for disable optimize at force when using --disable-optimize.
- Also RTL_FLAGS isn't include CFLAGS, I have to add it to CFLAGS for msvs build
- since CROSS_COMPILE may not set, set --build=$target for non-cross compile.

Comment 13

[Makoto Kato [:m_kato]]

test is passed on spidermonkey only build on i686 linux on x86-64 linux and --disable-optimize on win32.

Comment 14

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

> backed out
> https://hg.mozilla.org/integration/mozilla-inbound/rev/9f64519c330f

This backout made m-c:

http://hg.mozilla.org/mozilla-central/rev/9f64519c330f

Comment 15

[Mike Hommey [:glandium]]

Comment on attachment 8335145 [details] [diff] [review]
v3

Review of attachment 8335145 [details] [diff] [review]:
-----------------------------------------------------------------

Kind of awful, but meh.

Comment 16

[Makoto Kato [:m_kato]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d22b3851aff0

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d22b3851aff0