Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upFix provisioning from pyvenv interpreter #1452
Merged
Conversation
Kentzo
force-pushed the
Kentzo:fix-provisioning-from-pyvenv
branch
from
19d6e1c
to
4f1743a
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
Kentzo:fix-provisioning-from-pyvenv
branch
from
4f1743a
to
40f4860
|
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 |
This comment has been minimized.
This comment has been minimized.
|
@Kentzo could you fix the tests so this can be merged? thanks! |
This comment has been minimized.
This comment has been minimized.
|
@asottile Are you sure the failure is related to the change? I may take a look, but I'm completely unfamiliar with the codebase. |
This comment has been minimized.
This comment has been minimized.
|
I don't think the ci failure is related, but let's not merge until we fix master and rebase. Thanks! |
This comment has been minimized.
This comment has been minimized.
|
This now has been released under https://tox.readthedocs.io/en/latest/changelog.html#v3-14-1-2019-11-13 |
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.
Kentzo commentedNov 6, 2019
•
edited
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)