fix: move coverage pkg to end of sys.path to avoid collisions#1045
fix: move coverage pkg to end of sys.path to avoid collisions#1045
Conversation
Signed-off-by: Thulio Ferraz Assis <[email protected]>
Signed-off-by: Thulio Ferraz Assis <[email protected]>
f0rmiga
changed the title
| This is installed as the script entry point. | ||
|
|
||
| """ | ||
| + sys.path.append(sys.path.pop(0)) |
There was a problem hiding this comment.
How does this affect the template that launches the coverage?
I am mainly thinking about this line here: https://github.com/bazelbuild/bazel/blob/0696ba32a789bbf3100f62fa2c1547fc74e36006/tools/python/python_bootstrap_template.txt#L502
There was a problem hiding this comment.
I was not aware of that. @rickeylev are you familiar with that part of the code?
There was a problem hiding this comment.
I'm not familiar with that code in the bootstrap. Looking through blame, it was added in 2020 as part of the initial work to get coverage working. Does coverage still rely on that behavior? It sounds brittle and the sort of thing that would get fixed in the intervening years...
CI was green with this, right? Presubmits are setup to run coverage commands last I saw.
Looking at the code paths -- I'm no expert here -- I think it ends up working out, because I think what happens is:
Python starts up, puts . at the start of path.
Coverage runs, enters main(), runs this patch, which moves . to the end of sys.path
Shortly thereafter, coverage enters CoverageScript.command_line, which does sys.path.insert(0, ''). It says it does this not for itself, but so "plugins can e.g. read django settings". So it doesn't sound related to coverage being able to work in general.
I didn't see the part where coverage undoes the sys.path.insert() call, but I didn't look through all its code. I'll assume its true.
The last step there makes me thing the comment in the bootstrap is outdated or misleading, as it sounds like the current directory being sys.path[0] isn't necessary for coverage itself.
There was a problem hiding this comment.
There was a problem hiding this comment.
The coverage run command has always adjusted the first entry in sys.path, to properly emulate how Python runs your program. Now this adjustment is skipped if sys.path[0] is already different than Python's default. This fixes coveragepy/coveragepy#715.
4 participants
When we run tests for the locked requirements files, pip-compile will import pip internally, and eventually, it tries to import
html.entries, which is part of the standard library. Since thecoveragepackage provides anhtml.pyfile, it ends up being picked up by pip if thecoveragepackage is early on thesys.path.This patch presents a generic fix for the problem with coverage by moving its own directory to the end of
sys.path.