Closed Bug 912371 Opened 6 years ago Closed 6 years ago

ICU cross compiling support

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla28

People

(Reporter: m_kato, Assigned: m_kato)

References

Details

Attachments

(3 files, 1 obsolete file)

Although Android (and B2G) needs more hacks, we should add general cross compile code for ICU.
Attached patch Add cross support (obsolete) — Splinter Review
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.
Attachment #799331 - Attachment is obsolete: true
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?
Attachment #806543 - Flags: review?(mh+mozilla)
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
Attachment #806543 - Flags: review?(mh+mozilla) → feedback+
Blocks: 921040
Attached patch patch v2Splinter Review
Attachment #829143 - Flags: review?(mh+mozilla)
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.
Attachment #829143 - Flags: review?(mh+mozilla) → review+
Blocks: 937901
Blocks: 866301
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)
Blocks: 864843
https://hg.mozilla.org/mozilla-central/rev/4110a8986a2a
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Depends on: 940246
Depends on: 940324
backed out
https://hg.mozilla.org/integration/mozilla-inbound/rev/9f64519c330f
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch v3Splinter Review
not test yet on mac x86
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.
Attachment #8335145 - Flags: review?(mh+mozilla)
test is passed on spidermonkey only build on i686 linux on x86-64 linux and --disable-optimize on win32.
Target Milestone: mozilla28 → ---
Comment on attachment 8335145 [details] [diff] [review]
v3

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

Kind of awful, but meh.
Attachment #8335145 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/d22b3851aff0
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
No longer depends on: 940324
Duplicate of this bug: 940324
Duplicate of this bug: 934715
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.