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 https://github.com/astral-sh/uv/issues/10089.
This commit is contained in:
Charlie Marsh 2025-01-06 17:41:43 -05:00 committed by GitHub
parent eaaf8896ed
commit 0d57d298e6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 180 additions and 62 deletions

View file

@ -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,17 +84,25 @@ impl CandidateSelector {
index: Option<&'a IndexUrl>,
env: &ResolverEnvironment,
) -> Option<Candidate<'a>> {
let is_excluded = exclusions.contains(package_name);
let reinstall = exclusions.reinstall(package_name);
let upgrade = exclusions.upgrade(package_name);
// Check for a preference from a lockfile or a previous fork that satisfies the range and
// is allowed.
// 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,
) {
@ -101,19 +110,52 @@ impl CandidateSelector {
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 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)
// 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 installed {} {} that satisfies {range}",
installed.name,
installed.version
);
return Some(installed);
}
}
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() {
[] => {}

View file

@ -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()
};
if let Upgrade::Packages(packages) = upgrade {
exclusions.extend(packages.into_keys());
};
if exclusions.is_empty() {
Self::None
} else {
Self::Some(exclusions)
}
}
Self { reinstall, upgrade }
}
pub fn reinstall(&self, package: &PackageName) -> bool {
self.reinstall.contains(package)
}
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)
}
}