1637305 - ./mach try syntax throws TypeError on Windows

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P1)

Description

[Andrew Halberstadt [:ahal]]

From https://bugzilla.mozilla.org/show_bug.cgi?id=1637271#c2

$ mach try syntax -b do -p win32,win64,win32-mingwclang,win64-mingwclang,linux32,linux64 -u cppunit,mochitest -t none
['D:\\mozilla-build\\/msys/bin/sh.exe', 'u:/mz/mozilla-central/python/mozbuild/mozbuild/mozconfig_loader', 'u:/mz/mozilla-central', 'U:/mz/mozconfig', 'd:\\mozilla-build\\python3\\python3.exe', 'u:/mz/mozilla-central/python/mozbuild/mozbuild/action/dump_env.py']
('d:\\mozilla-build\\python\\Scripts\\hg.EXE', b'status', b'--modified', b'--added', b'--removed', b'--deleted')
Error running mach:

    ['try', 'syntax', '-b', 'do', '-p', 'win32,win64,win32-mingwclang,win64-mingwclang,linux32,linux64', '-u', 'cppunit,mochitest', '-t', 'none']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.
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: a bytes-like object is required, not 'str'

  File "u:\mz\mozilla-central\tools/tryselect/mach_commands.py", line 417, in try_syntax
    return self.run(**kwargs)
  File "u:\mz\mozilla-central\tools/tryselect/mach_commands.py", line 178, in run
    return mod.run(**kwargs)
  File "u:\mz\mozilla-central\tools\tryselect\selectors\syntax.py", line 614, in run
    return at.run(**kwargs)
  File "u:\mz\mozilla-central\tools\tryselect\selectors\syntax.py", line 609, in run
    closed_tree=kwargs["closed_tree"])
  File "u:\mz\mozilla-central\tools\tryselect\push.py", line 144, in push_to_try
    check_working_directory(push)
  File "u:\mz\mozilla-central\tools\tryselect\push.py", line 83, in check_working_directory
    if not vcs.working_directory_clean():
  File "u:\mz\mozilla-central\python/mozversioncontrol\mozversioncontrol\__init__.py", line 412, in working_directory_clean
    return not len(self._run(*args).strip())
  File "u:\mz\mozilla-central\python/mozversioncontrol\mozversioncontrol\__init__.py", line 318, in _run
    return super(HgRepository, self)._run(*args, **runargs)
  File "u:\mz\mozilla-central\python/mozversioncontrol\mozversioncontrol\__init__.py", line 115, in _run
    universal_newlines=True)
  File "d:\mozilla-build\python3\lib\subprocess.py", line 395, in check_output
    **kwargs).stdout
  File "d:\mozilla-build\python3\lib\subprocess.py", line 475, in run
    with Popen(*popenargs, **kwargs) as process:
  File "d:\mozilla-build\python3\lib\subprocess.py", line 778, in __init__
    restore_signals, start_new_session)
  File "d:\mozilla-build\python3\lib\subprocess.py", line 1122, in _execute_child
    args = list2cmdline(args)
  File "d:\mozilla-build\python3\lib\subprocess.py", line 533, in list2cmdline
    needquote = (" " in arg) or ("\t" in arg) or not arg

I can't reproduce on WSL, so assuming this is Windows only.

Comment 1

[Andrew Halberstadt [:ahal]]

This is a bug in mozversioncontrol. For some reason the working_directory_clean function explicitly uses byte-strings as its argument:
https://searchfox.org/mozilla-central/source/python/mozversioncontrol/mozversioncontrol/__init__.py#402

Looks like it's always been like this and we can likely just use text. Notably, the git implementation uses text, only the hg implementation uses bytes.

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1637305 - [mozversioncontrol] Don't use bytes in HGRepository.working_directory_clean, r?rstewart

Comment 3

[Andrew Halberstadt [:ahal]]

:toshi, does the attached patch fix the issue for you? I'm pretty confident it will, but I wasn't able to reproduce so would like to verify.

Comment 5

[Toshihito Kikuchi [:toshi]]

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

:toshi, does the attached patch fix the issue for you? I'm pretty confident it will, but I wasn't able to reproduce so would like to verify.

Thank you looking into this quickly! With D74880, I hit a different error as below.

