refactor(venv): use ensurepip and common methods #29
Merged
joabech merged 2 commits intoanalogdevicesinc:mainfrom Apr 20, 2026
Merged
refactor(venv): use ensurepip and common methods #29joabech merged 2 commits intoanalogdevicesinc:mainfrom
joabech merged 2 commits intoanalogdevicesinc:mainfrom
Conversation
Simple, inline fixes to handle the windows path and format. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
There is no guarantee pip is installed in the host to inherit to the virtual environment. It is a bad practice to use host pip without upgrading since it will be out-of-date. The rationale behind the command is described at: https://peps.python.org/pep-0453/ ensurepip --upgrade is not used because it ensures pip is at least as recent as the one available in ensurepip, not the latest released. Instead, upgrade explicitly with pip. The method run_python_venv_creation was added to remove the repeated venv creation logic. Signed-off-by: Jorge Marques <jorge.marques@analog.com>
Contributor
|
Thanks, this looks like a good improvement. I'll run a few tests, then I'll merge if I see nothing strange.
No, that's not the case, but it's only the |
Contributor
|
I've tested this out in a few scenarios now and it's all good, thanks, this is a great improvement. Merging! |
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 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.
There is no guarantee pip is installed in the host to inherit to the
virtual environment. It is a bad practice to use host pip without
upgrading since it will be out-of-date.
The rationale behind the command is described at:
https://peps.python.org/pep-0453/
ensurepip --upgrade is not used because it ensures pip is at least as
recent as the one available in ensurepip, not the latest released.
Instead, upgrade explicitly with pip.
The method run_python_venv_creation was added to remove the repeated
venv creation logic.
Also loosely fix for windows (checked paths only), but tests should run on windows if we want to (pretend) to support (for unknown reasons).
Removed:
get_venv_pip_path: invoked vevn's pip directly. Despite functional, for all other logic we require invoke asvenv/bin/python -m, it was a weird exception.Tests:
Notes:
This pr does not investigate doc_manager, I touched it solely to move on with the venv refactor.
I believe there is a need for true venv generics, there is an assumption that venv is only for the documentation but there are many handy tools on it across the sdks.