GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-125398: Convert paths in venv activate script when using Git Bash under Windows by julienp · Pull Request #125399 · python/cpython

Conversation

@julienp

@julienp julienp commented Oct 13, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

With #112508 the check to convert 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.

Fixes #125398

  • Issue: The virtualenv activate script does not correctly detect the Windows Git Bash shell #125398

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@julienp
julienp requested a review from vsajip as a code owner October 13, 2024 14:40
@ghost

ghost commented Oct 13, 2024 •
edited by ghost
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app

bedevere-app Bot commented Oct 13, 2024

Copy link
Copy Markdown

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 skip news label instead.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added the awaiting review label Oct 13, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Oct 13, 2024
The virtualenv activate script does not correctly detect the Windows Git Bash shell #125398
Closed
@julienp
Convert paths in venv activate script when using Git Bash under Windows
61ab573 
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.
@julienp
julienp force-pushed the julienp/venv-mingw branch from 344c038 to 61ab573 Compare October 13, 2024 15:06
rruuaanng
rruuaanng reviewed Oct 14, 2024
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2024-10-13-15-04-58.gh-issue-125398.UW7Ndv.rst Outdated
@@ -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.

@rruuaanng rruuaanng Oct 14, 2024

Copy link
Copy Markdown
Contributor

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.

Suggested change
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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
julienp reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@erlend-aasland erlend-aasland Oct 15, 2024

Copy link
Copy Markdown
Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@vsajip vsajip Oct 15, 2024

Copy link
Copy Markdown
Member

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@erlend-aasland erlend-aasland Oct 15, 2024

Copy link
Copy Markdown
Contributor

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` :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@erlend-aasland erlend-aasland Oct 15, 2024

Copy link
Copy Markdown
Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@zooba zooba Oct 15, 2024

Copy link
Copy Markdown
Member

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).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@erlend-aasland erlend-aasland Oct 15, 2024

Copy link
Copy Markdown
Contributor

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 ;)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@zooba zooba Oct 16, 2024

Copy link
Copy Markdown
Member

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")

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@vsajip vsajip Oct 17, 2024

Copy link
Copy Markdown
Member

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
zooba reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@julienp @rruuaanng
Update Misc/NEWS.d/next/Library/2024-10-13-15-04-58.gh-issue-125398.U…
a81e9cd 
…W7Ndv.rst

Co-authored-by: RUANG (Roy James) <[email protected]>
@vsajip vsajip added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 15, 2024
@bedevere-bot

bedevere-bot commented Oct 15, 2024

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @vsajip for commit a81e9cd 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Oct 15, 2024
@erlend-aasland
erlend-aasland requested a review from a team October 15, 2024 10:49
vsajip
vsajip reviewed Oct 15, 2024
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2024-10-13-15-04-58.gh-issue-125398.UW7Ndv.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@vsajip
Change to use the envvar role.
0879c48
vsajip
vsajip reviewed Oct 16, 2024
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2024-10-13-15-04-58.gh-issue-125398.UW7Ndv.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@vsajip
Try to circumvent doc linting error.
d47476d
@vsajip
vsajip merged commit 2a378db into python:main Oct 19, 2024
@bedevere-app bedevere-app Bot removed the awaiting review label Oct 19, 2024
@vsajip vsajip added needs backport to 3.12 only security fixes needs backport to 3.13 only security fixes labels Oct 19, 2024
@miss-islington-app

miss-islington-app Bot commented Oct 19, 2024

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app

miss-islington-app Bot commented Oct 19, 2024

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app

miss-islington-app Bot commented Oct 19, 2024

Copy link
Copy Markdown

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

cherry_picker 2a378dba987e125521b678364f0cd44b92dd5d52 3.12
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington-app miss-islington-app Bot assigned vsajip Oct 19, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 19, 2024
@julienp @miss-islington
pythongh-125398: Convert paths in venv activate script when using Git…
66bda75 
… 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 <[email protected]>
@bedevere-app

bedevere-app Bot commented Oct 19, 2024

Copy link
Copy Markdown

GH-125733 is a backport of this pull request to the 3.13 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Oct 19, 2024
vsajip pushed a commit that referenced this pull request Oct 19, 2024
@miss-islington
[3.13] gh-125398: Convert paths in venv activate script when using Gi…
9e43330 
…t Bash under Windows (GH-125399) (GH-125733)

(cherry picked from commit 2a378db)
@julienp

julienp commented Oct 22, 2024

Copy link
Copy Markdown
Contributor Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@julienp
julienp deleted the julienp/venv-mingw branch October 22, 2024 22:33
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
@julienp @ebonnal
pythongh-125398: Convert paths in venv activate script when using Git…
f781a5d 
… 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.
@hugovk hugovk removed the needs backport to 3.12 only security fixes label Feb 25, 2025
@LuNoX LuNoX mentioned this pull request Jun 7, 2026
gh-151042: venv: Pass VIRTUAL_ENV through cygpath inside fish on Windows #151043
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Aug 12, 2026
feat: vector semantic search tool cbtw-apac/HarborRAG#27
Merged
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vsajip vsajip vsajip left review comments
@zooba zooba zooba left review comments
@erlend-aasland erlend-aasland erlend-aasland left review comments
+1 more reviewer
@rruuaanng rruuaanng rruuaanng left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@vsajip vsajip

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

The virtualenv activate script does not correctly detect the Windows Git Bash shell

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@julienp @bedevere-bot @vsajip @zooba @erlend-aasland @rruuaanng @hugovk
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.