Absolutize all of the execute_pex_args in the venv script. #12727
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
[ci skip-rust] [ci skip-build-wheels]
stuhood
force-pushed
the
stuhood/absolutize-all-of-argv
branch
from
77f972c
to
694952f
Compare
Eric-Arellano
approved these changes
Sep 1, 2021
jsirois
approved these changes
Sep 1, 2021
| @@ -943,6 +944,7 @@ async def create_venv_pex( | |||
| pex=pex.script, | |||
| python=python.script, | |||
| bin=FrozenDict((bin_name, venv_script.script) for bin_name, venv_script in scripts.items()), | |||
| venv_rel_dir=venv_rel_dir.as_posix(), | |||
There was a problem hiding this comment.
Folks have a habit of doing this, but str(venv_rel_dir) will be Windows-proof and is actually correct today too.
| @@ -852,6 +852,7 @@ class VenvPex: | |||
| pex: Script | |||
| python: Script | |||
| bin: FrozenDict[str, Script] | |||
| venv_rel_dir: str | |||
There was a problem hiding this comment.
It's probably good to tack on ~ # for tests to be friendly to future refactorers.
stuhood
added a commit
to stuhood/pants
that referenced
this pull request
Sep 1, 2021
…d#12727) The absolutizing of `execute_pex_args` was being applied to the entire list of arguments, rather than to individual arguments. When: 1. using a working directory 2. hitting the `Process` cache for VenvPex creation/seeding and missing for execution 3. missing the PEX venv cache ... the relevant `execute_pex_args` code would run to seed the venv, and fail with: ``` pants.engine.process.ProcessExecutionFailure: Process 'Run the pex and check its cwd' failed with exit code 2. stdout: stderr: <snip>/python3.6: can't open file 'test.pex': [Errno 2] No such file or directory ``` ...because the PEX filename was not absolutized. Add a reproducing test, and fix by absolutizing each argument. [ci skip-rust] [ci skip-build-wheels]
stuhood
added a commit
that referenced
this pull request
Sep 1, 2021
…k of #12727) (#12729) The absolutizing of `execute_pex_args` was being applied to the entire list of arguments, rather than to individual arguments. When: 1. using a working directory 2. hitting the `Process` cache for VenvPex creation/seeding and missing for execution 3. missing the PEX venv cache ... the relevant `execute_pex_args` code would run to seed the venv, and fail with: ``` pants.engine.process.ProcessExecutionFailure: Process 'Run the pex and check its cwd' failed with exit code 2. stdout: stderr: <snip>/python3.6: can't open file 'test.pex': [Errno 2] No such file or directory ``` ...because the PEX filename was not absolutized. Add a reproducing test, and fix by absolutizing each argument. [ci skip-rust] [ci skip-build-wheels]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The absolutizing of
execute_pex_argswas being applied to the entire list of arguments, rather than to individual arguments. When:Processcache for VenvPex creation/seeding and missing for execution... the relevant
execute_pex_argscode would run to seed the venv, and fail with:...because the PEX filename was not absolutized.
Add a reproducing test, and fix by absolutizing each argument.
[ci skip-rust]
[ci skip-build-wheels]