Remove `UpgradeSelection` struct (#15422)

## Summary After #15395, I realized that we didn't actually need a separate struct for this since we now pass it around as an `Option`. (The key change from #15395 is that when combining, we treat the options as a single unit.)
2025-08-21 19:35:07 +01:00 · 2025-08-21 19:35:07 +01:00 · e4de538dae
parent 11633549fd
commit e4de538dae
6 changed files with 66 additions and 116 deletions
--- a/crates/uv-cli/src/options.rs
+++ b/crates/uv-cli/src/options.rs
@ -1,7 +1,7 @@
 use anstream::eprintln;

 use uv_cache::Refresh;
-use uv_configuration::UpgradeSelection;
+use uv_configuration::Upgrade;
 use uv_distribution_types::{ConfigSettings, PackageConfigSettings, Requirement};
 use uv_resolver::{ExcludeNewer, ExcludeNewerPackage, PrereleaseMode};
 use uv_settings::{Combine, PipOptions, ResolverInstallerOptions, ResolverOptions};
@ -334,7 +334,7 @@ pub fn resolver_options(
                .filter_map(Maybe::into_option)
                .collect()
        }),
-        upgrade: UpgradeSelection::from_args(
+        upgrade: Upgrade::from_args(
            flag(upgrade, no_upgrade, "no-upgrade"),
            upgrade_package.into_iter().map(Requirement::from).collect(),
        ),
@ -445,7 +445,7 @@ pub fn resolver_installer_options(
                .filter_map(Maybe::into_option)
                .collect()
        }),
-        upgrade: UpgradeSelection::from_args(
+        upgrade: Upgrade::from_args(
            flag(upgrade, no_upgrade, "upgrade"),
            upgrade_package.into_iter().map(Requirement::from).collect(),
        ),
--- a/crates/uv-configuration/src/package_options.rs
+++ b/crates/uv-configuration/src/package_options.rs
@ -134,55 +134,6 @@ impl From<Reinstall> for Refresh {
    }
 }

