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.

Sign up for GitHub

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

Jump to bottom

Inconsistent behavior when source .venv/Scripts/activate on Windows #1518

Closed
jfcherng opened this issue Feb 16, 2024 · 2 comments · Fixed by #1577
Closed

Inconsistent behavior when source .venv/Scripts/activate on Windows #1518

jfcherng opened this issue Feb 16, 2024 · 2 comments · Fixed by #1577
Labels
windows Specific to the Windows platform

Comments

@jfcherng
Copy link
Contributor

jfcherng commented Feb 16, 2024 •
edited
Loading

When in git-bash (https://gitforwindows.org/) shell on Windows,

With pip

python -m venv .venv
source .venv/Scripts/activate
echo $PATH  # ".venv/Scripts" is added into PATH

With uv

uv v
source .venv/Scripts/activate
echo $PATH  # ".venv/bin" is added into PATH, and this is wrong
@zanieb zanieb added the windows Specific to the Windows platform label Feb 16, 2024
@dhruvmanila dhruvmanila mentioned this issue Feb 17, 2024
Merged
dhruvmanila added a commit that referenced this issue Feb 17, 2024
@dhruvmanila
Consistent use of BIN_NAME in activation scripts (#1577)
e4389e5 
This PR fixes the bug where the `BIN_NAME` replacement field wasn't
being used in the activator scripts.

fixes: #1518 

## Test plan

As I don't have a Windows machine, I switched the `bin_name` value here
to point to `Scripts` on `unix` platform:


https://github.com/astral-sh/uv/blob/2a76c5908416b1e4c565349c86bc41a01515c8bf/crates/gourgeist/src/bare.rs#L99-L105

<details><summary>Code diff</summary>
<p>

```diff
```diff
diff --git a/crates/gourgeist/src/bare.rs b/crates/gourgeist/src/bare.rs
index 4c7808d3..0e0b41cf 100644
--- a/crates/gourgeist/src/bare.rs
+++ b/crates/gourgeist/src/bare.rs
@@ -97,9 +97,9 @@ pub fn create_bare_venv(location: &Utf8Path,
interpreter: &Interpreter) -> io::R
// TODO(konstin): I bet on windows we'll have to strip the prefix again
     let location = location.canonicalize_utf8()?;
     let bin_name = if cfg!(unix) {
-        "bin"
-    } else if cfg!(windows) {
         "Scripts"
+    } else if cfg!(windows) {
+        "bin"
     } else {
         unimplemented!("Only Windows and Unix are supported")
     };
```

</p>
</details> 

I then created the virtual environment as usual and tested out that the path modifications were correct:

```console
$ cargo run --bin uv -- venv
    Finished dev [unoptimized + debuginfo] target(s) in 0.13s
     Running `target/debug/uv venv`
Using Python 3.12.1 interpreter at
/Users/dhruv/.pyenv/versions/3.12.1/bin/python3.12
Creating virtualenv at: .venv

$ source .venv/Scripts/activate

$ echo $PATH
/Users/dhruv/work/astral/uv/.venv/Scripts:[...]

$ which python
/Users/dhruv/work/astral/uv/.venv/Scripts/python
```

I'm not sure how else to test this without having access to a Windows machine
@jfcherng
Copy link
Contributor Author

I can confirmed my issue is resolved in v0.1.4. Thanks!

@charliermarsh
Copy link
Member

Awesome, thank you for following up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
windows Specific to the Windows platform
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Consistent use of BIN_NAME in activation scripts astral-sh/uv
3 participants
@charliermarsh @zanieb @jfcherng