Skip to content
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

Add install scheme for virtual environments #89576

Closed
FFY00 opened this issue Oct 8, 2021 · 16 comments
Closed

Add install scheme for virtual environments #89576

FFY00 opened this issue Oct 8, 2021 · 16 comments
Assignees
Labels
3.10 only security fixes 3.11 only security fixes type-feature A feature request or enhancement

Comments

@FFY00
Copy link
Member

FFY00 commented Oct 8, 2021

BPO 45413
Nosy @jaraco, @encukou, @stefanor, @zooba, @hroncok, @frenzymadness, @gaborbernat, @FFY00
PRs
  • bpo-45413: Define "posix_venv", "nt_venv" and "venv" sysconfig installation schemes #31034
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/FFY00'
    closed_at = <Date 2022-03-18.16:07:36.702>
    created_at = <Date 2021-10-08.17:50:02.466>
    labels = ['type-feature', '3.10', '3.11']
    title = 'Add install scheme for virtual environments'
    updated_at = <Date 2022-03-18.16:07:36.702>
    user = 'https://github.com/FFY00'

    bugs.python.org fields:

    activity = <Date 2022-03-18.16:07:36.702>
    actor = 'FFY00'
    assignee = 'FFY00'
    closed = True
    closed_date = <Date 2022-03-18.16:07:36.702>
    closer = 'FFY00'
    components = []
    creation = <Date 2021-10-08.17:50:02.466>
    creator = 'FFY00'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 45413
    keywords = ['patch']
    message_count = 16.0
    messages = ['403485', '403486', '403513', '403720', '412201', '412202', '412302', '412794', '412805', '412896', '412911', '412933', '413405', '414510', '415479', '415509']
    nosy_count = 8.0
    nosy_names = ['jaraco', 'petr.viktorin', 'stefanor', 'steve.dower', 'hroncok', 'frenzy', 'gaborjbernat', 'FFY00']
    pr_nums = ['31034']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue45413'
    versions = ['Python 3.10', 'Python 3.11']

    @FFY00
    Copy link
    Member Author

    FFY00 commented Oct 8, 2021

    Python 3.10 introduced sysconfig._get_preferred_schemes[1], a mechanism to allow downstream distributors to overwrite the default install scheme.
    This is done to support downstream modifications where distributors change the installation layout (eg. different site-packages directory). So, distributors will change the default scheme to one that correctly represents their layout.

    This presents an issue for projects/people that need to bootstrap virtual environments, like virtualenv (see [2]). As distributors might now be customizing the default install scheme, there is no guarantee that the information returned by sysconfig.get_default_scheme/get_paths is correct for the virtual environment, the only guarantee we have is that it correct for the *current* environment. When bootstrapping a virtual environment, we need to know its layout, so that we can place the files in the correct locations.

    The usual solution in situations like this would be to invoke the interpreter we are targeting to get the correct information (eg. path/to/python -m sysconfig), but that is not possible here as the environment does not exist yet -- we are the ones trying to create it.

    To solve this issue, I propose the addition of a "virtual" or "venv" install scheme, for virtual environments. The idea is that virtual environments (defined by the presence of pyvenv.cfg, see [3][4]) will always use this scheme, they will use the paths specified by this scheme and sysconfig.get_default_scheme will always return it (_get_preferred_schemes would have no effect here!).
    This makes it possible to know the virtual environment layout for another interpreter, via sysconfig.get_paths(scheme='virtual').

    I am not cure if this should be adopted in 3.10 or 3.11, as it effectively makes it impossible to reliably construct virtual environments, and requires workarounds such as [5], that pretty much implements this proposal with non-standardized downstream patches.

    [1] https://docs.python.org/3/library/sysconfig.html#sysconfig.\_get_preferred_schemes
    [2] pypa/virtualenv#2208
    [3] https://docs.python.org/3/library/site.html?highlight=pyvenv.cfg
    [4] https://docs.python.org/3/library/venv.html?highlight=pyvenv.cfg
    [5] pypa/virtualenv#2209

    @FFY00 FFY00 added 3.10 only security fixes 3.11 only security fixes labels Oct 8, 2021
    @FFY00 FFY00 self-assigned this Oct 8, 2021
    @FFY00 FFY00 added type-feature A feature request or enhancement 3.10 only security fixes 3.11 only security fixes labels Oct 8, 2021
    @FFY00 FFY00 self-assigned this Oct 8, 2021
    @FFY00 FFY00 added the type-feature A feature request or enhancement label Oct 8, 2021
    @FFY00 FFY00 changed the title Add install scheme for virtual environment Add install scheme for virtual environments Oct 8, 2021
    @FFY00 FFY00 changed the title Add install scheme for virtual environment Add install scheme for virtual environments Oct 8, 2021
    @hroncok
    Copy link
    Mannequin

    hroncok mannequin commented Oct 8, 2021

    The existing install schemes contain values for all different kinds of OSes, somehow named according to them.

    If we introduce a single "virtual"/"venv" scheme, it would need to have different contents on different OSes (e.g. Windows vs POSIX). I don't think that would cause any actual trouble, but it would be somewhat different than all the other schemes.

    If we introduce multiple ones (e.g. "posix_venv" and "nt_venv") we would need an additional layer to get the one appropriate for the current platform.

    Hence, I think having a single one is more pragmatic.

    @FFY00
    Copy link
    Member Author

    FFY00 commented Oct 9, 2021

    Yes, we could have several schemes, but I think having only one is more sensible.

    The implementation would be fairly easy. We would just copy the "nt" scheme if on Windows, otherwise "posix_prefix".

    @encukou
    Copy link
    Member

    encukou commented Oct 12, 2021

    Starting out with just "venv" doesn't mean we can't add "posix_venv"/"nt_venv" later (if e.g. someone needs to install into a filesystem for another platform).

    @hroncok
    Copy link
    Mannequin

    hroncok mannequin commented Jan 31, 2022

    I'll try to draft this change for Python 3.11.

    @hroncok
    Copy link
    Mannequin

    hroncok mannequin commented Jan 31, 2022

    I've created a draft PR in #31034

    It is still missing tests and I have not checked it on Windows, hence still a draft.

    @hroncok
    Copy link
    Mannequin

    hroncok mannequin commented Feb 1, 2022

    The PR is now ready for review.

    @zooba
    Copy link
    Member

    zooba commented Feb 7, 2022

    I think we want the scheme to be static and accessible on all platforms, like the others. So it probably should be 'nt_venv' and 'posix_venv' with additional/improved logic to help apps determine when they need each.

    @FFY00
    Copy link
    Member Author

    FFY00 commented Feb 8, 2022

    I agree.

    @encukou
    Copy link
    Member

    encukou commented Feb 9, 2022

    I think we want the scheme to be static and accessible on all platforms, like the others. So it probably should be 'nt_venv' and 'posix_venv' with additional/improved logic to help apps determine when they need each.

    Why? (This is a real question, I genuinely don't know.)

    To put this in context, this has been discussed since October, and there was agreement on how to change it.
    Now a PR exists. It is tested. Also, since this fixes urgent breaking changes, patches for "venv" is already in Fedora and Ubuntu deadsnakes PPA, and virtualenv also supports it. Changing direction now will mean lots of work changing and re-testing anything. That's definitely possible, just frustrating, but if someone calls to change something again, we risk not making it in 3.11.

    How can we ensure the decision won't change again?
    How can these discussions be made more effective?

    @zooba
    Copy link
    Member

    zooba commented Feb 9, 2022

    All I can say is that I wasn't aware of this discussion, or it blurred into the other discussions and didn't seem to need any attention from me.
    Once I was pinged on the pull request (and I'm *trying* to be better at checking GitHub notifications, though CPython is still the repo that makes it very hard), I took a look.

    I definitely don't want to say that I must be consulted on every sysconfig/site/getpath change (because I don't want to be!), but I hesitate to say that this was the wrong way for it to be caught. It's only "too late" after a release has included it, and up until then it's fine.

    So I guess it can be avoided in the future by checking the surrounding code and seeing how it's used? In this case, the schemes are all approximately static (for a given version of Python), and the *selection* of a scheme is based on the platform. The proposed venv scheme itself is based on the platform, while selection is static. That stands out to me as a difference.

    @FFY00
    Copy link
    Member Author

    FFY00 commented Feb 9, 2022

    I don't think the proposal is incompatible with what I discussed.

    I haven't been super clear on my opinions on the implementation, so let me try to clarify them.

    • I think that we should use a static scheme, accessible on all platforms.
    • If this scheme needs to be independently defined for each platform, we should have different variants, available on all platforms, but still keep a generic named one, as an alias to the platform specific scheme
    • We should not be re-using/aliasing existing schemes, particularly ones that are prone to downstream patching

    So, my proposal would be to define a single static scheme, and changing the interpreter path initialization logic to hardcode its paths when on virtual environments.
    If this presents any issue, and requires the scheme to be different for different platforms, we should add platform specific schemes and make the main one an alias to the correct scheme.

    Hopefully that clarifies things up a bit. We should sort it out as soon as possible and update the PR, I don't think the PR as-is is the best approach.

    What do you think?

    @hroncok
    Copy link
    Mannequin

    hroncok mannequin commented Feb 17, 2022

    I've adapted the PR. See the latest commit (Instead of *venv* scheme, have *posix_venv* and *nt_venv*).

    @encukou
    Copy link
    Member

    encukou commented Mar 4, 2022

    Steve, could you take a look at the PR discussion and chime in about how this should be done?

    @encukou
    Copy link
    Member

    encukou commented Mar 18, 2022

    New changeset 48d9262 by Miro Hrončok in branch 'main':
    bpo-45413: Define "posix_venv", "nt_venv" and "venv" sysconfig installation schemes (GH-31034)
    48d9262

    @FFY00
    Copy link
    Member Author

    FFY00 commented Mar 18, 2022

    With PR 31034 merged, we can now mark this as resolved.

    As mentioned in the PR, there are still some concerns about maintainability and avoiding similar issues to happen in the future. That can be done later, as people find time to work on it.

    Thanks!

    @FFY00 FFY00 closed this as completed Mar 18, 2022
    @FFY00 FFY00 closed this as completed Mar 18, 2022
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes 3.11 only security fixes type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants