◐ Shell
reader mode source ↗
Skip to content

bpo-45337: Use the realpath of the new executable when creating a venv on Windows#28663

Merged
zooba merged 6 commits into
python:mainfrom
zooba:bpo-45337
Oct 7, 2021
Merged

bpo-45337: Use the realpath of the new executable when creating a venv on Windows#28663
zooba merged 6 commits into
python:mainfrom
zooba:bpo-45337

Conversation

@zooba

@zooba zooba commented Sep 30, 2021

Copy link
Copy Markdown
Member

Also warn if the realpath does not match the user-specified location.

https://bugs.python.org/issue45337

…v on Windows.

Also warn if the realpath does not match the user-specified location.
@zooba zooba added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 1, 2021
@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @zooba for commit 49fa664 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 1, 2021
@zooba

zooba commented Oct 1, 2021

Copy link
Copy Markdown
Member Author

I triggered the buildbots mainly to test the new one we just added, this PR is still very open for updates :)

@ambv

ambv commented Oct 5, 2021

Copy link
Copy Markdown
Contributor

I see what you're trying to do here. In fact, I can't find a way to do it any better myself. That being said, I have a (minor) concern.

Your introduction of context.env_cmd while keeping context.env_exe around makes it unclear where each one should be used. Maybe better naming of the new context variable could help here. As it stands now, the reader has little chance to notice the subtle difference, so there's some danger of a future regression.

@vsajip

vsajip commented Oct 6, 2021

Copy link
Copy Markdown
Member

Your introduction of context.env_cmd while keeping context.env_exe around makes it unclear where each one should be used

Perhaps just documenting explicitly what they're for/why they're used would help, in particular if any other issues in this area come up relating to the MS-Store version of Python.

@zooba

zooba commented Oct 6, 2021

Copy link
Copy Markdown
Member Author

Perhaps just documenting explicitly what they're for/why they're used would help, in particular if any other issues in this area come up relating to the MS-Store version of Python.

I referenced the bug as an example of why we'd need this - unfortunately, this side of things keeps changing on the OS side too often, so I'm very hesitant to pretend in our source code that it's a known thing.

Also changed the member name to env_exec_cmd, which is what I had originally, but didn't like how long it was.

@zooba zooba merged commit 6811fda into python:main Oct 7, 2021
@miss-islington

Copy link
Copy Markdown
Contributor

Thanks @zooba for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

@bedevere-bot

Copy link
Copy Markdown

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

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Oct 7, 2021
@zooba

zooba commented Oct 7, 2021

Copy link
Copy Markdown
Member Author

Hopefully there were no further concerns. We've got time to fix them up if so

@bedevere-bot

Copy link
Copy Markdown

GH-28811 is a backport of this pull request to the 3.9 branch.

@zooba zooba deleted the bpo-45337 branch October 7, 2021 20:26
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 7, 2021
…v on Windows (pythonGH-28663)

(cherry picked from commit 6811fda)

Co-authored-by: Steve Dower <steve.dower@python.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 7, 2021
…v on Windows (pythonGH-28663)

(cherry picked from commit 6811fda)

Co-authored-by: Steve Dower <steve.dower@python.org>
miss-islington added a commit that referenced this pull request Oct 7, 2021
…v on Windows (GH-28663)

(cherry picked from commit 6811fda)

Co-authored-by: Steve Dower <steve.dower@python.org>
miss-islington added a commit that referenced this pull request Oct 7, 2021
…v on Windows (GH-28663)

(cherry picked from commit 6811fda)

Co-authored-by: Steve Dower <steve.dower@python.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants