1455065 - Environment parsing fails in Fedora 28

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Eitan Isaacson [:eeejay]]

Attachment: Output of python/mozbuild/mozbuild/action/dump_env.py

All mach commands fail because the environment variables are incorrectly parsed. I attached the output of dump_env.py, it includes multi-line bash functions that the parser attempts to consume.

Locally, I get caught up in this line:
IFS=' ';
.. which is part of BASH_FUNC__moduleraw%%

I'm not sure why one python script needs to parse the brittle output of another python script, seems like we can use environ directly in mozconfig.py. Or maybe use a more robust serializer like json.

In the meantime, we could probably strip out the bash functions.

Comment 1

[Eitan Isaacson [:eeejay]]

Attachment: Strip out bash functions from environment in dump_env.py

Comment 3

[Mike Hommey [:glandium]]

Comment on attachment 8969009 [details] [diff] [review]
Strip out bash functions from environment in dump_env.py

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

This unfortunately only papers over the problem. Let's actually fix it.

Comment 4

[Mike Hommey [:glandium]]

Attachment: Bug 1455065 - Shell quote environment variable values when dumping them in dump_env.py.

The mozconfig output parsing code already handles shell quoted strings
to some extent (except for bug 818377), because that's what `set`
outputs. By quoting environment variable values, we avoid a bunch of
problems with "weird" values.

Review commit: https://reviewboard.mozilla.org/r/237802/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/237802/

Comment 5

[Michael Shal [:mshal]]

Comment on attachment 8969107 [details]
Bug 1455065 - Shell quote environment variable values when dumping them in dump_env.py.

https://reviewboard.mozilla.org/r/237802/#review243998

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/210ee7ebb398
Shell quote environment variable values when dumping them in dump_env.py. r=mshal

Comment 7

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/210ee7ebb398

Comment 8

[Alex Vincent [:WeirdAl]]

[Tracking Requested - why for this release]:

I'm running into problems running mach configure on Fedora 28.  I think this bug has the fix, and it would be good to backport it to Firefox 60 for the esr-60 release.

Steps to reproduce:
(1) unified-mozilla repository, hg up release
(2) cat browser/config/version.txt # returns 60.0.1
(3) export MOZCONFIG=~/mozconfigs/fx-blank.mozconfig # a zero-byte file
(4) ./mach configure

Assertion failed in _parse_loader_output:
Traceback (most recent call last):
  File "/home/ajvincent/compiled/unified/mozilla/python/mozbuild/mozbuild/mozconfig.py", line 260, in read_mozconfig
    parsed = self._parse_loader_output(output)
  File "/home/ajvincent/compiled/unified/mozilla/python/mozbuild/mozbuild/mozconfig.py", line 375, in _parse_loader_output
    assert not in_variable
AssertionError
Error loading mozconfig: /home/ajvincent/mozconfigs/fx-blank.mozconfig

This bug does NOT happen when I use |hg up beta|, where version.txt is 61.0.

Comment 9

[Julien Cristau [:jcristau]]

Doesn't feel like this needs tracking but if Mike wants to nominate this patch for ESR60 we can consider it.

Comment 10

[Alex Vincent [:WeirdAl]]

For what it's worth, I have confirmed that this patch does fix esr-60 for Fedora Linux 28.

Comment 11

[Julien Cristau [:jcristau]]

Mind requesting esr60 uplift?

Comment 12

[Mike Hommey [:glandium]]

Comment on attachment 8969107 [details]
Bug 1455065 - Shell quote environment variable values when dumping them in dump_env.py.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: Build failure due to some environment variable in the build environment on Fedora.
Fix Landed on Version: 61
Risk to taking this patch (and alternatives if risky): This only affects how the build takes values from the environment to feed build options to configure. This has no consequence on Firefox itself.
String or UUID changes made by this patch: N/A

Comment 14

[Julien Cristau [:jcristau]]

Comment on attachment 8969107 [details]
Bug 1455065 - Shell quote environment variable values when dumping them in dump_env.py.

approved for 60.1esr

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/4a61437c8f0a