From 03e2e6b99ab6e690ac2cccde439545a9828628cb Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Sun, 23 Jun 2024 11:05:48 -0400 Subject: [PATCH] Fix construction of Python path in test context (#4443) When executables were not named `python3` e.g. `python3.11` we would construct a Python path that would only work for _some_ requests in tests since we don't search for those names unless a specific version is requested. To solve, we construct a test context with constant Python executable names. For example, if a test context was created with `3.11` and `3.12` we could end up with the search path `/usr/local/python-3.11/bin:/usr/local/python-3.12/bin` where the executables are named `python3.11` and `python3` respectively. A test invocation of uv requesting any Python toolchain version would then locate the `3.12` executable since the `3.11` executable doesn't have the generic name, but we want `3.11` to come first. On Windows, we just leave things as-is because executables are always called `python.exe`. Closes https://github.com/astral-sh/uv/issues/4376 --- crates/uv/tests/common/mod.rs | 65 +++++++++++++++++++++++++++-------- crates/uv/tests/venv.rs | 5 ++- 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/crates/uv/tests/common/mod.rs b/crates/uv/tests/common/mod.rs index 323f9f3ac..89db09b14 100644 --- a/crates/uv/tests/common/mod.rs +++ b/crates/uv/tests/common/mod.rs @@ -4,7 +4,7 @@ use assert_cmd::assert::{Assert, OutputAssertExt}; use assert_cmd::Command; use assert_fs::assert::PathAssert; -use assert_fs::fixture::{ChildPath, PathChild}; +use assert_fs::fixture::{ChildPath, PathChild, PathCreateDir, SymlinkToFile}; use predicates::prelude::predicate; use regex::Regex; use std::borrow::BorrowMut; @@ -64,6 +64,7 @@ pub const INSTA_FILTERS: &[(&str, &str)] = &[ pub struct TestContext { pub temp_dir: assert_fs::TempDir, pub cache_dir: assert_fs::TempDir, + pub python_dir: assert_fs::TempDir, pub venv: ChildPath, pub workspace_root: PathBuf, @@ -97,6 +98,7 @@ impl TestContext { pub fn new_with_versions(python_versions: &[&str]) -> Self { let temp_dir = assert_fs::TempDir::new().expect("Failed to create temp dir"); let cache_dir = assert_fs::TempDir::new().expect("Failed to create cache dir"); + let python_dir = assert_fs::TempDir::new().expect("Failed to create test Python dir"); // Canonicalize the temp dir for consistent snapshot behavior let canonical_temp_dir = temp_dir.canonicalize().unwrap(); @@ -128,6 +130,16 @@ impl TestContext { ) .collect(); + // Construct directories for each Python executable on Unix where the executable names + // need to be normalized + if cfg!(unix) { + for (version, executable) in &python_versions { + let parent = python_dir.child(version.to_string()); + parent.create_dir_all().unwrap(); + parent.child("python3").symlink_to_file(executable).unwrap(); + } + } + let mut filters = Vec::new(); filters.extend( @@ -152,7 +164,14 @@ impl TestContext { filters.extend( Self::path_patterns(executable) .into_iter() - .map(|pattern| (format!("{pattern}python.*"), format!("[PYTHON-{version}]"))), + .map(|pattern| (pattern.to_string(), format!("[PYTHON-{version}]"))), + ); + + // And for the symlink we created in the test the Python path + filters.extend( + Self::path_patterns(&python_dir.join(version.to_string())) + .into_iter() + .map(|pattern| (format!("{pattern}.*"), format!("[PYTHON-{version}]"))), ); // Add Python patch version filtering unless explicitly requested to ensure @@ -169,6 +188,11 @@ impl TestContext { .into_iter() .map(|pattern| (pattern, "[TEMP_DIR]/".to_string())), ); + filters.extend( + Self::path_patterns(&python_dir) + .into_iter() + .map(|pattern| (pattern, "[PYTHON_DIR]/".to_string())), + ); filters.extend( Self::path_patterns(&workspace_root) .into_iter() @@ -206,6 +230,7 @@ impl TestContext { Self { temp_dir, cache_dir, + python_dir, venv, workspace_root, python_version, @@ -563,7 +588,23 @@ impl TestContext { } pub fn python_path(&self) -> OsString { - std::env::join_paths(self.python_versions.iter().map(|(_, path)| path)).unwrap() + if cfg!(unix) { + // On Unix, we needed to normalize the Python executable names to `python3` for the tests + std::env::join_paths( + self.python_versions + .iter() + .map(|(version, _)| self.python_dir.join(version.to_string())), + ) + .unwrap() + } else { + // On Windows, just join the parent directories of the executables + std::env::join_paths( + self.python_versions + .iter() + .map(|(_, executable)| executable.parent().unwrap().to_path_buf()), + ) + .unwrap() + } } /// Standard snapshot filters _plus_ those for this test context. @@ -693,13 +734,14 @@ pub fn python_path_with_versions( temp_dir: &assert_fs::TempDir, python_versions: &[&str], ) -> anyhow::Result { - Ok(std::env::join_paths(python_toolchains_for_versions( - temp_dir, - python_versions, - )?)?) + Ok(std::env::join_paths( + python_toolchains_for_versions(temp_dir, python_versions)? + .into_iter() + .map(|path| path.parent().unwrap().to_path_buf()), + )?) } -/// Create a `PATH` with the requested Python versions available in order. +/// Returns a list of Python executables for the given versions. /// /// Generally this should be used with `UV_TEST_PYTHON_PATH`. pub fn python_toolchains_for_versions( @@ -716,12 +758,7 @@ pub fn python_toolchains_for_versions( ToolchainPreference::PreferInstalledManaged, &cache, ) { - toolchain - .into_interpreter() - .sys_executable() - .parent() - .expect("Python executable should always be in a directory") - .to_path_buf() + toolchain.into_interpreter().sys_executable().to_owned() } else { panic!("Could not find Python {python_version} for test"); } diff --git a/crates/uv/tests/venv.rs b/crates/uv/tests/venv.rs index 4dedfedcf..4adacf612 100644 --- a/crates/uv/tests/venv.rs +++ b/crates/uv/tests/venv.rs @@ -511,7 +511,10 @@ fn windows_shims() -> Result<()> { fs_err::create_dir(&shim_path)?; fs_err::write( shim_path.child("python.bat"), - format!("@echo off\r\n{}/python.exe %*", py38.1.display()), + format!( + "@echo off\r\n{}/python.exe %*", + py38.1.parent().unwrap().display() + ), )?; // Create a virtual environment at `.venv` with the shim