Skip to content

Commit

Permalink
Copy tool entrypoints on Windows instead of using a symbolic link (#4520
Browse files Browse the repository at this point in the history
)

This matches [pipx's
behavior](https://github.com/pypa/pipx/blob/4d8c05884317f7b646a66ffd4e933f4d1df471b7/src/pipx/commands/common.py#L69-L70).
Includes test changes to get past some blockers on Windows.

Resolves failures on #4509
  • Loading branch information
zanieb committed Jun 26, 2024
1 parent 6c8bb4c commit ec03575
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 37 deletions.
7 changes: 6 additions & 1 deletion crates/uv/src/commands/tool/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ use tracing::debug;
use uv_cache::Cache;
use uv_client::Connectivity;
use uv_configuration::{Concurrency, PreviewMode};
use uv_fs::{replace_symlink, Simplified};
#[cfg(unix)]
use uv_fs::replace_symlink;
use uv_fs::Simplified;
use uv_installer::SitePackages;
use uv_requirements::RequirementsSource;
use uv_tool::{entrypoint_paths, find_executable_directory, InstalledTools, Tool};
Expand Down Expand Up @@ -122,7 +124,10 @@ pub(crate) async fn install(
for (name, path) in entrypoints {
let target = executable_directory.join(path.file_name().unwrap());
debug!("Installing {name} to {}", target.user_display());
#[cfg(unix)]
replace_symlink(&path, &target).context("Failed to install entrypoint")?;
#[cfg(windows)]
fs_err::copy(&path, &target).context("Failed to install entrypoint")?;
}

debug!("Adding `{name}` to {}", path.user_display());
Expand Down
31 changes: 13 additions & 18 deletions crates/uv/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,24 +377,8 @@ impl TestContext {
/// For example, pinning package versions.
pub fn tool_install_without_exclude_newer(&self) -> std::process::Command {
let mut command = std::process::Command::new(get_bin());
command
.arg("tool")
.arg("install")
.arg("--cache-dir")
.arg(self.cache_dir.path())
.env("VIRTUAL_ENV", self.venv.as_os_str())
.env("UV_NO_WRAP", "1")
.env("HOME", self.home_dir.as_os_str())
.env("UV_TOOLCHAIN_DIR", "")
.env("UV_TEST_PYTHON_PATH", &self.python_path())
.current_dir(&self.temp_dir);

if cfg!(all(windows, debug_assertions)) {
// TODO(konstin): Reduce stack usage in debug mode enough that the tests pass with the
// default windows stack of 1MB
command.env("UV_STACK_SIZE", (4 * 1024 * 1024).to_string());
}

command.arg("tool").arg("install");
self.add_shared_args(&mut command);
command
}

Expand Down Expand Up @@ -677,6 +661,7 @@ pub fn python_toolchains_for_versions(

#[derive(Debug, Copy, Clone)]
pub enum WindowsFilters {
CachedPlatform,
Platform,
Universal,
}
Expand Down Expand Up @@ -757,6 +742,10 @@ pub fn run_and_format<T: AsRef<str>>(
WindowsFilters::Platform => {
["Resolved", "Prepared", "Installed", "Uninstalled"].iter()
}
// When cached, "Prepared" should not change.
WindowsFilters::CachedPlatform => {
["Resolved", "Installed", "Uninstalled"].iter()
}
WindowsFilters::Universal => {
["Prepared", "Installed", "Uninstalled"].iter()
}
Expand Down Expand Up @@ -850,6 +839,12 @@ macro_rules! uv_snapshot {
::insta::assert_snapshot!(snapshot, @$snapshot);
output
}};
($filters:expr, cached_windows_filters=true, $spawnable:expr, @$snapshot:literal) => {{
// Take a reference for backwards compatibility with the vec-expecting insta filters.
let (snapshot, output) = $crate::common::run_and_format($spawnable, &$filters, function_name!(), Some($crate::common::WindowsFilters::CachedPlatform));
::insta::assert_snapshot!(snapshot, @$snapshot);
output
}};
}

/// <https://stackoverflow.com/a/31749071/3549270>
Expand Down
69 changes: 51 additions & 18 deletions crates/uv/tests/tool_install.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#![cfg(all(feature = "python", feature = "pypi"))]

use std::process::Command;

use assert_fs::{
assert::PathAssert,
fixture::{FileTouch, PathChild},
Expand Down Expand Up @@ -47,6 +49,8 @@ fn tool_install() {
let executable = bin_dir.child(format!("black{}", std::env::consts::EXE_SUFFIX));
assert!(executable.exists());

// On Windows, we can't snapshot an executable file.
#[cfg(not(windows))]
insta::with_settings!({
filters => context.filters(),
}, {
Expand Down Expand Up @@ -74,56 +78,80 @@ fn tool_install() {
"###);
});

uv_snapshot!(context.filters(), Command::new("black").arg("--version").env("PATH", bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
black, 24.3.0 (compiled: yes)
Python (CPython) 3.12.[X]
----- stderr -----
"###);

// Install another tool
uv_snapshot!(context.filters(), context.tool_install()
.arg("pytest")
uv_snapshot!(context.filters(), cached_windows_filters=true, context.tool_install()
.arg("flask")
.env("UV_TOOL_DIR", tool_dir.as_os_str())
// Check that we support `XDG_BIN_HOME`
.env("XDG_BIN_HOME", bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `uv tool install` is experimental and may change without warning.
Resolved 4 packages in [TIME]
Prepared 3 packages in [TIME]
Installed 4 packages in [TIME]
+ iniconfig==2.0.0
+ packaging==24.0
+ pluggy==1.4.0
+ pytest==8.1.1
Resolved 7 packages in [TIME]
Prepared 6 packages in [TIME]
Installed 7 packages in [TIME]
+ blinker==1.7.0
+ click==8.1.7
+ flask==3.0.2
+ itsdangerous==2.1.2
+ jinja2==3.1.3
+ markupsafe==2.1.5
+ werkzeug==3.0.1
"###);

tool_dir.child("pytest").assert(predicate::path::is_dir());
tool_dir.child("flask").assert(predicate::path::is_dir());
assert!(bin_dir
.child(format!("pytest{}", std::env::consts::EXE_SUFFIX))
.child(format!("flask{}", std::env::consts::EXE_SUFFIX))
.exists());

#[cfg(not(windows))]
insta::with_settings!({
filters => context.filters(),
}, {
assert_snapshot!(fs_err::read_to_string(bin_dir.join("pytest")).unwrap(), @r###"
#![TEMP_DIR]/tools/pytest/bin/python
assert_snapshot!(fs_err::read_to_string(bin_dir.join("flask")).unwrap(), @r###"
#![TEMP_DIR]/tools/flask/bin/python
# -*- coding: utf-8 -*-
import re
import sys
from pytest import console_main
from flask.cli import main
if __name__ == "__main__":
sys.argv[0] = re.sub(r"(-script\.pyw|\.exe)?$", "", sys.argv[0])
sys.exit(console_main())
sys.exit(main())
"###);

});

uv_snapshot!(context.filters(), Command::new("flask").arg("--version").env("PATH", bin_dir.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
Python 3.12.[X]
Flask 3.0.2
Werkzeug 3.0.1
----- stderr -----
"###);

insta::with_settings!({
filters => context.filters(),
}, {
// We should have an additional tool entry
assert_snapshot!(fs_err::read_to_string(tool_dir.join("tools.toml")).unwrap(), @r###"
[tools]
black = { requirements = ["black"] }
pytest = { requirements = ["pytest"] }
flask = { requirements = ["flask"] }
"###);
});
}
Expand Down Expand Up @@ -165,6 +193,8 @@ fn tool_install_twice() {
let executable = bin_dir.child(format!("black{}", std::env::consts::EXE_SUFFIX));
assert!(executable.exists());

// On Windows, we can't snapshot an executable file.
#[cfg(not(windows))]
insta::with_settings!({
filters => context.filters(),
}, {
Expand Down Expand Up @@ -273,6 +303,9 @@ fn tool_install_entry_point_exists() {
}

/// Test `uv tool install` when the bin directory is inferred from `$HOME`
///
/// Only tested on Linux right now because it's not clear how to change the %USERPROFILE% on Windows
#[cfg(unix)]
#[test]
fn tool_install_home() {
let context = TestContext::new("3.12");
Expand Down Expand Up @@ -334,7 +367,7 @@ fn tool_install_xdg_data_home() {

context
.temp_dir
.child("data/bin/black")
.child(format!("data/bin/black{}", std::env::consts::EXE_SUFFIX))
.assert(predicate::path::exists());
}

Expand Down

0 comments on commit ec03575

Please sign in to comment.