Move argument normalization into settings construction (#3103)

## Summary

No behavior changes, but the idea here is that we move the argument
normalization code (e.g., create an `Upgrade` struct from `--upgrade`
and `--upgrade-package`) into the `settings.rs` file, where we build the
common settings structs.

This reduces a lot of the logic and duplication across commands in
`main.rs`.
This commit is contained in:
Charlie Marsh 2024-04-19 19:45:08 -04:00 committed by GitHub
parent ac9c4e1f38
commit 4a98839c1d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 117 additions and 202 deletions

View file

@ -23,7 +23,7 @@ pub struct SourceTreeResolver<'a, Context: BuildContext + Send + Sync> {
/// The requirements for the project.
source_trees: Vec<PathBuf>,
/// The extras to include when resolving requirements.
extras: &'a ExtrasSpecification<'a>,
extras: &'a ExtrasSpecification,
/// The hash policy to enforce.
hasher: &'a HashStrategy,
/// The in-memory index for resolving dependencies.
@ -36,7 +36,7 @@ impl<'a, Context: BuildContext + Send + Sync> SourceTreeResolver<'a, Context> {
/// Instantiate a new [`SourceTreeResolver`] for a given set of `source_trees`.
pub fn new(
source_trees: Vec<PathBuf>,
extras: &'a ExtrasSpecification<'a>,
extras: &'a ExtrasSpecification,
hasher: &'a HashStrategy,
context: &'a Context,
client: &'a RegistryClient,

View file

@ -147,14 +147,25 @@ impl std::fmt::Display for RequirementsSource {
}
#[derive(Debug, Default, Clone)]
pub enum ExtrasSpecification<'a> {
pub enum ExtrasSpecification {
#[default]
None,
All,
Some(&'a [ExtraName]),
Some(Vec<ExtraName>),
}
impl ExtrasSpecification<'_> {
impl ExtrasSpecification {
/// Determine the extras specification to use based on the command-line arguments.
pub fn from_args(all_extras: bool, extra: Vec<ExtraName>) -> Self {
if all_extras {
ExtrasSpecification::All
} else if extra.is_empty() {
ExtrasSpecification::None
} else {
ExtrasSpecification::Some(extra)
}
}
/// Returns true if a name is included in the extra specification.
pub fn contains(&self, name: &ExtraName) -> bool {
match self {

View file

@ -51,7 +51,7 @@ impl RequirementsSpecification {
#[instrument(skip_all, level = Level::DEBUG, fields(source = % source))]
pub async fn from_source(
source: &RequirementsSource,
extras: &ExtrasSpecification<'_>,
extras: &ExtrasSpecification,
client_builder: &BaseClientBuilder<'_>,
) -> Result<Self> {
Ok(match source {
@ -224,7 +224,7 @@ impl RequirementsSpecification {
requirements: &[RequirementsSource],
constraints: &[RequirementsSource],
overrides: &[RequirementsSource],
extras: &ExtrasSpecification<'_>,
extras: &ExtrasSpecification,
client_builder: &BaseClientBuilder<'_>,
) -> Result<Self> {
let mut spec = Self::default();

View file

@ -350,7 +350,7 @@ pub(crate) struct PipCompileArgs {
/// Refresh cached data for a specific package.
#[arg(long)]
pub(crate) refresh_package: Option<Vec<PackageName>>,
pub(crate) refresh_package: Vec<PackageName>,
/// The method to use when installing packages from the global cache.
///
@ -455,7 +455,7 @@ pub(crate) struct PipCompileArgs {
/// Allow upgrades for a specific package, ignoring pinned versions in the existing output
/// file.
#[arg(long, short = 'P')]
pub(crate) upgrade_package: Option<Vec<PackageName>>,
pub(crate) upgrade_package: Vec<PackageName>,
/// Include distribution hashes in the output file.
#[arg(long, overrides_with("no_generate_hashes"))]
@ -901,7 +901,7 @@ pub(crate) struct PipInstallArgs {
/// Allow upgrade of a specific package.
#[arg(long, short = 'P')]
pub(crate) upgrade_package: Option<Vec<PackageName>>,
pub(crate) upgrade_package: Vec<PackageName>,
/// Reinstall all packages, regardless of whether they're already installed.
#[arg(long, alias = "force-reinstall")]
@ -909,7 +909,7 @@ pub(crate) struct PipInstallArgs {
/// Reinstall a specific package, regardless of whether it's already installed.
#[arg(long)]
pub(crate) reinstall_package: Option<Vec<PackageName>>,
pub(crate) reinstall_package: Vec<PackageName>,
#[arg(
global = true,
@ -929,7 +929,7 @@ pub(crate) struct PipInstallArgs {
/// Refresh cached data for a specific package.
#[arg(long)]
pub(crate) refresh_package: Option<Vec<PackageName>>,
pub(crate) refresh_package: Vec<PackageName>,
/// Ignore package dependencies, instead only installing those packages explicitly listed
/// on the command line or in the requirements files.

View file

@ -53,7 +53,7 @@ pub(crate) async fn pip_compile(
requirements: &[RequirementsSource],
constraints: &[RequirementsSource],
overrides: &[RequirementsSource],
extras: ExtrasSpecification<'_>,
extras: ExtrasSpecification,
output_file: Option<&Path>,
resolution_mode: ResolutionMode,
prerelease_mode: PreReleaseMode,
@ -131,7 +131,7 @@ pub(crate) async fn pip_compile(
// If all the metadata could be statically resolved, validate that every extra was used. If we
// need to resolve metadata via PEP 517, we don't know which extras are used until much later.
if source_trees.is_empty() {
if let ExtrasSpecification::Some(extras) = extras {
if let ExtrasSpecification::Some(extras) = &extras {
let mut unused_extras = extras
.iter()
.filter(|extra| !used_extras.contains(extra))

View file

@ -57,7 +57,7 @@ pub(crate) async fn pip_install(
requirements: &[RequirementsSource],
constraints: &[RequirementsSource],
overrides: &[RequirementsSource],
extras: &ExtrasSpecification<'_>,
extras: &ExtrasSpecification,
resolution_mode: ResolutionMode,
prerelease_mode: PreReleaseMode,
dependency_mode: DependencyMode,
@ -426,7 +426,7 @@ async fn read_requirements(
requirements: &[RequirementsSource],
constraints: &[RequirementsSource],
overrides: &[RequirementsSource],
extras: &ExtrasSpecification<'_>,
extras: &ExtrasSpecification,
client_builder: &BaseClientBuilder<'_>,
) -> Result<RequirementsSpecification, Error> {
// If the user requests `extras` but does not provide a valid source (e.g., a `pyproject.toml`),

View file

@ -10,12 +10,9 @@ use clap::{CommandFactory, Parser};
use owo_colors::OwoColorize;
use tracing::instrument;
use distribution_types::IndexLocations;
use uv_cache::{Cache, Refresh};
use uv_client::Connectivity;
use uv_configuration::{NoBinary, NoBuild, Reinstall, SetupPyStrategy, Upgrade};
use uv_requirements::{ExtrasSpecification, RequirementsSource};
use uv_resolver::DependencyMode;
use uv_cache::Cache;
use uv_requirements::RequirementsSource;
use crate::cli::{CacheCommand, CacheNamespace, Cli, Commands, PipCommand, PipNamespace};
#[cfg(feature = "self-update")]
@ -178,7 +175,7 @@ async fn run() -> Result<ExitStatus> {
// Resolve the settings from the command-line arguments and workspace configuration.
let args = PipCompileSettings::resolve(args, workspace);
let cache = cache.with_refresh(Refresh::from_args(args.refresh, args.refresh_package));
let cache = cache.with_refresh(args.refresh);
let requirements = args
.src_file
.into_iter()
@ -194,43 +191,17 @@ async fn run() -> Result<ExitStatus> {
.into_iter()
.map(RequirementsSource::from_overrides_txt)
.collect::<Vec<_>>();
let index_urls = IndexLocations::new(
args.shared.index_url,
args.shared.extra_index_url,
args.shared.find_links,
args.shared.no_index,
);
// TODO(charlie): Move into `PipCompileSettings::resolve`.
let extras = if args.shared.all_extras {
ExtrasSpecification::All
} else if args.shared.extra.is_empty() {
ExtrasSpecification::None
} else {
ExtrasSpecification::Some(&args.shared.extra)
};
let upgrade = Upgrade::from_args(args.upgrade, args.upgrade_package);
let no_build = NoBuild::from_args(args.shared.only_binary, args.shared.no_build);
let dependency_mode = if args.shared.no_deps {
DependencyMode::Direct
} else {
DependencyMode::Transitive
};
let setup_py = if args.shared.legacy_setup_py {
SetupPyStrategy::Setuptools
} else {
SetupPyStrategy::Pep517
};
commands::pip_compile(
&requirements,
&constraints,
&overrides,
extras,
args.shared.extras,
args.shared.output_file.as_deref(),
args.shared.resolution,
args.shared.prerelease,
dependency_mode,
upgrade,
args.shared.dependency_mode,
args.upgrade,
args.shared.generate_hashes,
args.shared.no_emit_package,
args.shared.no_strip_extras,
@ -241,18 +212,14 @@ async fn run() -> Result<ExitStatus> {
args.shared.emit_find_links,
args.shared.emit_marker_expression,
args.shared.emit_index_annotation,
index_urls,
args.shared.index_locations,
args.shared.index_strategy,
args.shared.keyring_provider,
setup_py,
args.shared.setup_py,
args.shared.config_setting,
if args.shared.offline {
Connectivity::Offline
} else {
Connectivity::Online
},
args.shared.connectivity,
args.shared.no_build_isolation,
no_build,
args.shared.no_build,
args.shared.python_version,
args.shared.python_platform,
args.shared.exclude_newer,
@ -275,46 +242,28 @@ async fn run() -> Result<ExitStatus> {
// Resolve the settings from the command-line arguments and workspace configuration.
let args = PipSyncSettings::resolve(args, workspace);
let cache = cache.with_refresh(Refresh::from_args(args.refresh, args.refresh_package));
let index_urls = IndexLocations::new(
args.shared.index_url,
args.shared.extra_index_url,
args.shared.find_links,
args.shared.no_index,
);
let cache = cache.with_refresh(args.refresh);
let sources = args
.src_file
.into_iter()
.map(RequirementsSource::from_requirements_file)
.collect::<Vec<_>>();
let reinstall = Reinstall::from_args(args.reinstall, args.reinstall_package);
let no_binary = NoBinary::from_args(args.shared.no_binary);
let no_build = NoBuild::from_args(args.shared.only_binary, args.shared.no_build);
let setup_py = if args.shared.legacy_setup_py {
SetupPyStrategy::Setuptools
} else {
SetupPyStrategy::Pep517
};
commands::pip_sync(
&sources,
&reinstall,
&args.reinstall,
args.shared.link_mode,
args.shared.compile_bytecode,
args.shared.require_hashes,
index_urls,
args.shared.index_locations,
args.shared.index_strategy,
args.shared.keyring_provider,
setup_py,
if args.shared.offline {
Connectivity::Offline
} else {
Connectivity::Online
},
args.shared.setup_py,
args.shared.connectivity,
&args.shared.config_setting,
args.shared.no_build_isolation,
no_build,
no_binary,
args.shared.no_build,
args.shared.no_binary,
args.shared.strict,
args.shared.python,
args.shared.system,
@ -331,7 +280,7 @@ async fn run() -> Result<ExitStatus> {
// Resolve the settings from the command-line arguments and workspace configuration.
let args = PipInstallSettings::resolve(args, workspace);
let cache = cache.with_refresh(Refresh::from_args(args.refresh, args.refresh_package));
let cache = cache.with_refresh(args.refresh);
let requirements = args
.package
.into_iter()
@ -353,60 +302,29 @@ async fn run() -> Result<ExitStatus> {
.into_iter()
.map(RequirementsSource::from_overrides_txt)
.collect::<Vec<_>>();
let index_urls = IndexLocations::new(
args.shared.index_url,
args.shared.extra_index_url,
args.shared.find_links,
args.shared.no_index,
);
let extras = if args.shared.all_extras {
ExtrasSpecification::All
} else if args.shared.extra.is_empty() {
ExtrasSpecification::None
} else {
ExtrasSpecification::Some(&args.shared.extra)
};
let reinstall = Reinstall::from_args(args.reinstall, args.reinstall_package);
let upgrade = Upgrade::from_args(args.upgrade, args.upgrade_package);
let no_binary = NoBinary::from_args(args.shared.no_binary);
let no_build = NoBuild::from_args(args.shared.only_binary, args.shared.no_build);
let dependency_mode = if args.shared.no_deps {
DependencyMode::Direct
} else {
DependencyMode::Transitive
};
let setup_py = if args.shared.legacy_setup_py {
SetupPyStrategy::Setuptools
} else {
SetupPyStrategy::Pep517
};
commands::pip_install(
&requirements,
&constraints,
&overrides,
&extras,
&args.shared.extras,
args.shared.resolution,
args.shared.prerelease,
dependency_mode,
upgrade,
index_urls,
args.shared.dependency_mode,
args.upgrade,
args.shared.index_locations,
args.shared.index_strategy,
args.shared.keyring_provider,
reinstall,
args.reinstall,
args.shared.link_mode,
args.shared.compile_bytecode,
args.shared.require_hashes,
setup_py,
if args.shared.offline {
Connectivity::Offline
} else {
Connectivity::Online
},
args.shared.setup_py,
args.shared.connectivity,
&args.shared.config_setting,
args.shared.no_build_isolation,
no_build,
no_binary,
args.shared.no_build,
args.shared.no_binary,
args.shared.strict,
args.shared.exclude_newer,
args.shared.python,
@ -441,11 +359,7 @@ async fn run() -> Result<ExitStatus> {
args.shared.system,
args.shared.break_system_packages,
cache,
if args.shared.offline {
Connectivity::Offline
} else {
Connectivity::Online
},
args.shared.connectivity,
globals.native_tls,
args.shared.keyring_provider,
printer,
@ -534,13 +448,6 @@ async fn run() -> Result<ExitStatus> {
// Resolve the settings from the command-line arguments and workspace configuration.
let args = settings::VenvSettings::resolve(args, workspace);
let index_locations = IndexLocations::new(
args.shared.index_url,
args.shared.extra_index_url,
args.shared.find_links,
args.shared.no_index,
);
// Since we use ".venv" as the default name, we use "." as the default prompt.
let prompt = args.prompt.or_else(|| {
if args.name == PathBuf::from(".venv") {
@ -554,16 +461,12 @@ async fn run() -> Result<ExitStatus> {
&args.name,
args.shared.python.as_deref(),
args.shared.link_mode,
&index_locations,
&args.shared.index_locations,
args.shared.index_strategy,
args.shared.keyring_provider,
uv_virtualenv::Prompt::from_args(prompt),
args.system_site_packages,
if args.shared.offline {
Connectivity::Offline
} else {
Connectivity::Online
},
args.shared.connectivity,
args.seed,
args.shared.exclude_newer,
globals.native_tls,

View file

@ -1,13 +1,16 @@
use std::path::PathBuf;
use distribution_types::{FlatIndexLocation, IndexUrl};
use distribution_types::IndexLocations;
use install_wheel_rs::linker::LinkMode;
use uv_cache::CacheArgs;
use uv_cache::{CacheArgs, Refresh};
use uv_client::Connectivity;
use uv_configuration::{
ConfigSettings, IndexStrategy, KeyringProviderType, PackageNameSpecifier, TargetTriple,
ConfigSettings, IndexStrategy, KeyringProviderType, NoBinary, NoBuild, Reinstall,
SetupPyStrategy, TargetTriple, Upgrade,
};
use uv_normalize::{ExtraName, PackageName};
use uv_resolver::{AnnotationStyle, ExcludeNewer, PreReleaseMode, ResolutionMode};
use uv_normalize::PackageName;
use uv_requirements::ExtrasSpecification;
use uv_resolver::{AnnotationStyle, DependencyMode, ExcludeNewer, PreReleaseMode, ResolutionMode};
use uv_toolchain::PythonVersion;
use uv_workspace::{PipOptions, Workspace};
@ -76,10 +79,8 @@ pub(crate) struct PipCompileSettings {
pub(crate) src_file: Vec<PathBuf>,
pub(crate) constraint: Vec<PathBuf>,
pub(crate) r#override: Vec<PathBuf>,
pub(crate) refresh: bool,
pub(crate) refresh_package: Vec<PackageName>,
pub(crate) upgrade: bool,
pub(crate) upgrade_package: Vec<PackageName>,
pub(crate) refresh: Refresh,
pub(crate) upgrade: Upgrade,
// Shared settings.
pub(crate) shared: PipSharedSettings,
@ -155,11 +156,8 @@ impl PipCompileSettings {
src_file,
constraint,
r#override,
refresh,
refresh_package: refresh_package.unwrap_or_default(),
upgrade,
upgrade_package: upgrade_package.unwrap_or_default(),
refresh: Refresh::from_args(refresh, refresh_package),
upgrade: Upgrade::from_args(upgrade, upgrade_package),
// Shared settings.
shared: PipSharedSettings::combine(
@ -224,10 +222,8 @@ impl PipCompileSettings {
pub(crate) struct PipSyncSettings {
// CLI-only settings.
pub(crate) src_file: Vec<PathBuf>,
pub(crate) reinstall: bool,
pub(crate) reinstall_package: Vec<PackageName>,
pub(crate) refresh: bool,
pub(crate) refresh_package: Vec<PackageName>,
pub(crate) reinstall: Reinstall,
pub(crate) refresh: Refresh,
// Shared settings.
pub(crate) shared: PipSharedSettings,
@ -277,10 +273,8 @@ impl PipSyncSettings {
Self {
// CLI-only settings.
src_file,
reinstall,
reinstall_package,
refresh,
refresh_package,
reinstall: Reinstall::from_args(reinstall, reinstall_package),
refresh: Refresh::from_args(refresh, refresh_package),
// Shared settings.
shared: PipSharedSettings::combine(
@ -330,12 +324,9 @@ pub(crate) struct PipInstallSettings {
pub(crate) editable: Vec<String>,
pub(crate) constraint: Vec<PathBuf>,
pub(crate) r#override: Vec<PathBuf>,
pub(crate) upgrade: bool,
pub(crate) upgrade_package: Vec<PackageName>,
pub(crate) reinstall: bool,
pub(crate) reinstall_package: Vec<PackageName>,
pub(crate) refresh: bool,
pub(crate) refresh_package: Vec<PackageName>,
pub(crate) upgrade: Upgrade,
pub(crate) reinstall: Reinstall,
pub(crate) refresh: Refresh,
pub(crate) dry_run: bool,
// Shared settings.
pub(crate) shared: PipSharedSettings,
@ -404,12 +395,9 @@ impl PipInstallSettings {
editable,
constraint,
r#override,
upgrade,
upgrade_package: upgrade_package.unwrap_or_default(),
reinstall,
reinstall_package: reinstall_package.unwrap_or_default(),
refresh,
refresh_package: refresh_package.unwrap_or_default(),
upgrade: Upgrade::from_args(upgrade, upgrade_package),
reinstall: Reinstall::from_args(reinstall, reinstall_package),
refresh: Refresh::from_args(refresh, refresh_package),
dry_run,
// Shared settings.
@ -749,24 +737,19 @@ impl VenvSettings {
#[allow(clippy::struct_excessive_bools)]
#[derive(Debug, Clone)]
pub(crate) struct PipSharedSettings {
pub(crate) index_locations: IndexLocations,
pub(crate) python: Option<String>,
pub(crate) system: bool,
pub(crate) extras: ExtrasSpecification,
pub(crate) break_system_packages: bool,
pub(crate) offline: bool,
pub(crate) index_url: Option<IndexUrl>,
pub(crate) extra_index_url: Vec<IndexUrl>,
pub(crate) no_index: bool,
pub(crate) find_links: Vec<FlatIndexLocation>,
pub(crate) connectivity: Connectivity,
pub(crate) index_strategy: IndexStrategy,
pub(crate) keyring_provider: KeyringProviderType,
pub(crate) no_build: bool,
pub(crate) no_binary: Vec<PackageNameSpecifier>,
pub(crate) only_binary: Vec<PackageNameSpecifier>,
pub(crate) no_binary: NoBinary,
pub(crate) no_build: NoBuild,
pub(crate) no_build_isolation: bool,
pub(crate) strict: bool,
pub(crate) extra: Vec<ExtraName>,
pub(crate) all_extras: bool,
pub(crate) no_deps: bool,
pub(crate) dependency_mode: DependencyMode,
pub(crate) resolution: ResolutionMode,
pub(crate) prerelease: PreReleaseMode,
pub(crate) output_file: Option<PathBuf>,
@ -775,7 +758,7 @@ pub(crate) struct PipSharedSettings {
pub(crate) no_header: bool,
pub(crate) custom_compile_command: Option<String>,
pub(crate) generate_hashes: bool,
pub(crate) legacy_setup_py: bool,
pub(crate) setup_py: SetupPyStrategy,
pub(crate) config_setting: ConfigSettings,
pub(crate) python_version: Option<PythonVersion>,
pub(crate) python_platform: Option<TargetTriple>,
@ -840,9 +823,21 @@ impl PipSharedSettings {
.unwrap_or_default();
Self {
extra: args.extra.or(extra).unwrap_or_default(),
all_extras: args.all_extras.or(all_extras).unwrap_or_default(),
no_deps: args.no_deps.or(no_deps).unwrap_or_default(),
index_locations: IndexLocations::new(
args.index_url.or(index_url),
args.extra_index_url.or(extra_index_url).unwrap_or_default(),
args.find_links.or(find_links).unwrap_or_default(),
args.no_index.or(no_index).unwrap_or_default(),
),
extras: ExtrasSpecification::from_args(
args.all_extras.or(all_extras).unwrap_or_default(),
args.extra.or(extra).unwrap_or_default(),
),
dependency_mode: if args.no_deps.or(no_deps).unwrap_or_default() {
DependencyMode::Direct
} else {
DependencyMode::Transitive
},
resolution: args.resolution.or(resolution).unwrap_or_default(),
prerelease: args.prerelease.or(prerelease).unwrap_or_default(),
output_file: args.output_file.or(output_file),
@ -854,24 +849,30 @@ impl PipSharedSettings {
.annotation_style
.or(annotation_style)
.unwrap_or_default(),
offline: args.offline.or(offline).unwrap_or_default(),
index_url: args.index_url.or(index_url),
extra_index_url: args.extra_index_url.or(extra_index_url).unwrap_or_default(),
no_index: args.no_index.or(no_index).unwrap_or_default(),
connectivity: if args.offline.or(offline).unwrap_or_default() {
Connectivity::Offline
} else {
Connectivity::Online
},
index_strategy: args.index_strategy.or(index_strategy).unwrap_or_default(),
keyring_provider: args
.keyring_provider
.or(keyring_provider)
.unwrap_or_default(),
find_links: args.find_links.or(find_links).unwrap_or_default(),
generate_hashes: args.generate_hashes.or(generate_hashes).unwrap_or_default(),
legacy_setup_py: args.legacy_setup_py.or(legacy_setup_py).unwrap_or_default(),
setup_py: if args.legacy_setup_py.or(legacy_setup_py).unwrap_or_default() {
SetupPyStrategy::Setuptools
} else {
SetupPyStrategy::Pep517
},
no_build_isolation: args
.no_build_isolation
.or(no_build_isolation)
.unwrap_or_default(),
no_build: args.no_build.or(no_build).unwrap_or_default(),
only_binary: args.only_binary.or(only_binary).unwrap_or_default(),
no_build: NoBuild::from_args(
args.only_binary.or(only_binary).unwrap_or_default(),
args.no_build.or(no_build).unwrap_or_default(),
),
config_setting: args.config_settings.or(config_settings).unwrap_or_default(),
python_version: args.python_version.or(python_version),
python_platform: args.python_platform.or(python_platform),
@ -895,7 +896,7 @@ impl PipSharedSettings {
.break_system_packages
.or(break_system_packages)
.unwrap_or_default(),
no_binary: args.no_binary.or(no_binary).unwrap_or_default(),
no_binary: NoBinary::from_args(args.no_binary.or(no_binary).unwrap_or_default()),
compile_bytecode: args
.compile_bytecode
.or(compile_bytecode)