Closed Bug 960505 Opened 6 years ago Closed 6 years ago
Make it easier for Mozilla code that uses ICU to be compiled with the necessary ICU include paths
I have a patch for this that I'll clean up and attach shortly.
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.
Summary: Export headers from ICU so that Mozilla code can use it → Make it easier for Mozilla code that uses ICU to be compiled with the necessary ICU include paths
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.
Attachment #8367807 - Flags: review?(mh+mozilla) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.