-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
gh-125398: Convert paths in venv activate script when using Git Bash under Windows #125399
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
Conversation
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
With python#112508 the check to converts paths when running on Windows was changed from using the non-posix environment variable `$OSTYPE` to using `uname` instead. However this missed the fact that when running under Git Bash on Windows, uname reports `MINGW*` (`$OSTYPE` is still `msys`). This results in `$PATH` being set to something like `D:\a\github-actions-shells\github-actions-shells\venv/Scripts:…`, instead of `/d/a/github-actions-shells/github-actions-shells/venv/Scripts`. Notably, the Git Bash is the bash shell that’s used for GitHub Actions Windows runners, and ships with VSCode.
344c038
to
61ab573
Compare
@@ -0,0 +1 @@ | |||
Fix the conversion of the VIRTUAL_ENV path in the activate script in :mod:`venv` when running in Git Bash for Windows. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Recommend used this.
Fix the conversion of the VIRTUAL_ENV path in the activate script in :mod:`venv` when running in Git Bash for Windows. | |
Fix the conversion of the ``VIRTUAL_ENV`` path in the activate script in :mod:`venv` when running in Git Bash for Windows. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, the correct markup would be :envvar:
:) See also the relevant parts of the devguide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I edited the PR to use the :envvar:
role, but it seems to fail documentation linting:
WARNING: 'envvar' reference target not found: VIRTUAL_ENV [ref.envvar]
Perhaps a nits file needs updating? @erlend-aasland, any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use :envvar:`!VIRTUAL_ENV`
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally though, VIRTUAL_ENV
should be documented in Doc/library/venv.rst
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure VIRTUAL_ENV
is undocumented on purpose. It's sys.argv[0]
that determines whether you're in a venv or not (and if argv[0]
is a relative path, it's resolved using PATH
, which makes that the other relevant environment variable).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, but it's not undocumented; it is mentioned multiple places in our docs, but it lacks a "formal" .. envvar::
definition ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the documentation in the venv module for it, I'd be okay with adding the reference location to there (in short, it says "don't use or rely on this variable, but it might be set")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'm minded to commit this as it is currently (with !VIRTUAL_ENV
), and that envvar
definition can be added in a subsequent PR. Any objections?
…W7Ndv.rst Co-authored-by: RUANG (Roy James) <longjinyii@outlook.com>
Misc/NEWS.d/next/Library/2024-10-13-15-04-58.gh-issue-125398.UW7Ndv.rst
Outdated
Show resolved
Hide resolved
Misc/NEWS.d/next/Library/2024-10-13-15-04-58.gh-issue-125398.UW7Ndv.rst
Outdated
Show resolved
Hide resolved
Sorry, @julienp and @vsajip, I could not cleanly backport this to
|
… Bash under Windows (pythonGH-125399) * Convert paths in venv activate script when using Git Bash under Windows With python#112508 the check to converts paths when running on Windows was changed from using the non-posix environment variable `$OSTYPE` to using `uname` instead. However this missed the fact that when running under Git Bash on Windows, uname reports `MINGW*` (`$OSTYPE` is still `msys`). This results in `$PATH` being set to something like `D:\a\github-actions-shells\github-actions-shells\venv/Scripts:…`, instead of `/d/a/github-actions-shells/github-actions-shells/venv/Scripts`. Notably, the Git Bash is the bash shell that’s used for GitHub Actions Windows runners, and ships with VSCode. (cherry picked from commit 2a378db) Co-authored-by: Julien <julien@caffeine.lu>
GH-125733 is a backport of this pull request to the 3.13 branch. |
Thank you for merging this! I see the bot comment about the backport to 3.12 failing. I think that's expected, as this was a regression in 3.13.0, and the change that introduced the regression was not applied to 3.12. |
… Bash under Windows (pythonGH-125399) * Convert paths in venv activate script when using Git Bash under Windows With python#112508 the check to converts paths when running on Windows was changed from using the non-posix environment variable `$OSTYPE` to using `uname` instead. However this missed the fact that when running under Git Bash on Windows, uname reports `MINGW*` (`$OSTYPE` is still `msys`). This results in `$PATH` being set to something like `D:\a\github-actions-shells\github-actions-shells\venv/Scripts:…`, instead of `/d/a/github-actions-shells/github-actions-shells/venv/Scripts`. Notably, the Git Bash is the bash shell that’s used for GitHub Actions Windows runners, and ships with VSCode.
With #112508 the check to convert paths when running on Windows was changed from using the non-posix environment variable
$OSTYPE
to usinguname
instead.However this missed the fact that when running under Git Bash on Windows, uname reports
MINGW*
($OSTYPE
is stillmsys
).This results in
$PATH
being set to something likeD:\a\github-actions-shells\github-actions-shells\venv/Scripts:…
, instead of/d/a/github-actions-shells/github-actions-shells/venv/Scripts
.Notably, the Git Bash is the bash shell that’s used for GitHub Actions Windows runners, and ships with VSCode.
Fixes #125398