Closed Bug 948405 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox Build System :: General, defect)

x86_64
Windows 8.1
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla29

People

(Reporter: emk, Assigned: emk)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

I'm tired of the build system being broken on non-English Windows.
Attached patch patch (obsolete) — Splinter Review
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 on attachment 8347689 [details] [diff] [review]
patch

Bug 944551 patch will land very soon.
Attachment #8347689 - Flags: review?(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."
Attachment #8347689 - Flags: review?(gps) → feedback+
(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.
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.
Attached patch patch v2Splinter Review
Hm, actually it builds on non-Window platforms: https://tbpl.mozilla.org/?tree=Try&rev=3dbde5423996
Attachment #8347689 - Attachment is obsolete: true
Attachment #8350870 - Flags: review?(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.
Attachment #8350870 - Flags: review?(gps) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/039ef62a57c0
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/039ef62a57c0
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Whiteboard: [qa-]
Depends on: 1008851
Depends on: 1050561
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.