fix: take virtualenvs.prefer-active-python into account on EnvManager.get()#6986
Merged
neersighted merged 2 commits intopython-poetry:masterfrom Nov 22, 2022
Merged
Conversation
finswimmer
force-pushed
the
fix/env-info-activated-python
branch
from
efab161 to
a42859d
Compare
finswimmer
changed the title
src/poetry/utils/env.py
Outdated
Comment on lines
559
to
577
| def _get_python_version(self) -> list[int]: | ||
| version_info = list(sys.version_info[:3]) | ||
|
|
||
| if self._poetry.config.get("virtualenvs.prefer-active-python"): | ||
| executable = self._detect_active_python() | ||
|
|
||
| if executable: | ||
| python_patch = decode( | ||
| subprocess.check_output( | ||
| list_to_shell_command( | ||
| [executable, "-c", GET_PYTHON_VERSION_ONELINER] | ||
| ), | ||
| shell=True, | ||
| ).strip() | ||
| ) | ||
|
|
||
| version_info = [int(v) for v in python_patch.split(".")[:3]] | ||
|
|
||
| return version_info |
Member
There was a problem hiding this comment.
Suggested change
| def _get_python_version(self) -> list[int]: | |
| version_info = list(sys.version_info[:3]) | |
| if self._poetry.config.get("virtualenvs.prefer-active-python"): | |
| executable = self._detect_active_python() | |
| if executable: | |
| python_patch = decode( | |
| subprocess.check_output( | |
| list_to_shell_command( | |
| [executable, "-c", GET_PYTHON_VERSION_ONELINER] | |
| ), | |
| shell=True, | |
| ).strip() | |
| ) | |
| version_info = [int(v) for v in python_patch.split(".")[:3]] | |
| return version_info | |
| def _get_python_version(self) -> tuple[int, int, int]: | |
| if self._poetry.config.get("virtualenvs.prefer-active-python"): | |
| executable = self._detect_active_python() | |
| if executable: | |
| python_patch = decode( | |
| subprocess.check_output( | |
| list_to_shell_command( | |
| [executable, "-c", GET_PYTHON_VERSION_ONELINER] | |
| ), | |
| shell=True, | |
| ).strip() | |
| ) | |
| return tuple(int(v) for v in python_patch.split(".")[:3]) | |
| return sys.version_info[:3] |
Using a tuple would more clearly indicate that we expect 3 items.
Member
Author
There was a problem hiding this comment.
I implemented a version that returns a tuple. mypy was a little bit in the way here ...
finswimmer
force-pushed
the
fix/env-info-activated-python
branch
from
a42859d to
28ea70b
Compare
neersighted
force-pushed
the
fix/env-info-activated-python
branch
from
28ea70b to
3be157c
Compare
neersighted
approved these changes
Nov 22, 2022
Member
neersighted
left a comment
neersighted
left a comment
There was a problem hiding this comment.
I'd rather we had one source of truth for this/it feels like the abstraction is leaking even more, but this is a fix for a real issue and it doesn't make the amount of work needed to refactor this file greater.
neersighted
added
kind/bug
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
Until now
virtualenvs.prefer-active-pythonwasn't respected by theEnvManagerwhen it tries to get the venv. This is fixed by the PR.I took the chance to refactor
EnvManagerin that way, thatIOis now set during initializing a new object. Thus it is not necessary to pass it in into the several methods.Pull Request Check List
Resolves: #6893
Resolves: #5947