black python linter is unhappy with virtualenvs for some reason
Categories
(Developer Infrastructure :: Lint and Formatting, defect, P3)
Tracking
(firefox84 fixed)
Tracking | Status | |
---|---|---|
firefox84 | --- | fixed |
People
(Reporter: Gijs, Assigned: andi)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
$ ./mach lint -l black tools/lint/hooks.py --fix
Traceback (most recent call last):
File "path/to/mozilla-central/python/mozlint/mozlint/roller.py", line 60, in _run_worker
res = func(paths, config, **lintargs) or []
File "path/to/mozilla-central/python/mozlint/mozlint/types.py", line 54, in __call__
return self._lint(paths, config, **lintargs)
File "path/to/mozilla-central/python/mozlint/mozlint/types.py", line 143, in _lint
return func(files, config, **lintargs)
File "path/to/mozilla-central/tools/lint/python/black.py", line 129, in lint
return run_black(config, files, fix=fix, log=lintargs["log"])
File "path/to/mozilla-central/tools/lint/python/black.py", line 116, in run_black
log.debug("Black version {}".format(get_black_version(binary)))
File "path/to/mozilla-central/tools/lint/python/black.py", line 43, in get_black_version
output = subprocess.check_output(
File "/usr/local/Cellar/python@3.8/3.8.5/Frameworks/Python.framework/Versions/3.8/lib/python3.8/subprocess.py", line 411, in check_output
return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
File "/usr/local/Cellar/python@3.8/3.8.5/Frameworks/Python.framework/Versions/3.8/lib/python3.8/subprocess.py", line 489, in run
with Popen(*popenargs, **kwargs) as process:
File "/usr/local/Cellar/python@3.8/3.8.5/Frameworks/Python.framework/Versions/3.8/lib/python3.8/subprocess.py", line 854, in __init__
self._execute_child(args, executable, preexec_fn, close_fds,
File "/usr/local/Cellar/python@3.8/3.8.5/Frameworks/Python.framework/Versions/3.8/lib/python3.8/subprocess.py", line 1702, in _execute_child
raise child_exception_type(errno_num, err_msg, err_filename)
FileNotFoundError: [Errno 2] No such file or directory: '$HOME/.mozbuild/_virtualenvs/mach/bin/../bin/black'
A failure occurred in the black linter.
✖ 1 problem (0 errors, 0 warnings, 1 failure)
Reporter | ||
Updated•4 years ago
|
Comment 1•4 years ago
|
||
The severity field is not set for this bug.
:ahal, could you have a look please?
For more information, please visit auto_nag documentation.
Comment 2•4 years ago
|
||
Fwiw, I'm unable to reproduce on Linux (WSL). Perhaps it's a MacOS issue? I noticed a couple black related patches landing recently, so there's a chance it got fixed since this was filed. Are you still able to reproduce?
I also CC'ed Sylvestre and Ricky who may have more context.
Comment 3•4 years ago
|
||
Ah, I saw something like this in CI, and I fixed it in CI with https://phabricator.services.mozilla.com/D87904.
A LOT of changes have happened in virtualenv
s over the past 19 days, so first of all I would check whether you're still encountering the issue.
If it is, it's because the lint
codebase uses pip
to install black
in the virtualenv
you're using, but if pip
determines that black
is already installed, then pip
won't do anything. Then the lint
codebase attempts to locate black
in the virtualenv
directory -- but if pip
didn't install it, it won't be there.
The "correct" solution to the problem would be to call into black
with python -m black
which is guaranteed to work in that case, rather than attempting to locate a binary that may not exist.
Reporter | ||
Comment 4•4 years ago
|
||
I still see this on central rev d4e11195e398
, from yesterday.
However, running pip3 uninstall black
and then re-running the command yields the same error.
Is there anything else I can do/check to help here?
Comment 5•4 years ago
|
||
Does this patch fix it?
diff --git a/tools/lint/python/black.py b/tools/lint/python/black.py
index 7a78f5b44b77..0712052a924a 100644
--- a/tools/lint/python/black.py
+++ b/tools/lint/python/black.py
@@ -35,20 +35,21 @@ else:
bindir = os.path.join(sys.prefix, "bin")
-def get_black_version(binary):
+def get_black_version(cmd):
"""
- Returns found binary's version
+ Returns the version of black. cmd is a list of the form
+ ["python", "-m", "black"].
"""
try:
output = subprocess.check_output(
- [binary, "--version"],
+ cmd + ["--version"],
stderr=subprocess.STDOUT,
universal_newlines=True,
)
except subprocess.CalledProcessError as e:
output = e.output
- return re.match(r"black, version (.*)$", output)[1]
+ return re.search(r", version (.*)$", output)[1]
def parse_issues(config, output, paths, *, log):
@@ -113,11 +114,11 @@ def setup(root, **lintargs):
def run_black(config, paths, fix=None, *, log):
- binary = os.path.join(bindir, "black")
+ cmd = [os.path.join(bindir, "python"), "-m", "black"]
- log.debug("Black version {}".format(get_black_version(binary)))
+ log.debug("Black version {}".format(get_black_version(cmd)))
- cmd_args = [binary]
+ cmd_args = list(cmd)
if not fix:
cmd_args.append("--check")
base_command = cmd_args + paths
Reporter | ||
Comment 6•4 years ago
|
||
Apologies for the delay. With that I get:
Traceback (most recent call last):
File "python/mozlint/mozlint/roller.py", line 60, in _run_worker
res = func(paths, config, **lintargs) or []
File "python/mozlint/mozlint/types.py", line 54, in __call__
return self._lint(paths, config, **lintargs)
File "python/mozlint/mozlint/types.py", line 143, in _lint
return func(files, config, **lintargs)
File "tools/lint/python/black.py", line 132, in lint
return run_black(config, files, fix=fix, log=lintargs["log"])
File "tools/lint/python/black.py", line 119, in run_black
log.debug("Black version {}".format(get_black_version(cmd)))
File "tools/lint/python/black.py", line 52, in get_black_version
return re.search(r", version (.*)$", output)[1]
TypeError: 'NoneType' object is not subscriptable
If I modify the end of get_black_version
to:
versionmatch = re.search(r", version (.*)$", output)
return versionmatch[1] if versionmatch else None
then it no longer errors out, ie I get:
✖ 0 problems (0 errors, 0 warnings)
though tbh I don't understand how it would then work... and a naive attempt to introduce issues into a python file (really long line) doesn't seem to be able to trigger actual issues...
Reporter | ||
Comment 7•4 years ago
|
||
If I print e.output
in the catch clause I still get:
$HOME/.mozbuild/_virtualenvs/mach/bin/../bin/python: No module named black
Comment 8•4 years ago
|
||
I'm confused about why the virtualenv
it's hitting is the in-~/.mozbuild
one. It should be using the $objdir/_virtualenvs/init_py3
virtualenv
. Is there anything funny about your environment I should be aware of that would be causing this?
Comment 9•4 years ago
|
||
If it wasn't clear: 1. I can't reproduce that at HEAD, and I've verified that running mach lint
on my machine uses the in-objdir
virtualenv
as I mentioned. and 2. This is the cause of the issue, so if we determine why your mach lint
is doing this weird thing, that'll fix the problem.
Comment 10•4 years ago
|
||
Assigning to Andi after a conversation on chat.m.o.
We've isolated the issue to how mach lint
subprocesses into sub-jobs. We use a ProcessPoolExecutor which may start new processes that end up loading black.py
multiple times. When they do, the variable binpath
will be instantiated in those subprocesses to an incorrect value (as the changes that activate_this.py
makes to sys.prefix
don't persist in subprocesses). This results in the behavior we see here, namely, trying to open the black
binary at the incorrect path.
Comment 11•4 years ago
|
||
If you're looking for a good place to pass down a "virtualenv_path" or similar key, I'd do it here:
https://searchfox.org/mozilla-central/source/tools/lint/mach_commands.py#83
This way it stays out of mozlint core, and linters can find it in the lintargs
dict. There's precedent too as we set other build related configuration (maybe the virtualenv can even be derived from the build config we already pass in).
Comment 12•4 years ago
|
||
I pulled today, and this issue broke my lint command. I'm on macOS.
Comment 13•4 years ago
|
||
If anyone needs a workaround, this worked on my machine:
~/.mozbuild/_virtualenvs/mach/bin/pip install -r tools/lint/python/black_requirements.txt
~/.mozbuild/_virtualenvs/mach/bin/pip install -r tools/lint/python/flake8_requirements.txt
Comment 15•4 years ago
|
||
The logic the black
and flake8
linters were using to find the location of the appropriate binaries for linting was wrong in certain cases given how mach lint
uses subprocesses to batch work. Instead, we allow the option to override the old janky behavior with a known-good path.
Comment 16•4 years ago
|
||
Comment 17•4 years ago
|
||
bugherder |
Updated•2 years ago
|
Description
•