1228208 - MOZ_ICU_CFLAGS are passed to late, conflict with system ICU

Status: RESOLVED FIXED

(Core :: JavaScript: Internationalization API, defect)

Description

[Jan Beich]

Bug 1175413 likely regressed bug 1054154. FreeBSD remains sensitive to -I order where -isystem should have been used. pkg-config and ${X_CFLAGS} from autoconf are oblivious to it, so the build fails similar to bug 1218027.

  c++ -o Unified_cpp_util_internal0.o -c -I../../../../dist/stl_wrappers -I../../../../dist/system_wrappers -include /mozilla-central/config/gcc_hidden.h -DSTATIC_EXPORTABLE_JS_API -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/mozilla-central/intl/unicharutil/util/internal -I. -I/mozilla-central/intl/unicharutil/util -I../../../../dist/include  -I/objdir/dist/include/nspr -I/objdir/dist/include/nss       -fPIC  -DMOZILLA_CLIENT -include ../../../../mozilla-config.h -MD -MP -MF .deps/Unified_cpp_util_internal0.o.pp -Qunused-arguments   -I/usr/local/include -Qunused-arguments -Wno-unused-local-typedef -Wall -Wempty-body -Woverloaded-virtual -Wsign-compare -Wwrite-strings -Wno-invalid-offsetof -Wno-inline-new-delete -Wno-unused-local-typedef -Wno-c++0x-extensions -Wno-extended-offsetof -Wno-unknown-warning-option -Wno-return-type-c-linkage -fno-exceptions -fno-strict-aliasing -fno-rtti -fno-exceptions -fno-math-errno -std=gnu++0x -pipe  -DNDEBUG -DTRIMMED -g -O -fomit-frame-pointer  -I/mozilla-central/intl/icu/source/common -I/mozilla-central/intl/icu/source/i18n   /objdir/intl/unicharutil/util/internal/Unified_cpp_util_internal0.cpp
  In file included from /objdir/intl/unicharutil/util/internal/Unified_cpp_util_internal0.cpp:11:
  In file included from /mozilla-central/intl/unicharutil/util/ICUUtils.cpp:8:
  In file included from /mozilla-central/intl/unicharutil/util/ICUUtils.h:18:
  In file included from /usr/local/include/unicode/unum.h:20:
  In file included from /usr/local/include/unicode/uloc.h:25:
  In file included from /usr/local/include/unicode/uenum.h:24:
  In file included from /usr/local/include/unicode/strenum.h:14:
  In file included from /usr/local/include/unicode/unistr.h:31:
  In file included from /usr/local/include/unicode/std_string.h:33:
  In file included from ../../../../dist/stl_wrappers/string:50:
  In file included from ../../../../dist/system_wrappers/string:3:
  In file included from /usr/include/c++/v1/string:439:
  In file included from ../../../../dist/stl_wrappers/algorithm:50:
  In file included from ../../../../dist/system_wrappers/algorithm:3:
  In file included from /usr/include/c++/v1/algorithm:628:
  In file included from ../../../../dist/stl_wrappers/memory:50:
  In file included from ../../../../dist/system_wrappers/memory:3:
  In file included from /usr/include/c++/v1/memory:600:
  In file included from ../../../../dist/system_wrappers/typeinfo:3:
  /usr/include/c++/v1/typeinfo:72:7: error: visibility does not match previous
	declaration
  class _LIBCPP_EXCEPTION_ABI type_info
	^
  /usr/include/c++/v1/__config:221:47: note: expanded from macro
	'_LIBCPP_EXCEPTION_ABI'
  #define _LIBCPP_EXCEPTION_ABI __attribute__ ((__visibility__("default")))
						^
  /mozilla-central/config/gcc_hidden.h:6:13: note: previous attribute is here
  #pragma GCC visibility push(hidden)
	      ^
  1 error generated.

To fix this -I/mozilla-central/intl/icu/source/common, etc. should come before -I/usr/local/include effectively restoring LOCAL_INCLUDES behavior.

Comment 1

[Jan Beich]

Attachment: ugly workaround

typeinfo issue moved to bug 1228227. -I order still matters to avoid linking errors due to ABI mismatch between system and bundled ICU or their visibility.

Comment 2

[Jan Beich]

Attachment: ugly workaround, v1

