From 330b54b17369f2b75454cb6824d17abd07d57d83 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 21 Aug 2025 20:03:03 +0100 Subject: [PATCH] Add file-to-CLI overrides for reinstall configuration (#15426) ## Summary This is like #15395, but for `--reinstall` and `--reinstall-package`. --- crates/uv-cli/src/options.rs | 12 ++--- .../uv-configuration/src/package_options.rs | 46 +++++++++---------- crates/uv-settings/src/combine.rs | 11 ++++- crates/uv-settings/src/settings.rs | 11 ++--- crates/uv/src/commands/tool/install.rs | 4 +- crates/uv/src/settings.rs | 18 ++++---- crates/uv/tests/it/show_settings.rs | 1 - 7 files changed, 51 insertions(+), 52 deletions(-) diff --git a/crates/uv-cli/src/options.rs b/crates/uv-cli/src/options.rs index c9b076a24..220ac8737 100644 --- 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::Upgrade; +use uv_configuration::{Reinstall, Upgrade}; use uv_distribution_types::{ConfigSettings, PackageConfigSettings, Requirement}; use uv_resolver::{ExcludeNewer, ExcludeNewerPackage, PrereleaseMode}; use uv_settings::{Combine, PipOptions, ResolverInstallerOptions, ResolverOptions}; @@ -449,12 +449,10 @@ pub fn resolver_installer_options( flag(upgrade, no_upgrade, "upgrade"), upgrade_package.into_iter().map(Requirement::from).collect(), ), - reinstall: flag(reinstall, no_reinstall, "reinstall"), - reinstall_package: if reinstall_package.is_empty() { - None - } else { - Some(reinstall_package) - }, + reinstall: Reinstall::from_args( + flag(reinstall, no_reinstall, "reinstall"), + reinstall_package, + ), index_strategy, keyring_provider, resolution, diff --git a/crates/uv-configuration/src/package_options.rs b/crates/uv-configuration/src/package_options.rs index a22e1523d..0e251186d 100644 --- a/crates/uv-configuration/src/package_options.rs +++ b/crates/uv-configuration/src/package_options.rs @@ -25,17 +25,12 @@ pub enum Reinstall { impl Reinstall { /// Determine the reinstall strategy to use. - pub fn from_args(reinstall: Option, reinstall_package: Vec) -> Self { + pub fn from_args(reinstall: Option, reinstall_package: Vec) -> Option { match reinstall { - Some(true) => Self::All, - Some(false) => Self::None, - None => { - if reinstall_package.is_empty() { - Self::None - } else { - Self::Packages(reinstall_package, Vec::new()) - } - } + Some(true) => Some(Self::All), + Some(false) => Some(Self::None), + None if reinstall_package.is_empty() => None, + None => Some(Self::Packages(reinstall_package, Vec::new())), } } @@ -72,20 +67,23 @@ impl Reinstall { /// Combine a set of [`Reinstall`] 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(a1, a2), Self::None) => Self::Packages(a1, a2), - (Self::None, Self::Packages(b1, b2)) => Self::Packages(b1, b2), - // If both are `Packages`, the result is the union of the two. - (Self::Packages(mut a1, mut a2), Self::Packages(b1, b2)) => { - a1.extend(b1); - a2.extend(b2); - Self::Packages(a1, a2) - } + match self { + // Setting `--reinstall` or `--no-reinstall` should clear previous `--reinstall-package` selections. + Self::All | Self::None => self, + Self::Packages(self_packages, self_paths) => match other { + // If `--reinstall` was enabled previously, `--reinstall-package` is subsumed by reinstalling all packages. + Self::All => other, + // If `--no-reinstall` was enabled previously, then `--reinstall-package` enables an explicit reinstall of those packages. + Self::None => Self::Packages(self_packages, self_paths), + // If `--reinstall-package` was included twice, combine the requirements. + Self::Packages(other_packages, other_paths) => { + let mut combined_packages = self_packages; + combined_packages.extend(other_packages); + let mut combined_paths = self_paths; + combined_paths.extend(other_paths); + Self::Packages(combined_packages, combined_paths) + } + }, } } diff --git a/crates/uv-settings/src/combine.rs b/crates/uv-settings/src/combine.rs index 7021f9db3..18a3574eb 100644 --- a/crates/uv-settings/src/combine.rs +++ b/crates/uv-settings/src/combine.rs @@ -4,7 +4,7 @@ use std::{collections::BTreeMap, num::NonZeroUsize}; use url::Url; use uv_configuration::{ - ExportFormat, IndexStrategy, KeyringProviderType, RequiredVersion, TargetTriple, + ExportFormat, IndexStrategy, KeyringProviderType, Reinstall, RequiredVersion, TargetTriple, TrustedPublishing, Upgrade, }; use uv_distribution_types::{ @@ -190,6 +190,15 @@ impl Combine for Option { } } +impl Combine for Option { + fn combine(self, other: Self) -> Self { + match (self, other) { + (Some(a), Some(b)) => Some(a.combine(b)), + (a, b) => a.or(b), + } + } +} + impl Combine for serde::de::IgnoredAny { fn combine(self, _other: Self) -> Self { self diff --git a/crates/uv-settings/src/settings.rs b/crates/uv-settings/src/settings.rs index 9185eedd8..3680a531b 100644 --- a/crates/uv-settings/src/settings.rs +++ b/crates/uv-settings/src/settings.rs @@ -4,8 +4,8 @@ use serde::{Deserialize, Serialize}; use uv_cache_info::CacheKey; use uv_configuration::{ - IndexStrategy, KeyringProviderType, PackageNameSpecifier, RequiredVersion, TargetTriple, - TrustedHost, TrustedPublishing, Upgrade, + IndexStrategy, KeyringProviderType, PackageNameSpecifier, Reinstall, RequiredVersion, + TargetTriple, TrustedHost, TrustedPublishing, Upgrade, }; use uv_distribution_types::{ ConfigSettings, ExtraBuildVariables, Index, IndexUrl, IndexUrlError, PackageConfigSettings, @@ -408,8 +408,7 @@ pub struct ResolverInstallerOptions { pub compile_bytecode: Option, pub no_sources: Option, pub upgrade: Option, - pub reinstall: Option, - pub reinstall_package: Option>, + pub reinstall: Option, pub no_build: Option, pub no_build_package: Option>, pub no_binary: Option, @@ -481,8 +480,7 @@ impl From for ResolverInstallerOptions { .map(Into::into) .collect(), ), - reinstall, - reinstall_package, + reinstall: Reinstall::from_args(reinstall, reinstall_package.unwrap_or_default()), no_build, no_build_package, no_binary, @@ -2041,7 +2039,6 @@ impl From for ResolverInstallerOptions { no_sources: value.no_sources, upgrade: None, reinstall: None, - reinstall_package: None, no_build: value.no_build, no_build_package: value.no_build_package, no_binary: value.no_binary, diff --git a/crates/uv/src/commands/tool/install.rs b/crates/uv/src/commands/tool/install.rs index 8b0b9070a..d464aca8c 100644 --- a/crates/uv/src/commands/tool/install.rs +++ b/crates/uv/src/commands/tool/install.rs @@ -246,9 +246,7 @@ pub(crate) async fn install( // If the user passed `--force`, it implies `--reinstall-package ` let settings = if force { ResolverInstallerSettings { - reinstall: settings - .reinstall - .combine(Reinstall::package(package_name.clone())), + reinstall: Reinstall::package(package_name.clone()).combine(settings.reinstall), ..settings } } else { diff --git a/crates/uv/src/settings.rs b/crates/uv/src/settings.rs index 6a8dcf827..df802b5dc 100644 --- a/crates/uv/src/settings.rs +++ b/crates/uv/src/settings.rs @@ -2946,10 +2946,7 @@ impl From for ResolverInstallerSettings { upgrade: value.upgrade.unwrap_or_default(), }, compile_bytecode: value.compile_bytecode.unwrap_or_default(), - reinstall: Reinstall::from_args( - value.reinstall, - value.reinstall_package.unwrap_or_default(), - ), + reinstall: value.reinstall.unwrap_or_default(), } } } @@ -3335,11 +3332,14 @@ impl PipSettings { )) .unwrap_or_default(), reinstall: Reinstall::from_args( - args.reinstall.combine(reinstall), - args.reinstall_package - .combine(reinstall_package) - .unwrap_or_default(), - ), + args.reinstall, + args.reinstall_package.unwrap_or_default(), + ) + .combine(Reinstall::from_args( + reinstall, + reinstall_package.unwrap_or_default(), + )) + .unwrap_or_default(), build_options: BuildOptions::new( NoBinary::from_pip_args(args.no_binary.combine(no_binary).unwrap_or_default()) .combine(NoBinary::from_args( diff --git a/crates/uv/tests/it/show_settings.rs b/crates/uv/tests/it/show_settings.rs index c7018a6c6..d9fe8062e 100644 --- a/crates/uv/tests/it/show_settings.rs +++ b/crates/uv/tests/it/show_settings.rs @@ -3515,7 +3515,6 @@ fn resolve_tool() -> anyhow::Result<()> { no_sources: None, upgrade: None, reinstall: None, - reinstall_package: None, no_build: None, no_build_package: None, no_binary: None,