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
This commit is contained in:
Zanie Blue 2024-06-23 11:05:48 -04:00 committed by GitHub
parent 2288ff7bf4
commit 03e2e6b99a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 55 additions and 15 deletions

View file

@ -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<OsString> {
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");
}

View file

@ -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