Fix provisioning from pyvenv interpreter #1452
Merged
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
Kentzo
requested review from
asottile,
gaborbernat and
obestwalter
as code owners
Kentzo
force-pushed
the
fix-provisioning-from-pyvenv
branch
from
19d6e1c to
4f1743a
Compare
When the original interpreter [0] tox is called with is actually a pyvenv it will set the `__PYVENV_LAUNCHER__` environment variable to the path of pyvenv interpreter. If tox configuration specifies `requires` and the requirements are not satifsfied by [0], tox will create an intermediate virtual environment [1]. After setting up [1] tox will delegate the original call by spawning a new [1] interpreter with the identical identical list of arguments. It will also pass the value of `__PYVENV_LAUNCHER__` if present. This in turn causes `sys.executable` to resolve to [0] instead of [1] therefore ignoring all modifications including installed requirements. The fix is to make sure that when [0] spawns [1], `__PYVENV_LAUNCHER__` is removed from the environment.
Kentzo
force-pushed
the
fix-provisioning-from-pyvenv
branch
from
4f1743a to
40f4860
Compare
asottile
approved these changes
Nov 6, 2019
There was a problem hiding this comment.
lg2m, pre-commit has the same code: https://github.com/pre-commit/pre-commit/blob/0fd4a2ea38df4f276e43397e21ecb3fcbc96d122/pre_commit/main.py#L32-L36
shakesfist framework builds
gaborbernat
approved these changes
Nov 6, 2019
|
@Kentzo could you fix the tests so this can be merged? thanks! |
|
@asottile Are you sure the failure is related to the change? I may take a look, but I'm completely unfamiliar with the codebase. |
|
I don't think the ci failure is related, but let's not merge until we fix master and rebase. Thanks! |
|
This now has been released under https://tox.readthedocs.io/en/latest/changelog.html#v3-14-1-2019-11-13 |
4 tasks
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 bug is related to #148.
When the original interpreter [0] tox is called with is actually a pyvenv
it will set the
__PYVENV_LAUNCHER__environment variable the path of pyvenvinterpreter.
If tox configuration specifies
requiresand the requirements are notsatifsfied by [0], tox will create an intermediate virtual environment [1].
After setting up [1] tox will delegate the original call to it by spawning
a new [1] interpreter with the list of arguments identical to [0].
It would also pass the value of
__PYVENV_LAUNCHER__if present. This in turncauses
sys.executableto resolve to [0] instead of [1] therefore ignoring allmodifications including installed requirements.
The fix is to make sure that when [0] spawns [1],
__PYVENV_LAUNCHER__is removedfrom the environment.
Contribution checklist:
(also see CONTRIBUTING.rst for details)
in message body
<issue number>.<type>.rstfor example (588.bugfix.rst)<type>is must be one ofbugfix,feature,deprecation,breaking,doc,misc<your username>"superuser."CONTRIBUTORS(preserving alphabetical order)