Avoid downgrading packages when --upgrade is provided (#10097)

## Summary

When `--upgrade` is provided, we should retain already-installed
packages _if_ they're newer than whatever is available from the
registry.

Closes #10089.

crates/uv-configuration/src/package_options.rs

@@ -47,6 +47,15 @@ impl Reinstall {
         matches!(self, Self::All)
     }
 
+    /// Returns `true` if the specified package should be reinstalled.
+    pub fn contains(&self, package_name: &PackageName) -> bool {
+        match &self {
+            Self::None => false,
+            Self::All => true,
+            Self::Packages(packages) => packages.contains(package_name),
+        }
+    }
+
     /// Combine a set of [`Reinstall`] values.
     #[must_use]
     pub fn combine(self, other: Self) -> Self {

crates/uv-installer/src/plan.rs

@@ -66,11 +66,7 @@ impl<'a> Planner<'a> {
 
         for dist in self.resolution.distributions() {
             // Check if the package should be reinstalled.
-            let reinstall = match reinstall {
-                Reinstall::None => false,
-                Reinstall::All => true,
-                Reinstall::Packages(packages) => packages.contains(dist.name()),
-            };
+            let reinstall = reinstall.contains(dist.name());
 
             // Check if installation of a binary version of the package should be allowed.
             let no_binary = build_options.no_binary_package(dist.name());

crates/uv-requirements/src/upgrade.rs

@@ -68,6 +68,11 @@ pub fn read_lock_requirements(
     install_path: &Path,
     upgrade: &Upgrade,
 ) -> Result<LockedRequirements, LockError> {
+    // As an optimization, skip iterating over the lockfile is we're upgrading all packages anyway.
+    if upgrade.is_all() {
+        return Ok(LockedRequirements::default());
+    }
+
     let mut preferences = Vec::new();
     let mut git = Vec::new();
 

crates/uv-resolver/src/candidate_selector.rs

@@ -1,6 +1,7 @@
+use std::fmt::{Display, Formatter};
+
 use itertools::Itertools;
 use pubgrub::Range;
-use std::fmt::{Display, Formatter};
 use tracing::{debug, trace};
 
 use uv_configuration::IndexStrategy;
@@ -83,37 +84,78 @@ impl CandidateSelector {
         index: Option<&'a IndexUrl>,
         env: &ResolverEnvironment,
     ) -> Option<Candidate<'a>> {
-        let is_excluded = exclusions.contains(package_name);
-
-        // Check for a preference from a lockfile or a previous fork that satisfies the range and
-        // is allowed.
+        let reinstall = exclusions.reinstall(package_name);
+        let upgrade = exclusions.upgrade(package_name);
+
+        // If we have a preference (e.g., from a lockfile), search for a version matching that
+        // preference.
+        //
+        // If `--reinstall` is provided, we should omit any already-installed packages from here,
+        // since we can't reinstall already-installed packages.
+        //
+        // If `--upgrade` is provided, we should still search for a matching preference. In
+        // practice, preferences should be empty if `--upgrade` is provided, but it's the caller's
+        // responsibility to ensure that.
         if let Some(preferred) = self.get_preferred(
             package_name,
             range,
             version_maps,
             preferences,
             installed_packages,
-            is_excluded,
+            reinstall,
             index,
             env,
         ) {
             trace!("Using preference {} {}", preferred.name, preferred.version);
             return Some(preferred);
         }
 
-        // Check for a locally installed distribution that satisfies the range and is allowed.
-        if !is_excluded {
-            if let Some(installed) = Self::get_installed(package_name, range, installed_packages) {
+        // If we don't have a preference, find an already-installed distribution that satisfies the
+        // range.
+        let installed = if reinstall {
+            None
+        } else {
+            Self::get_installed(package_name, range, installed_packages)
+        };
+
+        // If we're not upgrading, we should prefer the already-installed distribution.
+        if !upgrade {
+            if let Some(installed) = installed {
+                trace!(
+                    "Using installed {} {} that satisfies {range}",
+                    installed.name,
+                    installed.version
+                );
+                return Some(installed);
+            }
+        }
+
+        // Otherwise, find the best candidate from the version maps.
+        let compatible = self.select_no_preference(package_name, range, version_maps, env);
+
+        // Cross-reference against the already-installed distribution.
+        //
+        // If the already-installed version is _more_ compatible than the best candidate
+        // from the version maps, use the installed version.
+        if let Some(installed) = installed {
+            if compatible.as_ref().is_none_or(|compatible| {
+                let highest = self.use_highest_version(package_name, env);
+                if highest {
+                    installed.version() >= compatible.version()
+                } else {
+                    installed.version() <= compatible.version()
+                }
+            }) {
                 trace!(
-                    "Using preference {} {} from installed package",
+                    "Using installed {} {} that satisfies {range}",
                     installed.name,
-                    installed.version,
+                    installed.version
                 );
                 return Some(installed);
             }
         }
 
-        self.select_no_preference(package_name, range, version_maps, env)
+        compatible
     }
 
     /// If the package has a preference, an existing version from an existing lockfile or a version
@@ -132,7 +174,7 @@ impl CandidateSelector {
         version_maps: &'a [VersionMap],
         preferences: &'a Preferences,
         installed_packages: &'a InstalledPackages,
-        is_excluded: bool,
+        reinstall: bool,
         index: Option<&'a IndexUrl>,
         env: &ResolverEnvironment,
     ) -> Option<Candidate<'a>> {
@@ -159,7 +201,7 @@ impl CandidateSelector {
             range,
             version_maps,
             installed_packages,
-            is_excluded,
+            reinstall,
             env,
         )
     }
@@ -172,7 +214,7 @@ impl CandidateSelector {
         range: &Range<Version>,
         version_maps: &'a [VersionMap],
         installed_packages: &'a InstalledPackages,
-        is_excluded: bool,
+        reinstall: bool,
         env: &ResolverEnvironment,
     ) -> Option<Candidate<'a>> {
         for (marker, version) in preferences {
@@ -181,8 +223,9 @@ impl CandidateSelector {
                 continue;
             }
 
-            // Check for a locally installed distribution that matches the preferred version.
-            if !is_excluded {
+            // Check for a locally installed distribution that matches the preferred version, unless
+            // we have to reinstall, in which case we can't reuse an already-installed distribution.
+            if !reinstall {
                 let installed_dists = installed_packages.get_packages(package_name);
                 match installed_dists.as_slice() {
                     [] => {}

crates/uv-resolver/src/exclusions.rs

@@ -1,48 +1,27 @@
-use rustc_hash::FxHashSet;
 use uv_configuration::{Reinstall, Upgrade};
 use uv_pep508::PackageName;
 
 /// Tracks locally installed packages that should not be selected during resolution.
 #[derive(Debug, Default, Clone)]
-pub enum Exclusions {
-    #[default]
-    None,
-    /// Exclude some local packages from consideration, e.g. from `--reinstall-package foo --upgrade-package bar`
-    Some(FxHashSet<PackageName>),
-    /// Exclude all local packages from consideration, e.g. from `--reinstall` or `--upgrade`
-    All,
+pub struct Exclusions {
+    reinstall: Reinstall,
+    upgrade: Upgrade,
 }
 
 impl Exclusions {
     pub fn new(reinstall: Reinstall, upgrade: Upgrade) -> Self {
-        if upgrade.is_all() || reinstall.is_all() {
-            Self::All
-        } else {
-            let mut exclusions: FxHashSet<PackageName> =
-                if let Reinstall::Packages(packages) = reinstall {
-                    FxHashSet::from_iter(packages)
-                } else {
-                    FxHashSet::default()
-                };
+        Self { reinstall, upgrade }
+    }
 
-            if let Upgrade::Packages(packages) = upgrade {
-                exclusions.extend(packages.into_keys());
-            };
+    pub fn reinstall(&self, package: &PackageName) -> bool {
+        self.reinstall.contains(package)
+    }
 
-            if exclusions.is_empty() {
-                Self::None
-            } else {
-                Self::Some(exclusions)
-            }
-        }
+    pub fn upgrade(&self, package: &PackageName) -> bool {
+        self.upgrade.contains(package)
     }
 
-    /// Returns true if the package is excluded and a local distribution should not be used.
     pub fn contains(&self, package: &PackageName) -> bool {
-        match self {
-            Self::None => false,
-            Self::Some(packages) => packages.contains(package),
-            Self::All => true,
-        }
+        self.reinstall(package) || self.upgrade(package)
     }
 }

crates/uv/tests/it/pip_install.rs

@@ -2617,6 +2617,93 @@ fn no_deps_editable() {
     context.assert_command("import aiohttp").failure();
 }
 
+/// Avoid downgrading already-installed packages when `--upgrade` is provided.
+#[test]
+fn install_no_downgrade() -> Result<()> {
+    let context = TestContext::new("3.12");
+
+    // Create a local package named `idna`.
+    let idna = context.temp_dir.child("idna");
+    idna.child("pyproject.toml").write_str(indoc! {r#"
+        [project]
+        name = "idna"
+        version = "1000"
+        requires-python = ">=3.12"
+        dependencies = []
+
+        [build-system]
+        requires = ["setuptools>=42"]
+        build-backend = "setuptools.build_meta"
+    "#})?;
+
+    // Install the local `idna`.
+    uv_snapshot!(context.filters(), context.pip_install()
+        .arg("./idna"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 1 package in [TIME]
+    Prepared 1 package in [TIME]
+    Installed 1 package in [TIME]
+     + idna==1000 (from file://[TEMP_DIR]/idna)
+    "###
+    );
+
+    // Install `anyio`, which depends on `idna`.
+    uv_snapshot!(context.filters(), context.pip_install()
+        .arg("anyio"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 3 packages in [TIME]
+    Prepared 2 packages in [TIME]
+    Installed 2 packages in [TIME]
+     + anyio==4.3.0
+     + sniffio==1.3.1
+    "###
+    );
+
+    // Install `anyio` with `--upgrade`, which should retain the local `idna`.
+    uv_snapshot!(context.filters(), context.pip_install()
+        .arg("-U")
+        .arg("anyio"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 3 packages in [TIME]
+    Audited 3 packages in [TIME]
+    "###
+    );
+
+    // Install `anyio` with `--reinstall`, which should downgrade `idna`.
+    uv_snapshot!(context.filters(), context.pip_install()
+        .arg("--reinstall")
+        .arg("anyio"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 3 packages in [TIME]
+    Prepared 3 packages in [TIME]
+    Uninstalled 3 packages in [TIME]
+    Installed 3 packages in [TIME]
+     ~ anyio==4.3.0
+     - idna==1000 (from file://[TEMP_DIR]/idna)
+     + idna==3.6
+     ~ sniffio==1.3.1
+    "###
+    );
+
+    Ok(())
+}
+
 /// Upgrade a package.
 #[test]
 fn install_upgrade() {
@@ -5311,14 +5398,13 @@ fn already_installed_local_path_dependent() {
         .arg("--no-index")
         .arg("--find-links")
         .arg(build_vendor_links_url()), @r###"
-    success: false
-    exit_code: 1
+    success: true
+    exit_code: 0
     ----- stdout -----
 
     ----- stderr -----
-      × No solution found when resolving dependencies:
-      ╰─▶ Because first-local was not found in the provided package locations and second-local==0.1.0 depends on first-local, we can conclude that second-local==0.1.0 cannot be used.
-          And because only second-local==0.1.0 is available and you require second-local, we can conclude that your requirements are unsatisfiable.
+    Resolved 2 packages in [TIME]
+    Audited 2 packages in [TIME]
     "###
     );
 
@@ -5477,8 +5563,8 @@ fn already_installed_local_version_of_remote_package() {
     ----- stdout -----
 
     ----- stderr -----
-    Resolved 3 packages in [TIME]
-    Audited 3 packages in [TIME]
+    Resolved 1 package in [TIME]
+    Audited 1 package in [TIME]
     "###
     );