1046186 - build fails in intl/icu due to hardcoded usage of __timezone

Status: RESOLVED WONTFIX

(Firefox Build System :: General, defect)

Description

[maillist-mozilla]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140512150801

Steps to reproduce:

try to build firefox 31.0 on musl libc based system


Actual results:

/src/build/firefox/mozilla-release/intl/icu/source/common/putil.cpp:638:12: error: '__timezone' was not declared in this scope
/src/build/firefox/mozilla-release/intl/icu/source/common/putil.cpp:668:1: error: control reaches end of non-void function [-Werror=return-type]



Expected results:

the icu configure scripts has tests to check whether __timezone exists.
however the configure script is not used, instead a hardcoded configuration created on a GLIBC system is used.
i would have expected that
1) firefox doesnt ship external libs in its own tree 
   and if it depends on ICU it should just raise an error in the configure script if its not installed
2) if it ships embedded libraries it should at least call their configure scripts and use them as intended

Comment 1

[maillist-mozilla]

the bug is in
intl/icu/source/common/putilimp.h

#elif U_PLATFORM_IS_LINUX_BASED
#   if !defined(__UCLIBC__)
    /* __timezone is only available in glibc */
#       define U_TIMEZONE __timezone
#   endif


this should be


#elif U_PLATFORM_IS_LINUX_BASED
#   if defined(__GLIBC__)
    /* __timezone is only available in glibc */
#       define U_TIMEZONE __timezone
#   endif

Comment 2

[maillist-mozilla]

or maybe, since UCLIBC pretends to be GLIBC

#elif U_PLATFORM_IS_LINUX_BASED
#   if defined(__GLIBC__) && !defined(__UCLIBC__)
    /* __timezone is only available in glibc */
#       define U_TIMEZONE __timezone
#   endif

Comment 3

[Mike Hommey [:glandium]]

Since this is a, ICU build system problem, this should be filed with them and fixed there. We're unlikely to patch this in our tree if it's not mechanically coming from a new upstream version, considering how uncommon your setup is (and unsupported).

> 1) firefox doesnt ship external libs in its own tree 
>   and if it depends on ICU it should just raise an error in the
>   configure script if its not installed

which would prevent:
- building firefox on mac and windows
- shipping firefox binaries on linux

But _you_ can build with --with-system-icu.

> 2) if it ships embedded libraries it should at least call their
> configure scripts and use them as intended

The configure script *is* used. Unfortunately, it sets U_TIMEZONE and U_HAVE_TIMEZONE and AC_SUBSTs them. Which means they are available to config.status preprocessed files, not as defines during compilation. So the #if defined(U_TIMEZONE) || defined(U_HAVE_TIMEZONE) in intl/icu/source/common/putilimp.h can never be taken.
Again, this is an ICU bug.

Comment 4

[maillist-mozilla]

(In reply to Mike Hommey [:glandium] from comment #3)
> Since this is a, ICU build system problem, this should be filed with them
> and fixed there. We're unlikely to patch this in our tree if it's not
> mechanically coming from a new upstream version, considering how uncommon
> your setup is (and unsupported).
> 
> > 1) firefox doesnt ship external libs in its own tree 
> >   and if it depends on ICU it should just raise an error in the
> >   configure script if its not installed
> 
> which would prevent:
> - building firefox on mac and windows
> - shipping firefox binaries on linux

how so ? mac user should just use his own libicu which he can install via macports, homebrew, etc.
you'd ship binaries simply with a copy of dynamic libicu you've built.

> 
> But _you_ can build with --with-system-icu.
> 
> > 2) if it ships embedded libraries it should at least call their
> > configure scripts and use them as intended
> 
> The configure script *is* used. Unfortunately, it sets U_TIMEZONE and

no it's not. i read the build log and configure is not run.
when i build libicu manually using ./configure, it builds fine.

> U_HAVE_TIMEZONE and AC_SUBSTs them. Which means they are available to
> config.status preprocessed files, not as defines during compilation. So the
> #if defined(U_TIMEZONE) || defined(U_HAVE_TIMEZONE) in
> intl/icu/source/common/putilimp.h can never be taken.

that's again wrong since above patch fixes it.

> Again, this is an ICU bug.

it's not, because you're using ICU not as intended.
anyway, gonna file this with ICU so that they fix their "too-lazy-to-run-configure" hardcoded default config.

Comment 5

[maillist-mozilla]

http://bugs.icu-project.org/trac/ticket/11045

Comment 6

[Mike Hommey [:glandium]]

> no it's not. i read the build log and configure is not run.

Read your build log again. It *is* run.
http://hg.mozilla.org/releases/mozilla-release/file/cd52a7f89548/build/autoconf/icu.m4#l313

Comment 7

[Mike Hommey [:glandium]]

But, after running configure, that's the only places where U_TIMEZONE and U_HAVE_TIMEZONE are set:
$ grep -r U_HAVE_TIMEZONE *
intl/icu/target/config.log:U_HAVE_TIMEZONE='1'
intl/icu/target/config.status:S["U_HAVE_TIMEZONE"]="1"
$ grep -r U_TIMEZONE *
intl/icu/target/config.log:U_TIMEZONE='__timezone'
intl/icu/target/config.status:S["U_TIMEZONE"]="__timezone"

In fact, if I add #error under #if defined(U_TIMEZONE) || defined(U_HAVE_TIMEZONE) in intl/icu/source/common/putilimp.h, the build doesn't fail.