Skip to content

Commit

Permalink
Retain and respect settings in tool upgrades (#5937)
Browse files Browse the repository at this point in the history
## Summary

We now persist the `ResolverInstallerOptions` when writing out a tool
receipt. When upgrading, we grab the saved options, and merge with the
command-line arguments and user-level filesystem settings (CLI > receipt
> filesystem).
  • Loading branch information
charliermarsh authored Aug 9, 2024
1 parent 44f9452 commit f89403f
Show file tree
Hide file tree
Showing 27 changed files with 583 additions and 110 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/distribution-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ workspace = true
cache-key = { workspace = true }
distribution-filename = { workspace = true }
pep440_rs = { workspace = true }
pep508_rs = { workspace = true }
pep508_rs = { workspace = true, features = ["serde"] }
platform-tags = { workspace = true }
pypi-types = { workspace = true }
uv-fs = { workspace = true }
Expand Down
3 changes: 2 additions & 1 deletion crates/distribution-types/src/index_url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,8 @@ impl From<VerbatimUrl> for FlatIndexLocation {
/// The index locations to use for fetching packages. By default, uses the PyPI index.
///
/// From a pip perspective, this type merges `--index-url`, `--extra-index-url`, and `--find-links`.
#[derive(Debug, Clone)]
#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub struct IndexLocations {
index: Option<IndexUrl>,
extra_index: Vec<IndexUrl>,
Expand Down
2 changes: 1 addition & 1 deletion crates/install-wheel-rs/src/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ fn parse_scripts(
scripts_from_ini(extras, python_minor, ini)
}

#[derive(Debug, Clone, Copy, Serialize, Deserialize)]
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
#[cfg_attr(feature = "clap", derive(clap::ValueEnum))]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
Expand Down
21 changes: 21 additions & 0 deletions crates/pep508-rs/src/verbatim_url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,27 @@ impl From<Url> for VerbatimUrl {
}
}

#[cfg(feature = "serde")]
impl serde::Serialize for VerbatimUrl {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
self.url.serialize(serializer)
}
}

#[cfg(feature = "serde")]
impl<'de> serde::Deserialize<'de> for VerbatimUrl {
fn deserialize<D>(deserializer: D) -> Result<VerbatimUrl, D::Error>
where
D: serde::Deserializer<'de>,
{
let url = Url::deserialize(deserializer)?;
Ok(VerbatimUrl::from_url(url))
}
}

impl Pep508Url for VerbatimUrl {
type Err = VerbatimUrlError;

Expand Down
3 changes: 0 additions & 3 deletions crates/uv-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2759,9 +2759,6 @@ pub struct ToolUpgradeArgs {

#[command(flatten)]
pub build: BuildArgs,

#[command(flatten)]
pub refresh: RefreshArgs,
}

#[derive(Args)]
Expand Down
30 changes: 25 additions & 5 deletions crates/uv-cli/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,9 +306,17 @@ pub fn resolver_installer_options(
},
find_links: index_args.find_links,
upgrade: flag(upgrade, no_upgrade),
upgrade_package: Some(upgrade_package),
upgrade_package: if upgrade_package.is_empty() {
None
} else {
Some(upgrade_package)
},
reinstall: flag(reinstall, no_reinstall),
reinstall_package: Some(reinstall_package),
reinstall_package: if reinstall_package.is_empty() {
None
} else {
Some(reinstall_package)
},
index_strategy,
keyring_provider,
resolution,
Expand All @@ -320,14 +328,26 @@ pub fn resolver_installer_options(
config_settings: config_setting
.map(|config_settings| config_settings.into_iter().collect::<ConfigSettings>()),
no_build_isolation: flag(no_build_isolation, build_isolation),
no_build_isolation_package: Some(no_build_isolation_package),
no_build_isolation_package: if no_build_isolation_package.is_empty() {
None
} else {
Some(no_build_isolation_package)
},
exclude_newer,
link_mode,
compile_bytecode: flag(compile_bytecode, no_compile_bytecode),
no_build: flag(no_build, build),
no_build_package: Some(no_build_package),
no_build_package: if no_build_package.is_empty() {
None
} else {
Some(no_build_package)
},
no_binary: flag(no_binary, binary),
no_binary_package: Some(no_binary_package),
no_binary_package: if no_binary_package.is_empty() {
None
} else {
Some(no_binary_package)
},
no_sources: if no_sources { Some(true) } else { None },
}
}
2 changes: 1 addition & 1 deletion crates/uv-configuration/src/authentication.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use uv_auth::{self, KeyringProvider};

