mirror of https://github.com/astral-sh/uv
fix: Handle dotted package names in script path resolution (#15300)
<!-- Thank you for contributing to uv! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? - Does this pull request include references to any relevant issues? --> ## Summary Fix WindowsRunnable::from_script_path to correctly append extensions instead of replacing them when resolving executable paths. This resolves https://github.com/astral-sh/uv/issues/15165#issue-3304086689. - Add add_extension_to_path helper that appends extensions properly - Update extension resolution to use the new helper - Add tests ## Test Plan Added unit tests for the new and existing functionality that the change touches. Tested manually locally on Windows. <!-- How was it tested? --> --------- Co-authored-by: Zanie Blue <contact@zanie.dev>
This commit is contained in:
parent
191c9175fe
commit
9346b4d0f6
|
|
@ -6269,8 +6269,10 @@ name = "uv-shell"
|
|||
version = "0.0.1"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"fs-err",
|
||||
"home",
|
||||
"same-file",
|
||||
"tempfile",
|
||||
"tracing",
|
||||
"uv-fs",
|
||||
"uv-static",
|
||||
|
|
|
|||
|
|
@ -23,3 +23,7 @@ tracing = { workspace = true }
|
|||
windows-registry = { workspace = true }
|
||||
windows-result = { workspace = true }
|
||||
windows-sys = { workspace = true }
|
||||
|
||||
[dev-dependencies]
|
||||
fs-err = { workspace = true }
|
||||
tempfile = { workspace = true }
|
||||
|
|
|
|||
|
|
@ -2,9 +2,28 @@
|
|||
|
||||
use std::env::consts::EXE_EXTENSION;
|
||||
use std::ffi::OsStr;
|
||||
use std::path::Path;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::process::Command;
|
||||
|
||||
/// Append an extension to a [`PathBuf`].
|
||||
///
|
||||
/// Unlike [`Path::with_extension`], this function does not replace an existing extension.
|
||||
///
|
||||
/// If there is no file name, the path is returned unchanged.
|
||||
///
|
||||
/// This mimics the behavior of the unstable [`Path::with_added_extension`] method.
|
||||
fn add_extension_to_path(mut path: PathBuf, extension: &str) -> PathBuf {
|
||||
let Some(name) = path.file_name() else {
|
||||
// If there is no file name, we cannot add an extension.
|
||||
return path;
|
||||
};
|
||||
let mut name = name.to_os_string();
|
||||
name.push(".");
|
||||
name.push(extension.trim_start_matches('.'));
|
||||
path.set_file_name(name);
|
||||
path
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub enum WindowsRunnable {
|
||||
/// Windows PE (.exe)
|
||||
|
|
@ -90,7 +109,7 @@ impl WindowsRunnable {
|
|||
.map(|script_type| {
|
||||
(
|
||||
script_type,
|
||||
script_path.with_extension(script_type.to_extension()),
|
||||
add_extension_to_path(script_path.clone(), script_type.to_extension()),
|
||||
)
|
||||
})
|
||||
.find(|(_, script_path)| script_path.is_file())
|
||||
|
|
@ -98,3 +117,145 @@ impl WindowsRunnable {
|
|||
.unwrap_or_else(|| Command::new(runnable_name))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::path::PathBuf;
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
use fs_err as fs;
|
||||
#[cfg(target_os = "windows")]
|
||||
use std::io;
|
||||
|
||||
#[test]
|
||||
fn test_add_extension_to_path() {
|
||||
// Test with simple package name (no dots)
|
||||
let path = PathBuf::from("python");
|
||||
let result = add_extension_to_path(path, "exe");
|
||||
assert_eq!(result, PathBuf::from("python.exe"));
|
||||
|
||||
// Test with package name containing single dot
|
||||
let path = PathBuf::from("awslabs.cdk-mcp-server");
|
||||
let result = add_extension_to_path(path, "exe");
|
||||
assert_eq!(result, PathBuf::from("awslabs.cdk-mcp-server.exe"));
|
||||
|
||||
// Test with package name containing multiple dots
|
||||
let path = PathBuf::from("org.example.tool");
|
||||
let result = add_extension_to_path(path, "exe");
|
||||
assert_eq!(result, PathBuf::from("org.example.tool.exe"));
|
||||
|
||||
// Test with different extensions
|
||||
let path = PathBuf::from("script");
|
||||
let result = add_extension_to_path(path, "ps1");
|
||||
assert_eq!(result, PathBuf::from("script.ps1"));
|
||||
|
||||
// Test with path that has directory components
|
||||
let path = PathBuf::from("some/path/to/awslabs.cdk-mcp-server");
|
||||
let result = add_extension_to_path(path, "exe");
|
||||
assert_eq!(
|
||||
result,
|
||||
PathBuf::from("some/path/to/awslabs.cdk-mcp-server.exe")
|
||||
);
|
||||
|
||||
// Test with empty path (edge case)
|
||||
let path = PathBuf::new();
|
||||
let result = add_extension_to_path(path.clone(), "exe");
|
||||
assert_eq!(result, path); // Should return unchanged
|
||||
}
|
||||
|
||||
/// Helper function to create a temporary directory with test files
|
||||
#[cfg(target_os = "windows")]
|
||||
fn create_test_environment() -> io::Result<tempfile::TempDir> {
|
||||
let temp_dir = tempfile::tempdir()?;
|
||||
let scripts_dir = temp_dir.path().join("Scripts");
|
||||
fs::create_dir_all(&scripts_dir)?;
|
||||
|
||||
// Create test executable files
|
||||
fs::write(scripts_dir.join("python.exe"), "")?;
|
||||
fs::write(scripts_dir.join("awslabs.cdk-mcp-server.exe"), "")?;
|
||||
fs::write(scripts_dir.join("org.example.tool.exe"), "")?;
|
||||
fs::write(scripts_dir.join("multi.dot.package.name.exe"), "")?;
|
||||
fs::write(scripts_dir.join("script.ps1"), "")?;
|
||||
fs::write(scripts_dir.join("batch.bat"), "")?;
|
||||
fs::write(scripts_dir.join("command.cmd"), "")?;
|
||||
fs::write(scripts_dir.join("explicit.ps1"), "")?;
|
||||
|
||||
Ok(temp_dir)
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
#[test]
|
||||
fn test_from_script_path_single_dot_package() {
|
||||
let temp_dir = create_test_environment().expect("Failed to create test environment");
|
||||
let scripts_dir = temp_dir.path().join("Scripts");
|
||||
|
||||
// Test package name with single dot (awslabs.cdk-mcp-server)
|
||||
let command =
|
||||
WindowsRunnable::from_script_path(&scripts_dir, OsStr::new("awslabs.cdk-mcp-server"));
|
||||
|
||||
// The command should be constructed with the correct executable path
|
||||
let expected_path = scripts_dir.join("awslabs.cdk-mcp-server.exe");
|
||||
assert_eq!(command.get_program(), expected_path.as_os_str());
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
#[test]
|
||||
fn test_from_script_path_multiple_dots_package() {
|
||||
let temp_dir = create_test_environment().expect("Failed to create test environment");
|
||||
let scripts_dir = temp_dir.path().join("Scripts");
|
||||
|
||||
// Test package name with multiple dots (org.example.tool)
|
||||
let command =
|
||||
WindowsRunnable::from_script_path(&scripts_dir, OsStr::new("org.example.tool"));
|
||||
|
||||
let expected_path = scripts_dir.join("org.example.tool.exe");
|
||||
assert_eq!(command.get_program(), expected_path.as_os_str());
|
||||
|
||||
// Test another multi-dot package
|
||||
let command =
|
||||
WindowsRunnable::from_script_path(&scripts_dir, OsStr::new("multi.dot.package.name"));
|
||||
|
||||
let expected_path = scripts_dir.join("multi.dot.package.name.exe");
|
||||
assert_eq!(command.get_program(), expected_path.as_os_str());
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
#[test]
|
||||
fn test_from_script_path_simple_package_name() {
|
||||
let temp_dir = create_test_environment().expect("Failed to create test environment");
|
||||
let scripts_dir = temp_dir.path().join("Scripts");
|
||||
|
||||
// Test simple package name without dots
|
||||
let command = WindowsRunnable::from_script_path(&scripts_dir, OsStr::new("python"));
|
||||
|
||||
let expected_path = scripts_dir.join("python.exe");
|
||||
assert_eq!(command.get_program(), expected_path.as_os_str());
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
#[test]
|
||||
fn test_from_script_path_explicit_extensions() {
|
||||
let temp_dir = create_test_environment().expect("Failed to create test environment");
|
||||
let scripts_dir = temp_dir.path().join("Scripts");
|
||||
|
||||
// Test explicit .ps1 extension
|
||||
let command = WindowsRunnable::from_script_path(&scripts_dir, OsStr::new("explicit.ps1"));
|
||||
|
||||
let expected_path = scripts_dir.join("explicit.ps1");
|
||||
assert_eq!(command.get_program(), "powershell");
|
||||
|
||||
// Verify the arguments contain the script path
|
||||
let args: Vec<&OsStr> = command.get_args().collect();
|
||||
assert!(args.contains(&OsStr::new("-File")));
|
||||
assert!(args.contains(&expected_path.as_os_str()));
|
||||
|
||||
// Test explicit .bat extension
|
||||
let command = WindowsRunnable::from_script_path(&scripts_dir, OsStr::new("batch.bat"));
|
||||
assert_eq!(command.get_program(), "cmd");
|
||||
|
||||
// Test explicit .cmd extension
|
||||
let command = WindowsRunnable::from_script_path(&scripts_dir, OsStr::new("command.cmd"));
|
||||
assert_eq!(command.get_program(), "cmd");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3065,3 +3065,42 @@ fn tool_run_reresolve_python() -> anyhow::Result<()> {
|
|||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Test that Windows executable resolution works correctly for package names with dots.
|
||||
/// This test verifies the fix for the bug where package names containing dots were
|
||||
/// incorrectly handled when adding Windows executable extensions.
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn tool_run_windows_dotted_package_name() -> anyhow::Result<()> {
|
||||
let context = TestContext::new("3.12").with_filtered_counts();
|
||||
let tool_dir = context.temp_dir.child("tools");
|
||||
let bin_dir = context.temp_dir.child("bin");
|
||||
|
||||
// Copy the test package to a temporary location
|
||||
let workspace_packages = context.workspace_root.join("scripts").join("packages");
|
||||
let test_package_source = workspace_packages.join("package.name.with.dots");
|
||||
let test_package_dest = context.temp_dir.child("package.name.with.dots");
|
||||
|
||||
copy_dir_all(&test_package_source, &test_package_dest)?;
|
||||
|
||||
// Test that uv tool run can find and execute the dotted package name
|
||||
uv_snapshot!(context.filters(), context.tool_run()
|
||||
.arg("--from")
|
||||
.arg(test_package_dest.path())
|
||||
.arg("package.name.with.dots")
|
||||
.env(EnvVars::UV_TOOL_DIR, tool_dir.as_os_str())
|
||||
.env(EnvVars::XDG_BIN_HOME, bin_dir.as_os_str()), @r###"
|
||||
success: true
|
||||
exit_code: 0
|
||||
----- stdout -----
|
||||
package.name.with.dots version 0.1.0
|
||||
|
||||
----- stderr -----
|
||||
Resolved [N] packages in [TIME]
|
||||
Prepared [N] packages in [TIME]
|
||||
Installed [N] packages in [TIME]
|
||||
+ package-name-with-dots==0.1.0 (from file://[TEMP_DIR]/package.name.with.dots)
|
||||
"###);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,6 @@
|
|||
# package.name.with.dots
|
||||
|
||||
Test package for verifying Windows executable handling with dotted package names.
|
||||
|
||||
This package is used to test the fix for the uvx Windows executable bug where package names
|
||||
containing dots were incorrectly handled when adding Windows executable extensions.
|
||||
|
|
@ -0,0 +1,11 @@
|
|||
[project]
|
||||
name = "package.name.with.dots"
|
||||
version = "0.1.0"
|
||||
requires-python = ">=3.8"
|
||||
|
||||
[tool.uv.build-backend.data]
|
||||
scripts = "scripts"
|
||||
|
||||
[build-system]
|
||||
requires = ["uv_build>=0.8.0,<0.9"]
|
||||
build-backend = "uv_build"
|
||||
|
|
@ -0,0 +1,2 @@
|
|||
Write-Host "package.name.with.dots version 0.1.0"
|
||||
exit 0
|
||||
|
|
@ -0,0 +1,3 @@
|
|||
"""Test package for verifying Windows executable handling with dotted package names."""
|
||||
|
||||
__version__ = "0.1.0"
|
||||
Loading…
Reference in New Issue