Treat --upgrade-package on the command-line as overriding upgrade = false in configuration (#15395)

## Summary

Right now, if you put `upgrade = false` in a `uv.toml`, then pass
`--upgrade-package numpy` on the CLI, we won't upgrade NumPy. This PR
fixes that interaction by ensuring that when we "combine", we look at
those arguments holistically (i.e., we bundle `upgrade` and
`upgrade-package` into a single struct, which then goes through the
`.combine` logic), rather than combining `upgrade` and `upgrade-package`
independently.

If approved, I then need to add the same thing for `no-build-isolation`,
`reinstall`, `no-build`, and `no-binary`.
This commit is contained in:
Charlie Marsh 2025-08-21 16:20:55 +01:00 committed by GitHub
parent b950453891
commit 0397595e53
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 357 additions and 145 deletions

View file

@ -5,7 +5,7 @@ use url::Url;
use uv_configuration::{
ExportFormat, IndexStrategy, KeyringProviderType, RequiredVersion, TargetTriple,
TrustedPublishing,
TrustedPublishing, UpgradeSelection,
};
use uv_distribution_types::{
ConfigSettings, ExtraBuildVariables, Index, IndexUrl, PackageConfigSettings, PipExtraIndex,
@ -181,6 +181,15 @@ impl Combine for Option<PackageConfigSettings> {
}
}
impl Combine for Option<UpgradeSelection> {
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

View file

@ -301,7 +301,7 @@ fn warn_uv_toml_masked_fields(options: &Options) {
allow_insecure_host,
},
top_level:
ResolverInstallerOptions {
ResolverInstallerSchema {
index,
index_url,
extra_index_url,

View file

@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize};
use uv_cache_info::CacheKey;
use uv_configuration::{
IndexStrategy, KeyringProviderType, PackageNameSpecifier, RequiredVersion, TargetTriple,
TrustedHost, TrustedPublishing,
TrustedHost, TrustedPublishing, UpgradeSelection,
};
use uv_distribution_types::{
ConfigSettings, ExtraBuildVariables, Index, IndexUrl, IndexUrlError, PackageConfigSettings,
@ -52,7 +52,7 @@ pub struct Options {
pub globals: GlobalOptions,
#[serde(flatten)]
pub top_level: ResolverInstallerOptions,
pub top_level: ResolverInstallerSchema,
#[serde(flatten)]
pub install_mirrors: PythonInstallMirrors,
@ -161,7 +161,7 @@ pub struct Options {
impl Options {
/// Construct an [`Options`] with the given global and top-level settings.
pub fn simple(globals: GlobalOptions, top_level: ResolverInstallerOptions) -> Self {
pub fn simple(globals: GlobalOptions, top_level: ResolverInstallerSchema) -> Self {
Self {
globals,
top_level,
@ -369,8 +369,7 @@ pub struct ResolverOptions {
pub config_settings_package: Option<PackageConfigSettings>,
pub exclude_newer: ExcludeNewer,
pub link_mode: Option<LinkMode>,
pub upgrade: Option<bool>,
pub upgrade_package: Option<Vec<Requirement<VerbatimParsedUrl>>>,
pub upgrade: Option<UpgradeSelection>,
pub no_build: Option<bool>,
pub no_build_package: Option<Vec<PackageName>>,
pub no_binary: Option<bool>,
@ -384,10 +383,159 @@ pub struct ResolverOptions {
/// Shared settings, relevant to all operations that must resolve and install dependencies. The
/// union of [`InstallerOptions`] and [`ResolverOptions`].
#[derive(Debug, Clone, Default, CombineOptions)]
pub struct ResolverInstallerOptions {
pub index: Option<Vec<Index>>,
pub index_url: Option<PipIndex>,
pub extra_index_url: Option<Vec<PipExtraIndex>>,
pub no_index: Option<bool>,
pub find_links: Option<Vec<PipFindLinks>>,
pub index_strategy: Option<IndexStrategy>,
pub keyring_provider: Option<KeyringProviderType>,
pub resolution: Option<ResolutionMode>,
pub prerelease: Option<PrereleaseMode>,
pub fork_strategy: Option<ForkStrategy>,
pub dependency_metadata: Option<Vec<StaticMetadata>>,
pub config_settings: Option<ConfigSettings>,
pub config_settings_package: Option<PackageConfigSettings>,
pub no_build_isolation: Option<bool>,
pub no_build_isolation_package: Option<Vec<PackageName>>,
pub extra_build_dependencies: Option<ExtraBuildDependencies>,
pub extra_build_variables: Option<ExtraBuildVariables>,
pub exclude_newer: Option<ExcludeNewerTimestamp>,
pub exclude_newer_package: Option<ExcludeNewerPackage>,
pub link_mode: Option<LinkMode>,
pub compile_bytecode: Option<bool>,
pub no_sources: Option<bool>,
pub upgrade: Option<UpgradeSelection>,
pub reinstall: Option<bool>,
pub reinstall_package: Option<Vec<PackageName>>,
pub no_build: Option<bool>,
pub no_build_package: Option<Vec<PackageName>>,
pub no_binary: Option<bool>,
pub no_binary_package: Option<Vec<PackageName>>,
}
impl From<ResolverInstallerSchema> for ResolverInstallerOptions {
fn from(value: ResolverInstallerSchema) -> Self {
let ResolverInstallerSchema {
index,
index_url,
extra_index_url,
no_index,
find_links,
index_strategy,
keyring_provider,
resolution,
prerelease,
fork_strategy,
dependency_metadata,
config_settings,
config_settings_package,
no_build_isolation,
no_build_isolation_package,
extra_build_dependencies,
extra_build_variables,
exclude_newer,
exclude_newer_package,
link_mode,
compile_bytecode,
no_sources,
upgrade,
upgrade_package,
reinstall,
reinstall_package,
no_build,
no_build_package,
no_binary,
no_binary_package,
} = value;
Self {
index,
index_url,
extra_index_url,
no_index,
find_links,
index_strategy,
keyring_provider,
resolution,
prerelease,
fork_strategy,
dependency_metadata,
config_settings,
config_settings_package,
no_build_isolation,
no_build_isolation_package,
extra_build_dependencies,
extra_build_variables,
exclude_newer,
exclude_newer_package,
link_mode,
compile_bytecode,
no_sources,
upgrade: UpgradeSelection::from_args(
upgrade,
upgrade_package
.into_iter()
.flatten()
.map(Into::into)
.collect(),
),
reinstall,
reinstall_package,
no_build,
no_build_package,
no_binary,
no_binary_package,
}
}
}
impl ResolverInstallerSchema {
/// Resolve the [`ResolverInstallerSchema`] relative to the given root directory.
pub fn relative_to(self, root_dir: &Path) -> Result<Self, IndexUrlError> {
Ok(Self {
index: self
.index
.map(|index| {
index
.into_iter()
.map(|index| index.relative_to(root_dir))
.collect::<Result<Vec<_>, _>>()
})
.transpose()?,
index_url: self
.index_url
.map(|index_url| index_url.relative_to(root_dir))
.transpose()?,
extra_index_url: self
.extra_index_url
.map(|extra_index_url| {
extra_index_url
.into_iter()
.map(|extra_index_url| extra_index_url.relative_to(root_dir))
.collect::<Result<Vec<_>, _>>()
})
.transpose()?,
find_links: self
.find_links
.map(|find_links| {
find_links
.into_iter()
.map(|find_link| find_link.relative_to(root_dir))
.collect::<Result<Vec<_>, _>>()
})
.transpose()?,
..self
})
}
}
/// The JSON schema for the `[tool.uv]` section of a `pyproject.toml` file.
#[derive(Debug, Clone, Default, PartialEq, Eq, Deserialize, CombineOptions, OptionsMetadata)]
#[serde(rename_all = "kebab-case")]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct ResolverInstallerOptions {
pub struct ResolverInstallerSchema {
/// The package indexes to use when resolving dependencies.
///
/// Accepts either a repository compliant with [PEP 503](https://peps.python.org/pep-0503/)
@ -814,46 +962,6 @@ pub struct ResolverInstallerOptions {
pub no_binary_package: Option<Vec<PackageName>>,
}
impl ResolverInstallerOptions {
/// Resolve the [`ResolverInstallerOptions`] relative to the given root directory.
pub fn relative_to(self, root_dir: &Path) -> Result<Self, IndexUrlError> {
Ok(Self {
index: self
.index
.map(|index| {
index
.into_iter()
.map(|index| index.relative_to(root_dir))
.collect::<Result<Vec<_>, _>>()
})
.transpose()?,
index_url: self
.index_url
.map(|index_url| index_url.relative_to(root_dir))
.transpose()?,
extra_index_url: self
.extra_index_url
.map(|extra_index_url| {
extra_index_url
.into_iter()
.map(|extra_index_url| extra_index_url.relative_to(root_dir))
.collect::<Result<Vec<_>, _>>()
})
.transpose()?,
find_links: self
.find_links
.map(|find_links| {
find_links
.into_iter()
.map(|find_link| find_link.relative_to(root_dir))
.collect::<Result<Vec<_>, _>>()
})
.transpose()?,
..self
})
}
}
/// Shared settings, relevant to all operations that might create managed python installations.
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, CombineOptions, OptionsMetadata)]
#[serde(rename_all = "kebab-case")]
@ -1752,8 +1860,8 @@ impl PipOptions {
}
}
impl From<ResolverInstallerOptions> for ResolverOptions {
fn from(value: ResolverInstallerOptions) -> Self {
impl From<ResolverInstallerSchema> for ResolverOptions {
fn from(value: ResolverInstallerSchema) -> Self {
Self {
index: value.index,
index_url: value.index_url,
@ -1778,8 +1886,15 @@ impl From<ResolverInstallerOptions> for ResolverOptions {
.collect(),
),
link_mode: value.link_mode,
upgrade: value.upgrade,
upgrade_package: value.upgrade_package,
upgrade: UpgradeSelection::from_args(
value.upgrade,
value
.upgrade_package
.into_iter()
.flatten()
.map(Into::into)
.collect(),
),
no_build: value.no_build,
no_build_package: value.no_build_package,
no_binary: value.no_binary,
@ -1793,8 +1908,8 @@ impl From<ResolverInstallerOptions> for ResolverOptions {
}
}
impl From<ResolverInstallerOptions> for InstallerOptions {
fn from(value: ResolverInstallerOptions) -> Self {
impl From<ResolverInstallerSchema> for InstallerOptions {
fn from(value: ResolverInstallerSchema) -> Self {
Self {
index: value.index,
index_url: value.index_url,
@ -1830,7 +1945,7 @@ impl From<ResolverInstallerOptions> for InstallerOptions {
/// The options persisted alongside an installed tool.
///
/// A mirror of [`ResolverInstallerOptions`], without upgrades and reinstalls, which shouldn't be
/// A mirror of [`ResolverInstallerSchema`], without upgrades and reinstalls, which shouldn't be
/// persisted in a tool receipt.
#[derive(
Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize, CombineOptions, OptionsMetadata,
@ -1925,7 +2040,6 @@ impl From<ToolOptions> for ResolverInstallerOptions {
compile_bytecode: value.compile_bytecode,
no_sources: value.no_sources,
upgrade: None,
upgrade_package: None,
reinstall: None,
reinstall_package: None,
no_build: value.no_build,
@ -2120,7 +2234,7 @@ impl From<OptionsWire> for Options {
// Used twice for backwards compatibility
allow_insecure_host: allow_insecure_host.clone(),
},
top_level: ResolverInstallerOptions {
top_level: ResolverInstallerSchema {
index,
index_url,
extra_index_url,