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.

Sign up for GitHub

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

Jump to bottom

Favor the "venv" sysconfig install scheme over the default and distutils scheme #2209

Merged
merged 1 commit into from
Oct 28, 2021
Merged

Favor the "venv" sysconfig install scheme over the default and distutils scheme #2209

merged 1 commit into from
Oct 28, 2021

Conversation

hroncok
Copy link
Contributor

@hroncok hroncok commented Oct 8, 2021 •
edited
Loading

Python is preparing to allow re-distributors to set custom sysconfig install schemes in 3.11+:

https://bugs.python.org/issue43976

Fedora is already adapting the default installation scheme to their needs:

https://lists.fedoraproject.org/archives/list/python-devel@lists.fedoraproject.org/thread/AAGUFQZ4RZDU7KUN4HA43KQJCMSFR3GW/

With either of the above, the distributors need to signalize the paths used in virtual environments somehow. When they set the "venv" install scheme in sysconfig, it is now favored over the default sysconfig scheme as well as over distutils.

Fixes #2208

Thanks for contributing, make sure you address all the checklists (for details on how see

development documentation)!

  • ran the linter to address style issues (tox -e fix_lint)
  • wrote descriptive pull request text
  • ensured there are test(s) validating the fix
  • added news fragment in docs/changelog folder
  • updated/extended the documentation

I'll work on the docs next. Opening earlier to validate the test works on Widnows.

@hroncok hroncok mentioned this pull request Oct 8, 2021
Closed
@hroncok hroncok force-pushed the install_scheme branch 2 times, most recently from f83c61d to 4cf0aab Compare October 8, 2021 12:53
gaborbernat
gaborbernat reviewed Oct 23, 2021
View reviewed changes
Copy link
Contributor

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hroncok so what's next here?

@hroncok
Copy link
Contributor Author

hroncok commented Oct 23, 2021

I was giving https://bugs.python.org/issue45413 some time to sink in. Will finish this either way next week.

@hroncok hroncok force-pushed the install_scheme branch 2 times, most recently from 5824c97 to 52e2328 Compare October 25, 2021 11:37
@hroncok
Copy link
Contributor Author

hroncok commented Oct 25, 2021 •
edited
Loading

I wonder where to document this permanently. It seems like a bit too implementation-wise thing to put into user_guide.rst.

Other than that, this PR is ready for review. I've rebased it and added 2 fixup-like commits separately on top for easier review, with the intention to squash.

@hroncok hroncok marked this pull request as ready for review October 25, 2021 11:44
@hroncok
Copy link
Contributor Author

hroncok commented Oct 25, 2021

I do not understand the codedecov failure, it seems that all added code is covered with tests, but the overall coverage has decreased 😕

@gaborbernat
Copy link
Contributor

I do not understand the codedecov failure, it seems that all added code is covered with tests, but the overall coverage has decreased 😕

Me neither, codecov is very flaky 😆 But ignore that.

gaborbernat
gaborbernat reviewed Oct 25, 2021
View reviewed changes
src/virtualenv/discovery/py_info.py Outdated Show resolved Hide resolved
docs/changelog/2208.feature.rst Outdated Show resolved Hide resolved
tests/unit/discovery/py_info/test_py_info.py Outdated Show resolved Hide resolved
@hroncok hroncok force-pushed the install_scheme branch 2 times, most recently from 0218c8c to fa93ca7 Compare October 25, 2021 14:36
gaborbernat
gaborbernat requested changes Oct 25, 2021
View reviewed changes
Copy link
Contributor

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer if you'd define it as a unicode and then decode it on python 2 as a dict comprehension would make the intent clearer and they you could document why it's needed.

Otherwise looks good.

@gaborbernat
Copy link
Contributor

You'll need to please the PyPy interpreter too 😊

@hroncok
Copy link
Contributor Author

hroncok commented Oct 26, 2021

I had no idea it uses a custom install scheme name. Will do.

@hroncok
Copy link
Contributor Author

hroncok commented Oct 26, 2021

Done.

@hroncok
Copy link
Contributor Author

hroncok commented Oct 26, 2021

Also added pypy_nt for PyPy on Windows.

@hroncok
Favor the "venv" sysconfig install scheme over the default and distut…
ad38fda 
…ils scheme

Python is preparing to allow re-distributors to set custom sysconfig install schemes in 3.11+:

https://bugs.python.org/issue43976

Fedora is already adapting the default installation scheme to their needs:

https://lists.fedoraproject.org/archives/list/python-devel@lists.fedoraproject.org/thread/AAGUFQZ4RZDU7KUN4HA43KQJCMSFR3GW/

With either of the above, the distributors need to signalize the paths used
in virtual environments somehow.
When they set the "venv" install scheme in sysconfig,
it is now favored over the default sysconfig scheme as well as over distutils.

A similar technique was proposed to Python,
for the venv module: https://bugs.python.org/issue45413

Fixes pypa#2208
gaborbernat
gaborbernat approved these changes Oct 28, 2021
View reviewed changes
Copy link
Contributor

@gaborbernat gaborbernat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gaborbernat gaborbernat merged commit 8da79db into pypa:main Oct 28, 2021
@hroncok hroncok mentioned this pull request Mar 2, 2022
Merged
@torsava torsava mentioned this pull request May 6, 2022
Merged
2 tasks
@hroncok hroncok mentioned this pull request Aug 14, 2022
Closed
5 tasks
@yvt yvt mentioned this pull request Sep 19, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FFY00 FFY00 FFY00 left review comments

@gaborbernat gaborbernat gaborbernat approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Custom distutils/sysconfig INSTALL_SCHEME leads to different paths in virtualenv, how to avoid this?
3 participants
@hroncok @gaborbernat @FFY00