/// Keyring provider type to use for credential lookup.
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, serde::Deserialize)]
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
#[cfg_attr(feature = "clap", derive(clap::ValueEnum))]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
Expand Down
11 changes: 7 additions & 4 deletions crates/uv-configuration/src/build_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ impl Display for BuildKind {
}
}

#[derive(Debug, Default, Clone, PartialEq, Eq)]
#[derive(Debug, Default, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub struct BuildOptions {
no_binary: NoBinary,
no_build: NoBuild,
Expand Down Expand Up @@ -111,7 +112,8 @@ impl BuildOptions {
}
}

#[derive(Debug, Default, Clone, PartialEq, Eq)]
#[derive(Debug, Default, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub enum NoBinary {
/// Allow installation of any wheel.
#[default]
Expand Down Expand Up @@ -206,7 +208,8 @@ impl NoBinary {
}
}

#[derive(Debug, Default, Clone, PartialEq, Eq)]
#[derive(Debug, Default, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub enum NoBuild {
/// Allow building wheels from any source distribution.
#[default]
Expand Down Expand Up @@ -305,7 +308,7 @@ impl NoBuild {
}
}

#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, serde::Deserialize)]
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
#[cfg_attr(feature = "clap", derive(clap::ValueEnum))]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-configuration/src/config_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl<'de> serde::Deserialize<'de> for ConfigSettingValue {
/// list of strings.
///
/// See: <https://peps.python.org/pep-0517/#config-settings>
#[derive(Debug, Default, Clone)]
#[derive(Debug, Default, Clone, PartialEq, Eq)]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct ConfigSettings(BTreeMap<String, ConfigSettingValue>);

Expand Down
6 changes: 4 additions & 2 deletions crates/uv-configuration/src/package_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ use rustc_hash::FxHashMap;
use uv_cache::{Refresh, Timestamp};

/// Whether to reinstall packages.
#[derive(Debug, Default, Clone)]
#[derive(Debug, Default, Clone, serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub enum Reinstall {
/// Don't reinstall any packages; respect the existing installation.
#[default]
Expand Down Expand Up @@ -58,7 +59,8 @@ impl From<Reinstall> for Refresh {
}

/// Whether to allow package upgrades.
#[derive(Debug, Default, Clone)]
#[derive(Debug, Default, Clone, serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub enum Upgrade {
/// Prefer pinned versions from the existing lockfile, if possible.
#[default]
Expand Down
3 changes: 2 additions & 1 deletion crates/uv-configuration/src/sources.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub enum SourceStrategy {
/// Use `tool.uv.sources` when resolving dependencies.
#[default]
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-resolver/src/prerelease.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use uv_normalize::PackageName;
use crate::resolver::ForkSet;
use crate::{DependencyMode, Manifest, ResolverMarkers};

#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, serde::Deserialize)]
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
#[cfg_attr(feature = "clap", derive(clap::ValueEnum))]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
Expand Down
93 changes: 91 additions & 2 deletions crates/uv-settings/src/settings.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{fmt::Debug, num::NonZeroUsize, path::PathBuf};

use serde::Deserialize;
use serde::{Deserialize, Serialize};

use distribution_types::{FlatIndexLocation, IndexUrl};
use install_wheel_rs::linker::LinkMode;
Expand Down Expand Up @@ -212,7 +212,9 @@ pub struct ResolverOptions {
/// Shared settings, relevant to all operations that must resolve and install dependencies. The
/// union of [`InstallerOptions`] and [`ResolverOptions`].
#[allow(dead_code)]
#[derive(Debug, Clone, Default, Deserialize, CombineOptions, OptionsMetadata)]
#[derive(
Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize, CombineOptions, OptionsMetadata,
)]
#[serde(rename_all = "kebab-case")]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct ResolverInstallerOptions {
Expand Down Expand Up @@ -1243,3 +1245,90 @@ impl From<ResolverInstallerOptions> for InstallerOptions {
}
}
}

