948405 - Define a dummy non-ASCII environment variable to make sure that the build system is sane

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Masatoshi Kimura [:emk]]

I'm tired of the build system being broken on non-English Windows.

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: patch

Build fails without a patch from bug 944551 and succeeds with the patch:
https://tbpl.mozilla.org/?tree=Try&rev=f489d35affe8
https://tbpl.mozilla.org/?tree=Try&rev=29c5cad0c10d

Comment 2

[Masatoshi Kimura [:emk]]

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

Bug 944551 patch will land very soon.

Comment 3

[Gregory Szorc [:gps]]

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

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

::: configure.in
@@ +7676,5 @@
> +    dnl Make sure that the build system can handle non-ASCII characters
> +    dnl in environment variables to prevent it from breking silently on
> +    dnl non-English systems.
> +    NONASCII=$'\241\241'
> +    AC_SUBST(NONASCII)

Why is this under an if block? You can just put it in the root "scope."

Comment 4

[Masatoshi Kimura [:emk]]

(In reply to Gregory Szorc [:gps] from comment #3)
> > +    dnl Make sure that the build system can handle non-ASCII characters
> > +    dnl in environment variables to prevent it from breking silently on
> > +    dnl non-English systems.
> > +    NONASCII=$'\241\241'
> > +    AC_SUBST(NONASCII)
> 
> Why is this under an if block? You can just put it in the root "scope."

Because I'm not sure it works on non-Windows platforms. I intentionally chose a sequence which is invalid as UTF-8 to make sure that the build system doesn't confuse the Windows ANSI code page with UTF-8.

Comment 5

[Masatoshi Kimura [:emk]]

Also other platforms are unlikely to have non-ASCII environment variables. The offending value was always CL_INCLUDE_PREFIX. So I made the dummy envvar parallel to CL_INCLUDE_PREFIX as much as possible.

Comment 6

[Masatoshi Kimura [:emk]]

Attachment: patch v2

Hm, actually it builds on non-Window platforms: https://tbpl.mozilla.org/?tree=Try&rev=3dbde5423996

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8350870 [details] [diff] [review]
patch v2

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

Please drop a comment in config.mk before landing.

Comment 8

[Masatoshi Kimura [:emk]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/039ef62a57c0

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/039ef62a57c0