1578198 - Mach clobber broken on Windows - "TypeError: environment can only contain strings"

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, defect, P1, blocker)

Description

[:Gijs (he/him)]

Log:

$ ./mach help busted
Error running mach:

    ['help', 'busted']

The error occurred in mach itself. This is likely a bug in mach itself or a
fundamental problem with a loaded module.

You can invoke |./mach busted| to check if this issue is already on file. If it
isn't, please use |./mach busted file| to report it. If |./mach busted| is
misbehaving, you can also inspect the dependencies of bug 1543241.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

TypeError: environment can only contain strings

  File "path\to\m-c\python/mach\mach\main.py", line 360, in run
    return self._run(argv)
  File "path\to\m-c\python/mach\mach\main.py", line 404, in _run
    Registrar.register_conditional_names(context)
  File "path\to\m-c\python/mach\mach\registrar.py", line 63, in register_conditional_names
    instance = MachRegistrar._instance(handler, context)
  File "path\to\m-c\python/mach\mach\registrar.py", line 101, in _instance
    instance = cls(context)
  File "path\to\m-c\python/mozbuild\mozbuild\base.py", line 895, in __init__
    self.mozconfig
  File "path\to\m-c\python/mozbuild\mozbuild\base.py", line 324, in mozconfig
    return self.mozconfig_and_target[0]
  File "path\to\m-c\python/mozbuild\mozbuild\base.py", line 316, in mozconfig_and_target
    self.topsrcdir, self._mozconfig, os.environ.get('MOZCONFIG'))
  File "path\to\m-c\python/mozbuild\mozbuild\util.py", line 954, in __call__
    self[args] = self.func(*args)
  File "path\to\m-c\python/mozbuild\mozbuild\base.py", line 307, in get_mozconfig_and_target
    sandbox._value_for(sandbox['real_target']),
  File "path\to\m-c\python/mozbuild\mozbuild\configure\__init__.py", line 539, in _value_for
    return self._value_for_depends(self._depends[obj])
  File "path\to\m-c\python/mozbuild\mozbuild\util.py", line 963, in method_call
    cache[args] = self.func(instance, *args)
  File "path\to\m-c\python/mozbuild\mozbuild\configure\__init__.py", line 551, in _value_for_depends
    value = obj.result()
  File "path\to\m-c\python/mozbuild\mozbuild\util.py", line 963, in method_call
    cache[args] = self.func(instance, *args)
  File "path\to\m-c\python/mozbuild\mozbuild\configure\__init__.py", line 156, in result
    for d in self.dependencies]
  File "path\to\m-c\python/mozbuild\mozbuild\configure\__init__.py", line 156, in <listcomp>
    for d in self.dependencies]
  File "path\to\m-c\python/mozbuild\mozbuild\configure\__init__.py", line 542, in _value_for
    return self._value_for_depends(obj)
  File "path\to\m-c\python/mozbuild\mozbuild\util.py", line 963, in method_call
    cache[args] = self.func(instance, *args)
  File "path\to\m-c\python/mozbuild\mozbuild\configure\__init__.py", line 551, in _value_for_depends
    value = obj.result()
  File "path\to\m-c\python/mozbuild\mozbuild\util.py", line 963, in method_call
    cache[args] = self.func(instance, *args)
  File "path\to\m-c\python/mozbuild\mozbuild\configure\__init__.py", line 157, in result
    return self._func(*resolved_args)
  File "path\to\m-c\python/mozbuild\mozbuild\configure\__init__.py", line 1149, in wrapped
    return new_func(*args, **kwargs)
  File "path\to\m-c\python/mozbuild\mozbuild\configure\__init__.py", line 804, in wrapper
    ret = template(*args, **kwargs)
  File "path/to/m-c/build/moz.configure/checks.configure", line 58, in wrapped
    ret = func(*args, **kwargs)
  File "path\to\m-c\python/mozbuild\mozbuild\configure\__init__.py", line 1149, in wrapped
    return new_func(*args, **kwargs)
  File "path/to/m-c/build/moz.configure/init.configure", line 800, in real_host
    host = config_sub(shell, host)
  File "path\to\m-c\python/mozbuild\mozbuild\configure\__init__.py", line 1149, in wrapped
    return new_func(*args, **kwargs)
  File "path/to/m-c/build/moz.configure/init.configure", line 772, in config_sub
    return check_cmd_output(shell, config_sub, triplet).strip()
  File "path\to\m-c\python/mozbuild\mozbuild\configure\__init__.py", line 1149, in wrapped
    return new_func(*args, **kwargs)
  File "path/to/m-c/build/moz.configure/util.configure", line 55, in check_cmd_output
    retcode, stdout, stderr = get_cmd_output(*args, **kwargs)
  File "path\to\m-c\python/mozbuild\mozbuild\configure\__init__.py", line 1149, in wrapped
    return new_func(*args, **kwargs)
  File "path/to/m-c/build/moz.configure/util.configure", line 38, in get_cmd_output
    **kwargs)
  File "path\to\m-c\python/mozbuild\mozbuild\configure\__init__.py", line 899, in wrapper
    return function(*args, **kwargs)
  File "path\to\mozilla-build\python3\lib\subprocess.py", line 709, in __init__
    restore_signals, start_new_session)
  File "path\to\mozilla-build\python3\lib\subprocess.py", line 997, in _execute_child
    startupinfo)

