FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-92897: Ensure `venv --copies` respects source build property of the creating interpreter (GH-92899) by jkloth · Pull Request #92899 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-92897: Ensure venv --copies respects source build property of the creating interpreter (GH-92899) - #92899

Merged
vsajip merged 2 commits into
python:mainfrom
jkloth:gh92897
Jul 5, 2022
ConversationCommits (2)ChecksFiles changed
Merged

gh-92897: Ensure venv --copies respects source build property of the creating interpreter (GH-92899)#92899
vsajip merged 2 commits into
python:mainfrom
jkloth:gh92897

Conversation

jkloth commented May 17, 2022 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

There is a discrepancy between a symlink venv and a copied venv when determining whether the interpreter is considered to be from a source build. A symlink venv indicates True for sysconfig.is_python_build() whereas a copied venv returns False.

Since symlink venvs are the default for POSIX (where the majority of source builds would be used/tested), I have opted to use that behavior as correct. Windows users are far more likely to use the binary installer so odds of venvs being created from source builds are quite low. Likewise with venv --copies on POSIX.

These changes will allow for building extensions out of venvs created from source builds without any pre-configuration steps (see this section from pyperformance documentation).

Adding @zooba for Windows and @vsajip for venv

jkloth requested a review from vsajip as a code owner May 17, 2022 20:39

zooba commented May 17, 2022

Copy link
Copy Markdown
Member

Don't touch distutils - it's going to be deleted in this release anyway, and there are no maintenance releases where it is undeprecated.

If we're using it within the stdlib, update the references elsewhere. If setuptools is impacted, they need the patch.

I haven't looked any deeper than that - it's late. I'll try and take a look later this week

jkloth commented May 17, 2022

Copy link
Copy Markdown
Contributor Author

The changes to distutils are only to keep it from raising exceptions. Since it is being removed, I saw no need to keep private functions around just for use within distutils which would prevent a more straight forward implementation. Hence the modifications to distutils.

The changes to stdlib sysconfig have no impact on setuptools distutils.sysconfig so nothing to do there.

vstinner mentioned this pull request May 18, 2022
Closed

vstinner commented May 18, 2022

Copy link
Copy Markdown
Member

The changes to distutils are only to keep it from raising exceptions.

In practice, setuptools now injects its own distutils flavor, so I am not sure that it's worth it to modify it here. At least, you should get your change merged first in https://github.com/pypa/distutils/

jkloth commented May 18, 2022

Copy link
Copy Markdown
Contributor Author

distutils.sysconfig and distutils.test.test_sysconfig in the stdlib have diveraged from those in setuptools (PR #23142). Therefore the changes included here have ZERO correspondance to those in setuptools. distutils.sysconfig in setuptools does not rely on any private functions from sysconfig so there is no change to merge.

jkloth commented May 18, 2022

Copy link
Copy Markdown
Contributor Author

To make it absolutely clear, the changes here to distutils modules are solely to prevent an ImportError from occurring wherever distutils modules are still used in the stdlib. The changes are intended to keep it "working" until it can be fully removed at a later date.

zooba approved these changes May 18, 2022
View reviewed changes

zooba left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM, and appears to handle the case on Windows where a venv is created from a source build. I can only assume the behaviour is correct on other platforms?

jkloth commented May 19, 2022

Copy link
Copy Markdown
Contributor Author

As far as other platforms go, it is tested on Ubuntu (WSL).

vsajip changed the title gh-92897: Ensure venv --copies respects source build property of the creating interpreter gh-92897: Ensure venv --copies respects source build property of the creating interpreter (GH-92899) Jul 5, 2022
vsajip merged commit 0675975 into python:main Jul 5, 2022

miss-islington commented Jul 5, 2022

Copy link
Copy Markdown
Contributor

Thanks @jkloth for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

miss-islington commented Jul 5, 2022

Copy link
Copy Markdown
Contributor

Thanks @jkloth for the PR, and @vsajip for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington commented Jul 5, 2022

Copy link
Copy Markdown
Contributor

Sorry, @jkloth and @vsajip, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 067597522a9002f3b8aff7f46033f10acb2381e4 3.10

miss-islington commented Jul 5, 2022

Copy link
Copy Markdown
Contributor

Sorry @jkloth and @vsajip, I had trouble checking out the 3.11 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 067597522a9002f3b8aff7f46033f10acb2381e4 3.11

vsajip pushed a commit to vsajip/cpython that referenced this pull request Jul 5, 2022
[3.11] pythongh-92897: Ensure venv --copies respects source build p…
124acce 
…roperty of the creating interpreter (pythonGH-92899)

(cherry picked from commit 0675975)

Co-authored-by: Jeremy Kloth <jeremy.kloth@gmail.com>
vsajip mentioned this pull request Jul 5, 2022
Merged

bedevere-bot commented Jul 5, 2022

Copy link
Copy Markdown

GH-94567 is a backport of this pull request to the 3.11 branch.

bedevere-bot removed the needs backport to 3.11 only security fixes label Jul 5, 2022
vsajip mentioned this pull request Jul 5, 2022
Open
vsajip pushed a commit to vsajip/cpython that referenced this pull request Jul 5, 2022
pythongh-92897: Ensure venv --copies respects source build property…
5e0626c 
… of the creating interpreter (pythonGH-92899)

(cherry picked from commit 0675975)
vsajip mentioned this pull request Jul 5, 2022
Closed

bedevere-bot commented Jul 5, 2022

Copy link
Copy Markdown

GH-94572 is a backport of this pull request to the 3.10 branch.

vsajip added a commit that referenced this pull request Jul 5, 2022
[3.11] gh-92897: Ensure venv --copies respects source build propert…
49aeda9 
…y of the creating interpreter (GH-92899) (GH-94567)

(cherry picked from commit 0675975)

Co-authored-by: Jeremy Kloth <jeremy.kloth@gmail.com>
FFY00 mentioned this pull request Nov 21, 2022
Merged
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

zooba zooba approved these changes
vsajip Awaiting requested review from vsajip vsajip is a code owner

Assignees

vsajip

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

Back | FazBrowse Home | New Git URL