/// The options persisted alongside an installed tool.
///
/// A mirror of [`ResolverInstallerOptions`], without upgrades and reinstalls, which shouldn't be
/// persisted in a tool receipt.
#[derive(
Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize, CombineOptions, OptionsMetadata,
)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
#[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))]
pub struct ToolOptions {
pub index_url: Option<IndexUrl>,
pub extra_index_url: Option<Vec<IndexUrl>>,
pub no_index: Option<bool>,
pub find_links: Option<Vec<FlatIndexLocation>>,
pub index_strategy: Option<IndexStrategy>,
pub keyring_provider: Option<KeyringProviderType>,
pub resolution: Option<ResolutionMode>,
pub prerelease: Option<PrereleaseMode>,
pub config_settings: Option<ConfigSettings>,
pub no_build_isolation: Option<bool>,
pub no_build_isolation_package: Option<Vec<PackageName>>,
pub exclude_newer: Option<ExcludeNewer>,
pub link_mode: Option<LinkMode>,
pub compile_bytecode: Option<bool>,
pub no_sources: Option<bool>,
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<ResolverInstallerOptions> for ToolOptions {
fn from(value: ResolverInstallerOptions) -> Self {
Self {
index_url: value.index_url,
extra_index_url: value.extra_index_url,
no_index: value.no_index,
find_links: value.find_links,
index_strategy: value.index_strategy,
keyring_provider: value.keyring_provider,
resolution: value.resolution,
prerelease: value.prerelease,
config_settings: value.config_settings,
no_build_isolation: value.no_build_isolation,
no_build_isolation_package: value.no_build_isolation_package,
exclude_newer: value.exclude_newer,
link_mode: value.link_mode,
compile_bytecode: value.compile_bytecode,
no_sources: value.no_sources,
no_build: value.no_build,
no_build_package: value.no_build_package,
no_binary: value.no_binary,
no_binary_package: value.no_binary_package,
}
}
}

impl From<ToolOptions> for ResolverInstallerOptions {
fn from(value: ToolOptions) -> Self {
Self {
index_url: value.index_url,
extra_index_url: value.extra_index_url,
no_index: value.no_index,
find_links: value.find_links,
index_strategy: value.index_strategy,
keyring_provider: value.keyring_provider,
resolution: value.resolution,
prerelease: value.prerelease,
config_settings: value.config_settings,
no_build_isolation: value.no_build_isolation,
no_build_isolation_package: value.no_build_isolation_package,
exclude_newer: value.exclude_newer,
link_mode: value.link_mode,
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,
no_build_package: value.no_build_package,
no_binary: value.no_binary,
no_binary_package: value.no_binary_package,
}
}
}
5 changes: 3 additions & 2 deletions crates/uv-tool/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ pep508_rs = { workspace = true }
pypi-types = { workspace = true }
uv-cache = { workspace = true }
uv-fs = { workspace = true }
uv-state = { workspace = true }
uv-installer = { workspace = true }
uv-python = { workspace = true }
uv-settings = { workspace = true }
uv-state = { workspace = true }
uv-virtualenv = { workspace = true }
uv-installer = { workspace = true }

dirs-sys = { workspace = true }
fs-err = { workspace = true }
Expand Down
9 changes: 0 additions & 9 deletions crates/uv-tool/src/receipt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,6 @@ impl ToolReceipt {
}
}

// Ignore raw document in comparison.
impl PartialEq for ToolReceipt {
fn eq(&self, other: &Self) -> bool {
self.tool.eq(&other.tool)
}
}

impl Eq for ToolReceipt {}

impl From<Tool> for ToolReceipt {
fn from(tool: Tool) -> Self {
ToolReceipt {
Expand Down
Loading

0 comments on commit f89403f

Please sign in to comment.