The same error is shown for trying to run ./mach clobber

Comment 1

[:Gijs (he/him)]

m-c rev https://hg.mozilla.org/mozilla-central/rev/db860d4b1007 in case that matters.

Comment 2

[:Gijs (he/him)]

This seems to have broken somewhere in https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8d251dd4a9bebaa3e72a23d795122d4b12097ffc&tochange=cae93ef1993e02a136ef64d974856071b905997f so beta is also affected.

Comment 3

[:Gijs (he/him)]

mach clobber broke in bug 1577603, but it seems mach help busted was broken for longer.

Comment 4

[:Gijs (he/him)]

./mach help broke in bug 1473498

Comment 5

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #4)

./mach help broke in bug 1473498

Specifically, commit https://hg.mozilla.org/mozilla-central/rev/d5ef7a5e02a4

Comment 6

[:Gijs (he/him)]

This seems to be a result (at least in part?) of the "normalization" in https://hg.mozilla.org/mozilla-central/rev/f47e966507ebd6c0925ab9a163291f25e627f3c2 at https://searchfox.org/mozilla-central/rev/9415ecf29ba1acbef9381335e0ecde5916ca4073/python/mozbuild/mozbuild/configure/__init__.py#888 . This makes all the strings in the dictionary binary strings and that makes subprocess "upset". If I just comment out all this encoding, the problem goes away. Maybe glandium can comment if/why this is necessary on Windows and/or how to do the encoding and yet not upset the subprocess code?

Comment 7

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #6)

This seems to be a result (at least in part?) of the "normalization" in https://hg.mozilla.org/mozilla-central/rev/f47e966507ebd6c0925ab9a163291f25e627f3c2 at https://searchfox.org/mozilla-central/rev/9415ecf29ba1acbef9381335e0ecde5916ca4073/python/mozbuild/mozbuild/configure/__init__.py#888 . This makes all the strings in the dictionary binary strings and that makes subprocess "upset". If I just comment out all this encoding, the problem goes away. Maybe glandium can comment if/why this is necessary on Windows and/or how to do the encoding and yet not upset the subprocess code?

... though commenting this out means ./mach build gets upset., while mach clobber and mach help work...

Comment 8

[Andrew Halberstadt [:ahal]]

Today is a holiday so won't be able to do much deep digging until tomorrow, but I can clarify a few things...

The help issue is bug 1577501. There's a patch in there that will fix it pending glandium's review. Feel free to land on my behalf if there aren't any issues with it.

The clobber issue is indeed bug 1577603. We need to be more strict about testing these commands on Windows with non-ascii environment variables moving forward. Ideally with unittests, but at the very least with good local testing. I'll make sure to bring this up at the next py3 meeting on Friday.

