Skip to content

gh-151042: venv: Pass VIRTUAL_ENV through cygpath inside fish on Windows#151043

Merged
vsajip merged 2 commits into
python:mainfrom
LuNoX:fix-fish-activation-on-windows-breaks-PATH
Jun 22, 2026
Merged

gh-151042: venv: Pass VIRTUAL_ENV through cygpath inside fish on Windows#151043
vsajip merged 2 commits into
python:mainfrom
LuNoX:fix-fish-activation-on-windows-breaks-PATH

Conversation

@LuNoX

@LuNoX LuNoX commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

gh-151042: Fixed venv exporting mixed path styles to PATH on Windows inside fish (via Cygwin, MinGW or MSYS2)

This PR mirrors the changes from gh-103325, gh-125399 and gh-112508 to the fish activation script.

@python-cla-bot

python-cla-bot Bot commented Jun 7, 2026

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@bedevere-app

bedevere-app Bot commented Jun 7, 2026

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.

@bedevere-app

bedevere-app Bot commented Jun 7, 2026

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.

@LuNoX LuNoX force-pushed the fix-fish-activation-on-windows-breaks-PATH branch from 43977d2 to afb462e Compare June 7, 2026 13:43
@bedevere-app

bedevere-app Bot commented Jun 7, 2026

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.

@LuNoX LuNoX force-pushed the fix-fish-activation-on-windows-breaks-PATH branch from 9ac5922 to 0694664 Compare June 7, 2026 16:08
@LuNoX

LuNoX commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

@vsajip @FFY00 the equivalent PR was merged in both uv and virtualenv if you would like to compare for review :)

@vsajip

vsajip commented Jun 21, 2026

Copy link
Copy Markdown
Member

This patch and the virtualenv patch have

if string match -qr 'CYGWIN|MSYS|MINGW' (uname)

whereas the uv patch has

if string match -qr 'CYGWIN|MSYS|MINGW' (uname); and command -s cygpath >/dev/null

Isn't the more stringent check needed? Under CYGWIN one might expect cygpath to be available, but is it guaranteed to be present under MSYS/MINGW?

@LuNoX

LuNoX commented Jun 21, 2026

Copy link
Copy Markdown
Contributor Author

This patch and the virtualenv patch have

if string match -qr 'CYGWIN|MSYS|MINGW' (uname)

whereas the uv patch has

if string match -qr 'CYGWIN|MSYS|MINGW' (uname); and command -s cygpath >/dev/null

Isn't the more stringent check needed? Under CYGWIN one might expect cygpath to be available, but is it guaranteed to be present under MSYS/MINGW?

@vsajip, it's not necessary. MSYS2 ships the Cygwin tool cygpath by default. MinGW as an environment requires MSYS2 to be installed. The codebase also uses cygpath unguarded at several points thus indicating that it is a requirement.

It's fine to add a guard clause as the uv devs requested, I don't see how it could hurt, but its also not required.

@vsajip vsajip merged commit 476b649 into python:main Jun 22, 2026
51 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants