Fix identification of venv interpreters.#3114
Conversation
| assert b"| I am 3.11.13 |" in subprocess.check_output( | ||
| args=[pypy3_11_13_venv_binary, cowsay, "I am 3.11.13"], | ||
| env=make_env(PEX_PYTHON_PATH=pypy3_11_13_venv_binary), | ||
| ) |
There was a problem hiding this comment.
Prior to the fix this fails with:
:; uvrc test-integration -- --devpi -vvsk test_interpreter_upgrade_same_binary_hash -n0
...
tests/integration/test_issue_3113.py::test_interpreter_upgrade_same_binary_hash Traceback (most recent call last):
File "/tmp/pytest-of-jsirois/test_interpreter_upgrade-0e5350/opt/pypy/lib/pypy3.11/runpy.py", line 201, in _run_module_as_main
return _run_code(code, main_globals, N
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/tmp/pytest-of-jsirois/test_interpreter_upgrade-0e5350/opt/pypy/lib/pypy3.11/runpy.py", line 88, in _run_code
exec(code, run_globals)
File "/tmp/pytest-of-jsirois/test_interpreter_upgrade-0e5350/pex-root/unzipped_pexes/3/8763eebecff32b475e219b8c4c1c2abc2c2a1be3/__main__.py", line 242, in <module>
result, should_exit, is_globals =
^^^^^
File "/tmp/pytest-of-jsirois/test_interpreter_upgrade-0e5350/pex-root/unzipped_pexes/3/8763eebecff32b475e219b8c4c1c2abc2c2a1be3/__main__.py", line 234, in boot
result =
^^^^^^^^^^^^^^
File "/tmp/pytest-of-jsirois/test_interpreter_upgrade-0e5350/pex-root/unzipped_pexes/3/8763eebecff32b475e219b8c4c1c2abc2c2a1be3/.bootstrap/pex/pex_bootstrapper.py", line 695, in bootstrap_pex
maybe_reexec_pex(
File "/tmp/pytest-of-jsirois/test_interpreter_upgrade-0e5350/pex-root/unzipped_pexes/3/8763eebecff32b475e219b8c4c1c2abc2c2a1be3/.bootstrap/pex/pex_bootstrapper.py", line 420, in maybe_reexec_pex
resolved = target.resolve_base_interpreter()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/tmp/pytest-of-jsirois/test_interpreter_upgrade-0e5350/pex-root/unzipped_pexes/3/8763eebecff32b475e219b8c4c1c2abc2c2a1be3/.bootstrap/pex/interpreter.py", line 1530, in resolve_base_interpreter
raise self.BaseInterpreterResolutionError("\n".join(message))
pex.interpreter.PythonInterpreter.BaseInterpreterResolutionError: Failed to resolve the base interpreter for the virtual environment at /tmp/pytest-of-jsirois/test_interpreter_upgrade-0e5350/pypy3_11_13.venv.
Search of base_prefix /tmp/pytest-of-jsirois/test_interpreter_upgrade-0e5350/opt/pypy found no equivalent interpreter for /tmp/pytest-of-jsirois/test_interpreter_upgrade-0e5350/pypy3_11_13.venv/bin/pypy3.11
FAILEDThere was a problem hiding this comment.
@cburroughs this test was pretty titchy to get right (both a single re-used install path is required as well as a limited interpreter search path), but it faithfully reproduces your original test case (besides using pre-compiled PyPy interpreters) AFAICT.
The INTERP-INFO atomic dir case is special since it contains just one file. Safe deletion in the general atomic dir case requires reader-writer file locks.
|
@cburroughs I'm going to move forward with this on green CI and get a release out. Let me know if you spot anything amiss and I'll address. |
cburroughs
left a comment
There was a problem hiding this comment.
I think I'm commenting as CI is landing, but the approach and code looks good to me. Thank you.
Regarding "partially fixes" in the CHANGES.md: It is "partial" because the solution is to try to ignore the cache as a last resort as opposed to trying to construct a cache-key that is accounts for this ahead of time. But it handles all the known cases. Do I have that right?
You do mostly have that right. It's not "But it handles all the known cases.", but instead it handles one of two known cases; namely the case of finding a venv base interpreter - which is your case. It does not cover the other case; namely, identifying the capabilities of a non venv interpreter. That case, as you suggest, would require hashing more than just the Python binary. Currently the hash for an interpreter is comprised of:
These are all fast to calculate (crucially, without actually running the Python binary) given the 1 input of the Python interpreter binary path. I'm not sure how you'd add in things like hashing linked shared libraries without getting more OS-specific than I'm comfortable handling right now. For example, I have no clue how you do this for Mac or Windows at the moment. I do know how I could do it for Linux (parts of the Pex projects already need to parse ELF for some information). And the failure mode for that case is, unlike your case, silent. The interpreter will misidentify the marker variable |
Fixes #3113