So let's make this bug solely about the clobber issue.

Comment 9

[Mark Banner (:standard8)]

In bug 1577826, we've think we've just fixed an issue with ESLint by changing:

- result = call_process(package_name, cmd, append_env={'PATH': os.pathsep.join(path)})
+ result = call_process(package_name, cmd, append_env={str('PATH'): str(os.pathsep.join(path))})

Giving Gijs mentioned the environment above, not sure if something like that might help here.

Comment 10

[Jorg K (GMT+2)]

mach clang-format is also broken, see bug 1577826 comment #16.

Comment 11

[Andrew Halberstadt [:ahal]]

It doesn't look like there's anything wrong with the patch in bug 1577603 per se. Rather, all invocations of mach with Python 3 are currently broken for Gijs, and bug 1577603 simply converted clobber to run with Python 3.

(In reply to :Gijs (he/him) from comment #7)

... though commenting this out means ./mach build gets upset., while mach clobber and mach help work...

To shed a bit of light here, ./mach help and ./mach clobber both run with Python 3. Almost everything else (including mach build) run with Python 2.

Putting that normalization behind an if sys.version_info < (3,) guard, and enforce text otherwise should do the trick. Similar to here:
https://searchfox.org/mozilla-central/source/python/mach/mach/util.py#11

Though I'll defer to :glandium on whether or not that's the proper fix here.

Comment 12

[Jorg K (GMT+2)]

Turned out that mach typo also runs Python 3.

Comment 13

[Andrew Halberstadt [:ahal]]

(In reply to Jorg K (GMT+2) from comment #12)

Turned out that mach typo also runs Python 3.

This should be fixed on central now:
https://bugzilla.mozilla.org/show_bug.cgi?id=1577908

Comment 14

[Andrew Halberstadt [:ahal]]

I booted up Windows 10 but can't reproduce this. I tried:

1. Adding unicode to an environment variable
2. Changing system locale to Japanese
3. Changing display language to Japanese

In all cases both mach help and mach clobber appear to work as expected.

Comment 15

[:Gijs (he/him)]

(In reply to Andrew Halberstadt [:ahal] from comment #14)

I booted up Windows 10 but can't reproduce this. I tried:

1. Adding unicode to an environment variable
2. Changing system locale to Japanese
3. Changing display language to Japanese

In all cases both mach help and mach clobber appear to work as expected.

Is there anything I can provide about my environment that would help in terms of reproducing? FWIW, my Windows is all in English, and AFAICT there are no unicode characters involved in any of the paths.

Comment 16

[Andrew Halberstadt [:ahal]]

Sorry, I have no idea. Glandium might have a better idea of how Windows handles locales and encodings and stuff.

But the end of comment 11 describes a solution that I think will work. I'll put up a quick patch and you can see if it fixes things and :glandium can decide if it's acceptable or not.

Comment 17

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1578198 - [configure] Ensure wrapped_subprocess uses text env with Python 3

Comment 18

[Andrew Halberstadt [:ahal]]

Bit of a shot in the dark, but let me know if this helps any (you may see a different error after this).

Comment 19

[:Gijs (he/him)]

I can confirm that the patch fixes both ./mach help and ./mach clobber for me (without the patch for bug 1577501 applied).

Comment 20

[:kip (Kearwood Gilbert) PTO Until 10/14]

Fix worked for me also, thanks!

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9fe5cd7003d5
[configure] Ensure wrapped_subprocess uses text env with Python 3 r=glandium

Comment 22

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/9fe5cd7003d5

Comment 23

[Liz Henry (:lizzard)]

Do you want to request uplift to beta?

Comment 24

[Andrew Halberstadt [:ahal]]

Yes, looks like the regressing patch landed there too. Thanks!

[Tracking Requested - why for this release]:
Fixes regression that landed on beta.

Comment 25

[Andrew Halberstadt [:ahal]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4963590b74c1295112c551a482ab210ac334b7c5