960505 - Make it easier for Mozilla code that uses ICU to be compiled with the necessary ICU include paths

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Jonathan Watt [:jwatt]]

I have a patch for this that I'll clean up and attach shortly.

Comment 1

[Jonathan Watt [:jwatt]]

Turns out my patch didn't really generalize.

Ideally I wanted to put the ICU headers under a dist/include/icu directory and include headers with a path like:

  #include "icu/i18n/unicode/decimfmt.h"

That would provide clean paths and would allow any mozilla file to use these headers without requiring extra include paths to be added to its moz.build file. Unfortunately that doesn't work because, for example, decimfmt.h contains:

  #include "unicode/dcfmtsym.h"

It might seem like it would be possible to export the headers to dist/include/icu anyway and then add $(DIST)/include/icu/i18n etc. to GENERATED_INCLUDES (or something) in the moz.build of any files that need to use the ICU headers in order to get the include directives in ICU headers working. There are several reasons not to do that though. First we need to support system ICU, and it expects those headers to be used from /usr/include after make install has been run. So the paths we need to use in include directives are pretty much forced on us (if only ICU had a nice clean include structure like protobuf, which has a structure that puts everything under a nice clean "google/protobuf" directory so you end up with #include <google/protobuf/stubs/common.h> etc.). Second, if moz.build files are going to need to set GENERATED_INCLUDES (or something) then they might as well set LOCAL_INCLUDES (or something similar) which is what spidermonkey does now. Third, it would be undesirable to allow ICU headers to be included using two different include paths.

Exporting the ICU headers isn't the right thing to do then, IMO. Rather we should simply make it easier for parts of the code that need ICU headers to add the necessary include paths.

Comment 2

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 3

[Jonathan Watt [:jwatt]]

Attachment: patch

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8367807 [details] [diff] [review]
patch

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

::: configure.in
@@ +3927,5 @@
>      PKG_CHECK_MODULES(MOZ_ICU, icu-i18n >= 50.1)
>      MOZ_JS_STATIC_LIBS="$MOZ_JS_STATIC_LIBS $MOZ_ICU_LIBS"
>      MOZ_SHARED_ICU=1
> +else
> +    MOZ_ICU_CFLAGS=" -I$_topsrcdir/intl/icu/source/common -I$_topsrcdir/intl/icu/source/i18n"

Make that MOZ_ICU_CFLAGS='-I$(topsrcdir)/intl/icu/source/common -I$(topsrcdir)/intl/icu/source/i18n' (with single quotes and $(topsrcdir) instead of $_topsrcdir) and that'll likely fix your build errors on windows. r+ with this change.

Comment 5

[Jonathan Watt [:jwatt]]

Thank you.

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

Comment 6

[Carsten Book [:Tomcat]]

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