Missed quite a few places which resulted in linking errors e.g.,

  intl/unicharutil/util/nsUnicodeProperties.cpp:137: error: undefined reference to 'u_charMirror_55'
  intl/unicharutil/util/nsUnicodeProperties.cpp:147: error: undefined reference to 'u_isMirrored_55'
  intl/unicharutil/util/nsUnicodeProperties.cpp:157: error: undefined reference to 'u_getCombiningClass_55'
  intl/unicharutil/util/nsUnicodeProperties.cpp:304: error: undefined reference to 'u_getIntPropertyValue_55'
  intl/unicharutil/util/nsUnicodeProperties.cpp:304: error: undefined reference to 'u_getIntPropertyValue_55'
  c++: error: linker command failed with exit code 1 (use -v to see invocation)

Comment 3

[Jan Beich]

Attachment: Make sure ICU flags are prepended before system flags

Comment 4

[Jan Beich]

Comment on attachment 8692392 [details] [diff] [review]
ugly workaround, v1

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ce1141a5be5

Comment 5

[Jan Beich]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e537ac2aeb8

Comment 6

[Jan Beich]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3169d82f3df1

Comment 7

[Jan Beich]

Attachment: Make sure ICU flags are prepended before system flags, v0

Less ugly than attachment 8692392 [details] [diff] [review] but I'm not sure it's the right approach.
Green on Try in comment 6 (via bzpost).

Comment 8

[Mike Hommey [:glandium] (PTO. Back on Nov 25)]

Comment on attachment 8692888 [details] [diff] [review]
Make sure ICU flags are prepended before system flags, v0

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

::: build/autoconf/icu.m4
@@ -20,5 @@
>      PKG_CHECK_MODULES(MOZ_ICU, icu-i18n >= 50.1)
>      MOZ_SHARED_ICU=1
> -else
> -    MOZ_ICU_CFLAGS="-I$_topsrcdir/intl/icu/source/common -I$_topsrcdir/intl/icu/source/i18n"
> -    AC_SUBST_LIST(MOZ_ICU_CFLAGS)

A simpler solution, IMHO, would be to use a different variable in this case, leaving MOZ_ICU_CFLAGS empty. So you can do something like:

  MOZ_ICU_INCLUDES="/intl/icu/source/common /intl/icu/source/i18n"
  AC_SUBST_LIST(MOZ_ICU_INCLUDES)

(yes, without -I$_topsrcdir and starting with a / for each) and in moz.build files:
  LOCAL_INCLUDES += CONFIG['MOZ_ICU_INCLUDES']

which will also work in the native ICU case since that would expand to LOCAL_INCLUDES += []

Comment 9

[Jan Beich]

Attachment: Make sure ICU flags are prepended before system flags, v1

https://treeherder.mozilla.org/#/jobs?repo=try&revision=49d6733d5d40
And green local build with --with-system-icu.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/35ac64d68809

Comment 11

[Jim Blandy :jimb]

This changeset seems to have introduced a mozbuild error, when building js/src stand-alone. I get the following error in changeset 35ac64d68809, but not in its parent 9b6075887a72

Reticulating splines...
Traceback (most recent call last):
  File "./config.status", line 449, in <module>
    config_status(**args)
  File "/home/jimb/moz/dbg/python/mozbuild/mozbuild/config_status.py", line 178, in config_status
    the_backend.consume(definitions)
  File "/home/jimb/moz/dbg/python/mozbuild/mozbuild/backend/base.py", line 118, in consume
    for obj in objs:
  File "/home/jimb/moz/dbg/python/mozbuild/mozbuild/frontend/emitter.py", line 165, in emit
    for out in output:
  File "/home/jimb/moz/dbg/python/mozbuild/mozbuild/frontend/reader.py", line 1062, in read_mozbuild
    raise bre
mozbuild.frontend.reader.BuildReaderError: 
==============================
ERROR PROCESSING MOZBUILD FILE
==============================

The error occurred while processing the following file:

    /home/jimb/moz/dbg/js/src/moz.build

The error was triggered on line 670 of this file:

    LOCAL_INCLUDES += CONFIG['MOZ_ICU_INCLUDES']

An error was encountered as part of executing the file itself. The error appears to be the fault of the script.

The error as reported by Python is:

    ["TypeError: 'NoneType' object is not iterable\n"]

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/35ac64d68809

Comment 13

[Jim Blandy :jimb]

I had neglected to rebuild 'js/src/configure'. Problem solved.