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

gh-126084: Fix venvwlauncher to run pythonw #126088

Merged
merged 8 commits into from
Oct 29, 2024

Conversation

chrullrich
Copy link
Contributor

@chrullrich chrullrich commented Oct 28, 2024

@chrullrich chrullrich force-pushed the fix-126084-venvlauncher branch from 70569a3 to b08755a Compare October 28, 2024 19:03
@ZeroIntensity
Copy link
Member

This is user-facing, so it needs a NEWS entry, per the bot. Probably needs a test too.

Also, for future reference, don't force push--it makes the review process more difficult. Everything is squash merged at the end anyway.

@chrullrich chrullrich requested a review from vsajip as a code owner October 29, 2024 09:36
@chrullrich chrullrich force-pushed the fix-126084-venvlauncher branch from cfe2498 to 040e6f9 Compare October 29, 2024 09:39
@chrullrich chrullrich force-pushed the fix-126084-venvlauncher branch from 040e6f9 to 58f9d7a Compare October 29, 2024 09:41
@chrullrich
Copy link
Contributor Author

That test works fine locally. What could be different about the CI environment?

@chrullrich
Copy link
Contributor Author

So it's not the realpath(). I don't want to waste massive amounts of time and energy by probing around the CI environment and running the whole lot each time. Any suggestions?

@devdanzin
Copy link
Contributor

Locally for me, _winapi.GetModuleFileName(0) always returns the path to the Python process that launched the subprocess. Would sys.executable work?

@chrullrich
Copy link
Contributor Author

Locally for me, _winapi.GetModuleFileName(0) always returns the path to the Python process that launched the subprocess.

It is supposed to return the path to the executable for the current process.

Would sys.executable work?

No:

C:\Daten>pyv\x64\Scripts\python.exe
Python 3.13.0 (tags/v3.13.0:60403a5, Oct  7 2024, 09:38:07) [MSC v.1941 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys; print(sys.executable)
C:\Daten\pyv\x64\Scripts\python.exe
>>> import _winapi; print(_winapi.GetModuleFileName(0))
C:\Program Files\Python313\python.exe

When in a venv, sys.executable is the wrapper in the venv. I'm looking for the "real" Python in the installation that was used to create the venv.

One moment please ...

@chrullrich
Copy link
Contributor Author

chrullrich commented Oct 29, 2024

This one should work in release and debug, but I'm curious to see whether it works in the GIL-less builds.

[Update: It does. The specific requirement is that the "w" immediately follows the "python" in the file name.]

@zooba zooba added needs backport to 3.12 bug and security fixes needs backport to 3.13 bugs and security fixes type-bug An unexpected behavior, bug, or error OS-windows labels Oct 29, 2024
@zooba
Copy link
Member

zooba commented Oct 29, 2024

Thanks for finding and fixing this! What a silly mistake (probably mine...)

@zooba zooba merged commit 802d405 into python:main Oct 29, 2024
44 checks passed
@miss-islington-app
Copy link

Thanks @chrullrich for the PR, and @zooba for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 29, 2024
…26088)

(cherry picked from commit 802d405)

Co-authored-by: Christian Ullrich <chris@chrullrich.net>
@miss-islington-app
Copy link

Sorry, @chrullrich and @zooba, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 802d405ff1233a48004a7c9b302baa76291514d1 3.12

@bedevere-app
Copy link

bedevere-app bot commented Oct 29, 2024

GH-126142 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Oct 29, 2024
@zooba zooba removed the needs backport to 3.12 bug and security fixes label Oct 29, 2024
@zooba
Copy link
Member

zooba commented Oct 29, 2024

Doesn't apply to 3.12, only 3.13.

zooba pushed a commit that referenced this pull request Oct 29, 2024
(cherry picked from commit 802d405)

Co-authored-by: Christian Ullrich <chris@chrullrich.net>
picnixz pushed a commit to picnixz/cpython that referenced this pull request Dec 8, 2024
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS-windows type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pythonw.exe from venv in 3.13 always creates console window
4 participants