Skip to content

Commit

Permalink
Fix Python executable installation when multiple patch versions are r…
Browse files Browse the repository at this point in the history
…equested
  • Loading branch information
zanieb committed Dec 3, 2024
1 parent 75949f3 commit 13b6560
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 7 deletions.
35 changes: 28 additions & 7 deletions crates/uv/src/commands/python/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,11 @@ pub(crate) async fn install(
None
};

let installations: Vec<_> = downloaded.iter().chain(satisfied.iter().copied()).collect();

// Ensure that the installations are _complete_ for both downloaded installations and existing
// installations that match the request
for installation in downloaded.iter().chain(satisfied.iter().copied()) {
for installation in installations.iter() {
installation.ensure_externally_managed()?;
installation.ensure_canonical_executables()?;

Expand Down Expand Up @@ -353,7 +355,13 @@ pub(crate) async fn install(
);

// Figure out what installation it references, if any
let existing = find_matching_bin_link(&existing_installations, &target);
let existing = find_matching_bin_link(
installations
.iter()
.copied()
.chain(existing_installations.iter()),
&target,
);

match existing {
None => {
Expand All @@ -373,7 +381,7 @@ pub(crate) async fn install(
target.simplified_display()
);
}
Some(existing) if existing == installation => {
Some(existing) if existing == *installation => {
// The existing link points to the same installation, so we're done unless
// they requested we reinstall
if !(reinstall || force) {
Expand Down Expand Up @@ -429,6 +437,17 @@ pub(crate) async fn install(

// Replace the existing link
fs_err::remove_file(&to)?;

if let Some(existing) = existing {
// Ensure we do not report installation of this executable for an existing
// key if we undo it
changelog
.installed_executables
.entry(existing.key().clone())
.or_default()
.remove(&target);
}

installation.create_bin_link(&target)?;
debug!(
"Updated executable at `{}` to {}",
Expand Down Expand Up @@ -562,6 +581,10 @@ pub(crate) fn format_executables(
return String::new();
};

if installed.is_empty() {
return String::new();
}

let names = installed
.iter()
.filter_map(|path| path.file_name())
Expand Down Expand Up @@ -612,7 +635,7 @@ fn warn_if_not_on_path(bin: &Path) {
/// Like [`ManagedPythonInstallation::is_bin_link`], but this method will only resolve the
/// given path one time.
fn find_matching_bin_link<'a>(
installations: &'a [ManagedPythonInstallation],
mut installations: impl Iterator<Item = &'a ManagedPythonInstallation>,
path: &Path,
) -> Option<&'a ManagedPythonInstallation> {
let target = if cfg!(unix) {
Expand All @@ -630,7 +653,5 @@ fn find_matching_bin_link<'a>(
unreachable!("Only Windows and Unix are supported")
};

installations
.iter()
.find(|installation| installation.executable() == target)
installations.find(|installation| installation.executable() == target)
}
36 changes: 36 additions & 0 deletions crates/uv/tests/it/python_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,42 @@ fn python_install_preview() {

// The executable should be removed
bin_python.assert(predicate::path::missing());

// Install multiple patch versions
uv_snapshot!(context.filters(), context.python_install().arg("--preview").arg("3.12.7").arg("3.12.6"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Installed 2 versions in [TIME]
+ cpython-3.12.6-[PLATFORM]
+ cpython-3.12.7-[PLATFORM] (python3.12)
"###);

let bin_python = context
.temp_dir
.child("bin")
.child(format!("python3.12{}", std::env::consts::EXE_SUFFIX));

// The link should be for the newer patch version
if cfg!(unix) {
insta::with_settings!({
filters => context.filters(),
}, {
insta::assert_snapshot!(
read_link_path(&bin_python), @"[TEMP_DIR]/managed/cpython-3.12.7-[PLATFORM]/bin/python3.12"
);
});
} else {
insta::with_settings!({
filters => context.filters(),
}, {
insta::assert_snapshot!(
read_link_path(&bin_python), @"[TEMP_DIR]/managed/cpython-3.12.7-[PLATFORM]/python"
);
});
}
}

#[test]
Expand Down

0 comments on commit 13b6560

Please sign in to comment.