$ mach try syntax -b do -p win32,win64,win32-mingwclang,win64-mingwclang,linux32,linux64 -u cppunit,mochitest -t none
['D:\\mozilla-build\\/msys/bin/sh.exe', 'u:/mz/mozilla-central/python/mozbuild/mozbuild/mozconfig_loader', 'u:/mz/mozilla-central', 'U:/mz/mozconfig', 'd:\\mozilla-build\\python3\\python3.exe', 'u:/mz/mozilla-central/python/mozbuild/mozbuild/action/dump_env.py']
('d:\\mozilla-build\\python\\Scripts\\hg.EXE', 'status', '--modified', '--added', '--removed', '--deleted')
('d:\\mozilla-build\\python\\Scripts\\hg.EXE', 'revert', '-a')
Error running mach:

    ['try', 'syntax', '-b', 'do', '-p', 'win32,win64,win32-mingwclang,win64-mingwclang,linux32,linux64', '-u', 'cppunit,mochitest', '-t', 'none']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.
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 "u:\mz\mozilla-central\tools/tryselect/mach_commands.py", line 417, in try_syntax
    return self.run(**kwargs)
  File "u:\mz\mozilla-central\tools/tryselect/mach_commands.py", line 178, in run
    return mod.run(**kwargs)
  File "u:\mz\mozilla-central\tools\tryselect\selectors\syntax.py", line 614, in run
    return at.run(**kwargs)
  File "u:\mz\mozilla-central\tools\tryselect\selectors\syntax.py", line 609, in run
    closed_tree=kwargs["closed_tree"])
  File "u:\mz\mozilla-central\tools\tryselect\push.py", line 182, in push_to_try
    vcs.push_to_try(commit_message)
  File "u:\mz\mozilla-central\python/mozversioncontrol\mozversioncontrol\__init__.py", line 427, in push_to_try
    env=self._env)
  File "d:\mozilla-build\python3\lib\subprocess.py", line 342, in check_call
    retcode = call(*popenargs, **kwargs)
  File "d:\mozilla-build\python3\lib\subprocess.py", line 323, in call
    with Popen(*popenargs, **kwargs) as p:
  File "d:\mozilla-build\python3\lib\subprocess.py", line 778, in __init__
    restore_signals, start_new_session)
  File "d:\mozilla-build\python3\lib\subprocess.py", line 1181, in _execute_child
    startupinfo)

Comment 6

[Ricky Stewart]

On that line of __init__.py you would need to call ensure_subprocess_env(self._env) to make sure the env only contains the correct types. I can throw a quick patch in for that.

Comment 7

[Toshihito Kikuchi [:toshi]]

(In reply to Ricky Stewart from comment #6)

On that line of __init__.py you would need to call ensure_subprocess_env(self._env) to make sure the env only contains the correct types. I can throw a quick patch in for that.

Sweet, I got 'mach try' back!! Here's a patch I applied: https://hg.mozilla.org/try/rev/bb4571de37a2b28d6da85e15482680428d223d6e.

Comment 8

[Ricky Stewart]

(The patch in question for reference: https://phabricator.services.mozilla.com/D74888)

Comment 9

[Honza Bambas (:mayhemer)]

(In reply to Toshihito Kikuchi [:toshi] from comment #7)

(In reply to Ricky Stewart from comment #6)

On that line of __init__.py you would need to call ensure_subprocess_env(self._env) to make sure the env only contains the correct types. I can throw a quick patch in for that.

Sweet, I got 'mach try' back!! Here's a patch I applied: https://hg.mozilla.org/try/rev/bb4571de37a2b28d6da85e15482680428d223d6e.

Confirming a fix for me too!

Comment 10

[Sid Stamm [:geekboy or :sstamm]]

(In reply to Honza Bambas (:mayhemer) from comment #9)

(In reply to Toshihito Kikuchi [:toshi] from comment #7)

Sweet, I got 'mach try' back!! Here's a patch I applied: https://hg.mozilla.org/try/rev/bb4571de37a2b28d6da85e15482680428d223d6e.

Confirming a fix for me too!

Me too! Fixes the problems I reported in duplicate bug 1637261. Thanks!

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/998362228c1f
[mozversioncontrol] Don't use bytes in HGRepository.working_directory_clean, r=rstewart

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/78ca58d04a9b
[mozversioncontrol] Don't use bytes in HGRepository.working_directory_clean, r=rstewart a=try-push-fix

Comment 13

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/78ca58d04a9b

Comment 14

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/998362228c1f

Comment 15

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1632429