-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
PEP 796: Relative Virtual Environments [initial draft] #4476
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Hello, before we go any further with this PR, has the idea been discussed on Discourse? And after that we'll need a sponsor before assigning the PEP number, do you have one yet? Let's unassign 796 for now. Please see: |
Thanks @hugovk and @StanFromIreland for the early review. I used a draft PR to see the CI results to further clean it up -- my apologies for wasting some of your time, but thank you regardless.
Yes: https://discuss.python.org/t/making-venvs-relocatable-friendly/96177 The criteria of "discussed enough with enough support" is vague, but what gave me enough confidence to begin a PEP and start a (draft) PR at this point was:
Thanks for clarifying that part! Yes, finding a sponsor is my next big step. |
Tip: you can enable GitHub Actions at https://github.com/rickeylev/peps/actions and run the CI on your fork.
Good luck! |
Thanks @ncoghlan for sponsoring and approving! Let's continue with PEP number 796. Next steps: a PEP editor to review, then merge, and then the PEP discussion can be opened for this proposal. |
Co-authored-by: Alyssa Coghlan <[email protected]>
This comment has been minimized.
This comment has been minimized.
@paveldikov thank you for the comment, please could you re-post it on Discourse? The peps repo / PR discussion is mainly for editorial discussion, rather than substantive comment on the proposal itself. A |
Co-authored-by: Hugo van Kemenade <[email protected]> Co-authored-by: Zanie Blue <[email protected]>
…to relative.venvs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the reviews! I'm out from under $dayJob and vacation backlogs a bit now and addressed comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some little notes
peps/pep-0796.rst
Outdated
This PEP describes how a relative path for ``home`` in a Python virtual | ||
environment's ``pyvenv.cfg`` is understood by the Python startup process. | ||
Specifically, how it is canonicalized into an absolute path later used | ||
by the runtime. This small detail is a fundamental building block for | ||
virtual environments to be more portable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I read this, it's not clear to me what exactly is being proposed here, it makes it seem like this is better suited for a doc.
peps/pep-0796.rst
Outdated
relative paths for ``home`` in ``pyvenv.cfg``. | ||
|
||
Currently, relative paths resolve relative to the process's current working | ||
directory. Because CWD isn't knowable in advance, it makes relative paths today |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence is grammatically incorrect, and this is the only usage of the acronym which is also not explained. I suggest it is just replace with the full spelling, and corrected: "Because the current working directory isn't ..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Belatedly removed the DO-NOT-MERGE label (that was added pending the Discourse discussion and sponsorship of the PEP) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PEP is currently missing a Security Implications section, which I think should be added given that arbitrary directory traversal is permitted -- at the very least explaining why this is fine.
Several editorial notes, I think the Motivation & Rationale sections should be strengthened to focus on the benefits from relative environments, there is currently (I believe) a lot of assumed context.
The PEP also discusses at some length a broader proposal for reloacatable venvs. Is it worth considering making that the proposal here? I don't know the specifics, so it might be that the changes needed for 'relocatable' are too large to tackle in one go.
A
peps/pep-0796.rst
Outdated
First, it is currently prescribed that the ``home`` value in ``pyvenv.cfg`` be | ||
an absolute path. The behavior of relative paths is unspecified. While |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where? Please provide a cross-reference to the Python documentation or packaging standards
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not documented anywhere, it just doesn't work if you try to do it. I guess we could link to the bug report that precipitated the PEP: python/cpython#135773
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a link to the source on gh?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, there's no docs I'm aware of, either. I've linked to the issue, which has discussion and explanation of the code paths. It's hard to link to "the code" (getpath.py) because it requires a lot of work to unwind the particular code paths. The issue explains it all, though.
Motivation | ||
========== | ||
|
||
There are two main motivations for allowing relative paths in ``pyvenv.cfg``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence doesn't add much. It's also somewhat confused by the next para which says that the reason they are wanted is because they're prohibited.
I would first (briefly) explain to the reader what the home
field in pyvenv.cfg
is for, then go on to discuss the benefits of relative paths. Assume the reader is technically competent, but doesn't have all the context you do of the history here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are two main motivations for allowing relative paths in ``pyvenv.cfg``. | |
The ``home`` field in ``pyvenv.cfg`` is used on interpreter startup to determine the actual Python | |
interpreter installation that is used to execute code in that virtual environment. Currently, | |
this path is required to be absolute for correct virtual environment operation - as the original PEP | |
adding virtual environments didn't cover any specific way of processing relative paths, their | |
behaviour is implementation dependent. CPython releases up to and including CPython 3.14 | |
resolve them relative to the current process working directory, making them too unreliable to | |
use in practice. |
@AA-Turner has a point that the subsection could benefit from restructuring in general, though:
- move the rationale paragraphs up here to the motivation section
- move some of the technical details motivation text down to the rationale section
- add the rationale for excluding the tool dependent environment portability features (those problems can be solved at the tool level, and need to be solved at the tool level because the right answers are dependent on the exact intended usage model of the relative virtual environments)
For PEP readers that aren't already familiar with the problem, the core points we want to get across are that there are use cases that rely on relative virtual environments (motivation), and while we can deal with most of the challenges involved in setting them up in ways we're happy with, the ways we have to deal with this problem are particularly horrible, and we'd like to ditch them in favour of properly defined interpreter level handling of relative paths in the home
key (rationale).
Co-authored-by: Alyssa Coghlan <[email protected]> Co-authored-by: Adam Turner <[email protected]> Co-authored-by: Stan Ulbrych <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Applied suggestions and addressed a few comments; didn't have time to address everything, though.
peps/pep-0796.rst
Outdated
First, it is currently prescribed that the ``home`` value in ``pyvenv.cfg`` be | ||
an absolute path. The behavior of relative paths is unspecified. While |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, there's no docs I'm aware of, either. I've linked to the issue, which has discussion and explanation of the code paths. It's hard to link to "the code" (getpath.py) because it requires a lot of work to unwind the particular code paths. The issue explains it all, though.
Second, such relative paths are a building block to enable portable virtual | ||
environments, i.e. copying a virtual environment as-is between hosts of | ||
compatible platforms. For example, by pointing to a parent directory, the | ||
virtual environment becomes independent of path prefix differences between | ||
hosts (e.g. ``/usr/local/`` in a container vs | ||
``/home/user/.pyenv/versions/3.12.0/bin`` in a user's dev environment). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal is to enable the basic building block of portable venvs: the runtime initializing itself properly (finding python home).
There's two main reasons it doesn't propose a detailed solution to portable venvs:
- Multiple possible implementations. Should the relative path point outside the venv? To a sub-directory? A sibling dir of the venv? Which to use depends on how the venv is copied to other locations.
- Hard questions with package installation. The discussion thread touches on this. A good example are
#!python
shebangs that installers rewrite. These questions are hard and require installers to figure out answers, but more importantly, they're separate from the core runtime initialization logic.
popular mechanism for running Python applications. This provides several | ||
benefits: | ||
|
||
* The closer the development environment is to the non-development environment, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"non development environment" means where something goes after you've done your local development. Typically CI or some production environment.
The difference between the two is each have their own virtual environment created and populated from scratch. This means each has to locate Python on its own, then download and install packages. Each of these steps introduces the potential for system-specific differences. e.g. /usr/bin/python3 may differ by OS or OS version, the python on path may be affected by the shell environment, the python install may have different things installed into it, package installation is non-trivial, and etc.
Does that help explain?
peps/pep-0796.rst
Outdated
relative paths for ``home`` in ``pyvenv.cfg``. | ||
|
||
Currently, relative paths resolve relative to the process's current working | ||
directory. Because CWD isn't knowable in advance, it makes relative paths today |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
peps/pep-0796.rst
Outdated
(Bazel, omitting the ``home`` key entirely to trigger an implementation | ||
dependent fallback to resolving via a symlinked interpreter binary on | ||
non-Windows systems) or by requiring a post-installation script to be executed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Basic requirements (all PEP Types)
pep-NNNN.rst
), PR title (PEP 123: <Title of PEP>
) andPEP
headerAuthor
orSponsor
, and formally confirmed their approvalAuthor
,Status
(Draft
),Type
andCreated
headers filled out correctlyPEP-Delegate
,Topic
,Requires
andReplaces
headers completed if appropriate.github/CODEOWNERS
for the PEPStandards Track requirements
Python-Version
set to valid (pre-beta) future Python version, if relevantDiscussions-To
andPost-History
Work towards python/cpython#136051
📚 Documentation preview 📚: https://pep-previews--4476.org.readthedocs.build/