-/// An upgrade selection as specified by a user on the command line or in a configuration file.
-#[derive(Debug, Default, Clone)]
-pub enum UpgradeSelection {
-    /// Prefer pinned versions from the existing lockfile, if possible.
-    #[default]
-    None,
-
-    /// Allow package upgrades for all packages, ignoring the existing lockfile.
-    All,
-
-    /// Allow package upgrades, but only for the specified packages.
-    Packages(Vec<Requirement>),
-}
-
-impl UpgradeSelection {
-    /// Determine the upgrade selection strategy from the command-line arguments.
-    pub fn from_args(upgrade: Option<bool>, upgrade_package: Vec<Requirement>) -> Option<Self> {
-        match upgrade {
-            Some(true) => Some(Self::All),
-            // TODO(charlie): `--no-upgrade` with `--upgrade-package` should allow the specified
-            // packages to be upgraded. Right now, `--upgrade-package` is silently ignored.
-            Some(false) => Some(Self::None),
-            None if upgrade_package.is_empty() => None,
-            None => Some(Self::Packages(upgrade_package)),
-        }
-    }
-
-    /// Combine a set of [`UpgradeSelection`] values.
-    #[must_use]
-    pub fn combine(self, other: Self) -> Self {
-        match self {
-            // Setting `--upgrade` or `--no-upgrade` should clear previous `--upgrade-package` selections.
-            Self::All | Self::None => self,
-            Self::Packages(self_packages) => match other {
-                // If `--upgrade` was enabled previously, `--upgrade-package` is subsumed by upgrading all packages.
-                Self::All => other,
-                // If `--no-upgrade` was enabled previously, then `--upgrade-package` enables an explicit upgrade of those packages.
-                Self::None => Self::Packages(self_packages),
-                // If `--upgrade-package` was included twice, combine the requirements.
-                Self::Packages(other_packages) => {
-                    let mut combined = self_packages;
-                    combined.extend(other_packages);
-                    Self::Packages(combined)
-                }
-            },
-        }
-    }
-}
-
 /// Whether to allow package upgrades.
 #[derive(Debug, Default, Clone)]
 pub enum Upgrade {
@ -197,28 +148,27 @@ pub enum Upgrade {
    Packages(FxHashMap<PackageName, Vec<Requirement>>),
 }

-/// Determine the [`Upgrade`] strategy from the command-line arguments.
-impl From<Option<UpgradeSelection>> for Upgrade {
-    fn from(value: Option<UpgradeSelection>) -> Self {
-        match value {
-            None => Self::None,
-            Some(UpgradeSelection::None) => Self::None,
-            Some(UpgradeSelection::All) => Self::All,
-            Some(UpgradeSelection::Packages(requirements)) => Self::Packages(
-                requirements
-                    .into_iter()
-                    .fold(FxHashMap::default(), |mut map, requirement| {
+impl Upgrade {
+    /// Determine the upgrade selection strategy from the command-line arguments.
+    pub fn from_args(upgrade: Option<bool>, upgrade_package: Vec<Requirement>) -> Option<Self> {
+        match upgrade {
+            Some(true) => Some(Self::All),
+            // TODO(charlie): `--no-upgrade` with `--upgrade-package` should allow the specified
+            // packages to be upgraded. Right now, `--upgrade-package` is silently ignored.
+            Some(false) => Some(Self::None),
+            None if upgrade_package.is_empty() => None,
+            None => Some(Self::Packages(upgrade_package.into_iter().fold(
+                FxHashMap::default(),
+                |mut map, requirement| {
                    map.entry(requirement.name.clone())
                        .or_default()
                        .push(requirement);
                    map
-                    }),
-            ),
+                },
+            ))),
        }
    }
-}

-impl Upgrade {
    /// Create an [`Upgrade`] strategy to upgrade a single package.
    pub fn package(package_name: PackageName) -> Self {
        Self::Packages({
@ -265,19 +215,23 @@ impl Upgrade {
    /// Combine a set of [`Upgrade`] values.
    #[must_use]
    pub fn combine(self, other: Self) -> Self {
-        match (self, other) {
-            // If both are `None`, the result is `None`.
-            (Self::None, Self::None) => Self::None,
-            // If either is `All`, the result is `All`.
-            (Self::All, _) | (_, Self::All) => Self::All,
-            // If one is `None`, the result is the other.
-            (Self::Packages(a), Self::None) => Self::Packages(a),
-            (Self::None, Self::Packages(b)) => Self::Packages(b),
-            // If both are `Packages`, the result is the union of the two.
-            (Self::Packages(mut a), Self::Packages(b)) => {
-                a.extend(b);
-                Self::Packages(a)
+        match self {
+            // Setting `--upgrade` or `--no-upgrade` should clear previous `--upgrade-package` selections.
+            Self::All | Self::None => self,
+            Self::Packages(self_packages) => match other {
+                // If `--upgrade` was enabled previously, `--upgrade-package` is subsumed by upgrading all packages.
+                Self::All => other,
+                // If `--no-upgrade` was enabled previously, then `--upgrade-package` enables an explicit upgrade of those packages.
+                Self::None => Self::Packages(self_packages),
+                // If `--upgrade-package` was included twice, combine the requirements.
+                Self::Packages(other_packages) => {
+                    let mut combined = self_packages;
+                    for (package, requirements) in other_packages {
+                        combined.entry(package).or_default().extend(requirements);
                    }
+                    Self::Packages(combined)
+                }
+            },
        }
    }
 }
--- a/crates/uv-settings/src/combine.rs
+++ b/crates/uv-settings/src/combine.rs
@ -5,7 +5,7 @@ use url::Url;

 use uv_configuration::{
    ExportFormat, IndexStrategy, KeyringProviderType, RequiredVersion, TargetTriple,
-    TrustedPublishing, UpgradeSelection,
+    TrustedPublishing, Upgrade,
 };
 use uv_distribution_types::{
    ConfigSettings, ExtraBuildVariables, Index, IndexUrl, PackageConfigSettings, PipExtraIndex,
@ -181,7 +181,7 @@ impl Combine for Option<PackageConfigSettings> {
    }
 }

-impl Combine for Option<UpgradeSelection> {
+impl Combine for Option<Upgrade> {
    fn combine(self, other: Self) -> Self {
        match (self, other) {
            (Some(a), Some(b)) => Some(a.combine(b)),
--- a/crates/uv-settings/src/settings.rs
+++ b/crates/uv-settings/src/settings.rs
@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize};
 use uv_cache_info::CacheKey;
 use uv_configuration::{
    IndexStrategy, KeyringProviderType, PackageNameSpecifier, RequiredVersion, TargetTriple,
-    TrustedHost, TrustedPublishing, UpgradeSelection,
+    TrustedHost, TrustedPublishing, Upgrade,
 };
 use uv_distribution_types::{
    ConfigSettings, ExtraBuildVariables, Index, IndexUrl, IndexUrlError, PackageConfigSettings,
@ -369,7 +369,7 @@ pub struct ResolverOptions {
    pub config_settings_package: Option<PackageConfigSettings>,
    pub exclude_newer: ExcludeNewer,
    pub link_mode: Option<LinkMode>,
-    pub upgrade: Option<UpgradeSelection>,
+    pub upgrade: Option<Upgrade>,
    pub no_build: Option<bool>,
    pub no_build_package: Option<Vec<PackageName>>,
    pub no_binary: Option<bool>,
@ -407,7 +407,7 @@ pub struct ResolverInstallerOptions {
    pub link_mode: Option<LinkMode>,
    pub compile_bytecode: Option<bool>,
    pub no_sources: Option<bool>,
-    pub upgrade: Option<UpgradeSelection>,
+    pub upgrade: Option<Upgrade>,
    pub reinstall: Option<bool>,
    pub reinstall_package: Option<Vec<PackageName>>,
    pub no_build: Option<bool>,
@ -473,7 +473,7 @@ impl From<ResolverInstallerSchema> for ResolverInstallerOptions {
            link_mode,
            compile_bytecode,
            no_sources,
-            upgrade: UpgradeSelection::from_args(
+            upgrade: Upgrade::from_args(
                upgrade,
                upgrade_package
                    .into_iter()
@ -1886,7 +1886,7 @@ impl From<ResolverInstallerSchema> for ResolverOptions {
                    .collect(),
            ),
            link_mode: value.link_mode,
-            upgrade: UpgradeSelection::from_args(
+            upgrade: Upgrade::from_args(
                value.upgrade,
                value
                    .upgrade_package
--- a/crates/uv/src/commands/tool/install.rs
+++ b/crates/uv/src/commands/tool/install.rs
@ -234,10 +234,7 @@ pub(crate) async fn install(
    let settings = if request.is_latest() {
        ResolverInstallerSettings {
            resolver: ResolverSettings {
-                upgrade: settings
-                    .resolver
-                    .upgrade
-                    .combine(Upgrade::package(package_name.clone())),
+                upgrade: Upgrade::package(package_name.clone()).combine(settings.resolver.upgrade),
                ..settings.resolver
            },
            ..settings
--- a/crates/uv/src/settings.rs
+++ b/crates/uv/src/settings.rs
@ -24,7 +24,7 @@ use uv_configuration::{
    BuildOptions, Concurrency, DependencyGroups, DryRun, EditableMode, ExportFormat,
    ExtrasSpecification, HashCheckingMode, IndexStrategy, InstallOptions, KeyringProviderType,
    NoBinary, NoBuild, Preview, ProjectBuildBackend, Reinstall, RequiredVersion, SourceStrategy,
-    TargetTriple, TrustedHost, TrustedPublishing, Upgrade, UpgradeSelection, VersionControlSystem,
+    TargetTriple, TrustedHost, TrustedPublishing, Upgrade, VersionControlSystem,
 };
 use uv_distribution_types::{
    ConfigSettings, DependencyMetadata, ExtraBuildVariables, Index, IndexLocations, IndexUrl,
@ -2852,7 +2852,7 @@ impl From<ResolverOptions> for ResolverSettings {
            exclude_newer: value.exclude_newer,
            link_mode: value.link_mode.unwrap_or_default(),
            sources: SourceStrategy::from_args(value.no_sources.unwrap_or_default()),
-            upgrade: Upgrade::from(value.upgrade),
+            upgrade: value.upgrade.unwrap_or_default(),
            build_options: BuildOptions::new(
                NoBinary::from_args(value.no_binary, value.no_binary_package.unwrap_or_default()),
                NoBuild::from_args(value.no_build, value.no_build_package.unwrap_or_default()),
@ -2943,7 +2943,7 @@ impl From<ResolverInstallerOptions> for ResolverInstallerSettings {
                prerelease: value.prerelease.unwrap_or_default(),
                resolution: value.resolution.unwrap_or_default(),
                sources: SourceStrategy::from_args(value.no_sources.unwrap_or_default()),
-                upgrade: Upgrade::from(value.upgrade),
+                upgrade: value.upgrade.unwrap_or_default(),
            },
            compile_bytecode: value.compile_bytecode.unwrap_or_default(),
            reinstall: Reinstall::from_args(
@ -3317,8 +3317,7 @@ impl PipSettings {
                args.no_sources.combine(no_sources).unwrap_or_default(),
            ),
            strict: args.strict.combine(strict).unwrap_or_default(),
-            upgrade: Upgrade::from(
-                UpgradeSelection::from_args(
+            upgrade: Upgrade::from_args(
                args.upgrade,
                args.upgrade_package
                    .into_iter()
@ -3326,15 +3325,15 @@ impl PipSettings {
                    .map(Requirement::from)
                    .collect(),
            )
-                .combine(UpgradeSelection::from_args(
+            .combine(Upgrade::from_args(
                upgrade,
                upgrade_package
                    .into_iter()
                    .flatten()
                    .map(Requirement::from)
                    .collect(),
-                )),
-            ),
+            ))
+            .unwrap_or_default(),
            reinstall: Reinstall::from_args(
                args.reinstall.combine(reinstall),
                args.reinstall_package