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

Escape Windows paths with spaces in venv activation command #2223

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

charliermarsh
Copy link
Member

Summary

Ensure that we print & "foo bar\Scripts\activate" if necessary.

@charliermarsh charliermarsh added the windows Specific to the Windows platform label Mar 5, 2024
@charliermarsh charliermarsh marked this pull request as ready for review March 5, 2024 22:53
@charliermarsh charliermarsh merged commit 65518c9 into main Mar 5, 2024
7 checks passed
@charliermarsh charliermarsh deleted the charlie/esc branch March 5, 2024 23:11
@samypr100
Copy link
Contributor

samypr100 commented Mar 6, 2024

@charliermarsh In cmd.exe it would just be "foo bar\Scripts\activate" instead of & "foo bar\Scripts\activate", so maybe doing something like
format!("& \"{executable}\" (PS) or \"{executable}\" (CMD)") would suffice given shell detection on windows is not very friendly?

@charliermarsh
Copy link
Member Author

Yeah I can't figure out how to detect if it's PowerShell or CMD... Do you know of any way to do it?

@samypr100
Copy link
Contributor

samypr100 commented Mar 6, 2024

From an env perspective? Not that I'm aware of unfortunately. I did see some code rely on PSModulePath for powershell detection by parsing it's value and checking it contains more than 3 entries (see https://stackoverflow.com/a/55598796)
which basically does is_power_shell = len(os.getenv('PSModulePath', '').split(os.pathsep)) >= 3 .

It's heuristic because a user could potentially modify PSModulePath to their system environment variables and it would break this detection, but without modifications it would work since PowerShell itself modifies PSModulePath on launch per docs and always prefixes three or more entries on startup (regardless of system env) since it's a special PowerShell variable.

@samypr100
Copy link
Contributor

Thinking about the oposite angle (which might be simpler), an asnwer on the same SO also mentions cmd.exe always defines PROMPT where as PowerShell does not, so maybe we can rely on that to detect cmd.exe and fallback to PS? https://stackoverflow.com/a/66415037

@samypr100
Copy link
Contributor

Example implementation of last comment in #2226

charliermarsh pushed a commit that referenced this pull request Mar 6, 2024
## Summary

Follow up from discussion in #2223

Detect CMD.exe by checking if `PROMPT` env var is set on windows,
otherwise assume it's PowerShell.

Note, this will not work if user modifies their system env vars to
include `PROMPT` by default or if they launch nested PowerShell from
Command Prompt (e.g. `Developer PowerShell for VS 2022`).

## Test Plan

Only tested locally, although we try to add some CI tests that
specifically use CMD.exe

Command Prompt
```
Microsoft Windows [Version 10.0.19044.3086]
(c) Microsoft Corporation. All rights reserved.

Z:\Users\samypr100\dev\uv>Z:\Users\samypr100\.cargo\bin\cargo.exe +stable run --color=always -- venv "Foo Bar"
    Finished dev [unoptimized + debuginfo] target(s) in 0.69s
     Running `target\debug\uv.exe venv "Foo Bar"`
Using Python 3.12.2 interpreter at: Z:\Users\samypr100\AppData\Local\Programs\Python\Python312\python.exe
Creating virtualenv at: Foo Bar
Activate with: "Foo Bar\Scripts\activate"
```

Power Shell
```
Windows PowerShell
Copyright (C) Microsoft Corporation. All rights reserved.

Try the new cross-platform PowerShell https://aka.ms/pscore6

PS Z:\Users\samypr100\dev\uv>Z:\Users\samypr100\.cargo\bin\cargo.exe +stable run --color=always -- venv "Foo Bar"
    Finished dev [unoptimized + debuginfo] target(s) in 0.63s
     Running `target\debug\uv.exe venv "Foo Bar"`
Using Python 3.12.2 interpreter at: Z:\Users\samypr100\AppData\Local\Programs\Python\Python312\python.exe
Creating virtualenv at: Foo Bar
Activate with: & "Foo Bar\Scripts\activate"
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
windows Specific to the Windows platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants