From be8725553907eacf2ae27f823bec9ff57dde0a7d Mon Sep 17 00:00:00 2001 From: John Mumm Date: Thu, 13 Mar 2025 09:10:28 +0100 Subject: [PATCH] Refactor *Settings to clarify and reduce repetition (#12138) When making changes to uv that require new (or altered) settings, there are many places in the code that need to change. This slows down work, reduces confidence in changes for new developers, and adds noise to PRs. The goal of this PR is to reduce the number of points that need to change (and that the developer needs to understand) when making changes to settings. This PR consolidates `ResolverSettings` and `ResolverInstallerSettings` by factoring out the shared settings and using a new field `resolver_settings` on `ResolverInstallerSettings`. This not only reduces repetition, but makes it easier for a human to parse the code without having to compare long lists of fields to spot differences (the difference was that `ResolverInstallerSettings` had two extra fields). This also removes `ResolverSettingsRef` and `ResolverInstallerSettingsRef`, using normal Rust references instead. For the time being, I've left `InstallerSettingsRef` in place because it appears to have a semantic meaning that might be relied upon. However, it would now be straightforward to refactor to pass `&ResolverInstallerSettings` wherever `InstallerSettingsRef` appears, further reducing sprawl. The change has the downside of adding `settings.resolver_settings.` and requiring dereferencing at various points where it was not required before (with the *SettingsRef approach). But this means there are significantly fewer places that must change to update settings. --- crates/uv/src/commands/build_frontend.rs | 22 +- crates/uv/src/commands/project/add.rs | 51 ++-- crates/uv/src/commands/project/environment.rs | 4 +- crates/uv/src/commands/project/export.rs | 2 +- crates/uv/src/commands/project/lock.rs | 44 ++-- crates/uv/src/commands/project/mod.rs | 111 ++++----- crates/uv/src/commands/project/remove.rs | 4 +- crates/uv/src/commands/project/run.rs | 10 +- crates/uv/src/commands/project/sync.rs | 9 +- crates/uv/src/commands/project/tree.rs | 2 +- crates/uv/src/commands/tool/install.rs | 22 +- crates/uv/src/commands/tool/run.rs | 1 + crates/uv/src/commands/tool/upgrade.rs | 4 +- crates/uv/src/lib.rs | 14 +- crates/uv/src/settings.rs | 225 +++++------------- crates/uv/tests/it/show_settings.rs | 52 ++-- 16 files changed, 242 insertions(+), 335 deletions(-) diff --git a/crates/uv/src/commands/build_frontend.rs b/crates/uv/src/commands/build_frontend.rs index cda03204b..69be6fda9 100644 --- a/crates/uv/src/commands/build_frontend.rs +++ b/crates/uv/src/commands/build_frontend.rs @@ -17,7 +17,7 @@ use crate::commands::project::{find_requires_python, ProjectError}; use crate::commands::reporters::PythonDownloadReporter; use crate::commands::ExitStatus; use crate::printer::Printer; -use crate::settings::{NetworkSettings, ResolverSettings, ResolverSettingsRef}; +use crate::settings::{NetworkSettings, ResolverSettings}; use uv_build_backend::check_direct_build; use uv_cache::{Cache, CacheBucket}; use uv_client::{BaseClientBuilder, FlatIndexClient, RegistryClientBuilder}; @@ -107,7 +107,7 @@ pub(crate) async fn build_frontend( hash_checking: Option, python: Option, install_mirrors: PythonInstallMirrors, - settings: ResolverSettings, + settings: &ResolverSettings, network_settings: &NetworkSettings, no_config: bool, python_preference: PythonPreference, @@ -132,7 +132,7 @@ pub(crate) async fn build_frontend( hash_checking, python.as_deref(), install_mirrors, - settings.as_ref(), + settings, network_settings, no_config, python_preference, @@ -175,7 +175,7 @@ async fn build_impl( hash_checking: Option, python_request: Option<&str>, install_mirrors: PythonInstallMirrors, - settings: ResolverSettingsRef<'_>, + settings: &ResolverSettings, network_settings: &NetworkSettings, no_config: bool, python_preference: PythonPreference, @@ -195,7 +195,7 @@ async fn build_impl( } // Extract the resolver settings. - let ResolverSettingsRef { + let ResolverSettings { index_locations, index_strategy, keyring_provider, @@ -338,20 +338,20 @@ async fn build_impl( build_logs, force_pep517, build_constraints, - no_build_isolation, + *no_build_isolation, no_build_isolation_package, network_settings, - index_strategy, - keyring_provider, - exclude_newer, - sources, + *index_strategy, + *keyring_provider, + *exclude_newer, + *sources, concurrency, build_options, sdist, wheel, list, dependency_metadata, - link_mode, + *link_mode, config_setting, preview, ); diff --git a/crates/uv/src/commands/project/add.rs b/crates/uv/src/commands/project/add.rs index b1f560e37..0c3f2013d 100644 --- a/crates/uv/src/commands/project/add.rs +++ b/crates/uv/src/commands/project/add.rs @@ -53,7 +53,7 @@ use crate::commands::project::{ use crate::commands::reporters::{PythonDownloadReporter, ResolverReporter}; use crate::commands::{diagnostics, project, ExitStatus, ScriptPath}; use crate::printer::Printer; -use crate::settings::{NetworkSettings, ResolverInstallerSettings, ResolverInstallerSettingsRef}; +use crate::settings::{NetworkSettings, ResolverInstallerSettings}; /// Add one or more packages to the project requirements. #[allow(clippy::fn_params_excessive_bools)] @@ -254,7 +254,7 @@ pub(crate) async fn add( let client_builder = BaseClientBuilder::new() .connectivity(network_settings.connectivity) .native_tls(network_settings.native_tls) - .keyring(settings.keyring_provider) + .keyring(settings.resolver.keyring_provider) .allow_insecure_host(network_settings.allow_insecure_host.clone()); // Read the requirements. @@ -295,7 +295,7 @@ pub(crate) async fn add( let sources = SourceStrategy::Enabled; // Add all authenticated sources to the cache. - for index in settings.index_locations.allowed_indexes() { + for index in settings.resolver.index_locations.allowed_indexes() { if let Some(credentials) = index.credentials() { let credentials = Arc::new(credentials); uv_auth::store_credentials(index.raw_url(), credentials.clone()); @@ -307,31 +307,40 @@ pub(crate) async fn add( // Initialize the registry client. let client = RegistryClientBuilder::try_from(client_builder)? - .index_urls(settings.index_locations.index_urls()) - .index_strategy(settings.index_strategy) + .index_urls(settings.resolver.index_locations.index_urls()) + .index_strategy(settings.resolver.index_strategy) .markers(target.interpreter().markers()) .platform(target.interpreter().platform()) .build(); // Determine whether to enable build isolation. let environment; - let build_isolation = if settings.no_build_isolation { + let build_isolation = if settings.resolver.no_build_isolation { environment = PythonEnvironment::from_interpreter(target.interpreter().clone()); BuildIsolation::Shared(&environment) - } else if settings.no_build_isolation_package.is_empty() { + } else if settings.resolver.no_build_isolation_package.is_empty() { BuildIsolation::Isolated } else { environment = PythonEnvironment::from_interpreter(target.interpreter().clone()); - BuildIsolation::SharedPackage(&environment, &settings.no_build_isolation_package) + BuildIsolation::SharedPackage( + &environment, + &settings.resolver.no_build_isolation_package, + ) }; // Resolve the flat indexes from `--find-links`. let flat_index = { let client = FlatIndexClient::new(&client, cache); let entries = client - .fetch(settings.index_locations.flat_indexes().map(Index::url)) + .fetch( + settings + .resolver + .index_locations + .flat_indexes() + .map(Index::url), + ) .await?; - FlatIndex::from_entries(entries, None, &hasher, &settings.build_options) + FlatIndex::from_entries(entries, None, &hasher, &settings.resolver.build_options) }; // Create a build dispatch. @@ -340,17 +349,17 @@ pub(crate) async fn add( cache, build_constraints, target.interpreter(), - &settings.index_locations, + &settings.resolver.index_locations, &flat_index, - &settings.dependency_metadata, + &settings.resolver.dependency_metadata, state.clone().into_inner(), - settings.index_strategy, - &settings.config_setting, + settings.resolver.index_strategy, + &settings.resolver.config_setting, build_isolation, - settings.link_mode, - &settings.build_options, + settings.resolver.link_mode, + &settings.resolver.build_options, &build_hasher, - settings.exclude_newer, + settings.resolver.exclude_newer, sources, // No workspace caching since `uv add` changes the workspace definition. WorkspaceCache::default(), @@ -637,7 +646,7 @@ pub(crate) async fn add( locked, &dependency_type, raw_sources, - settings.as_ref(), + &settings, &network_settings, installer_metadata, concurrency, @@ -673,7 +682,7 @@ async fn lock_and_sync( locked: bool, dependency_type: &DependencyType, raw_sources: bool, - settings: ResolverInstallerSettingsRef<'_>, + settings: &ResolverInstallerSettings, network_settings: &NetworkSettings, installer_metadata: bool, concurrency: Concurrency, @@ -688,7 +697,7 @@ async fn lock_and_sync( LockMode::Write(target.interpreter()) }, (&target).into(), - settings.into(), + &settings.resolver, network_settings, &lock_state, Box::new(DefaultResolveLogger), @@ -806,7 +815,7 @@ async fn lock_and_sync( LockMode::Write(target.interpreter()) }, (&target).into(), - settings.into(), + &settings.resolver, network_settings, &lock_state, Box::new(SummaryResolveLogger), diff --git a/crates/uv/src/commands/project/environment.rs b/crates/uv/src/commands/project/environment.rs index ee4e9213f..49b7cc236 100644 --- a/crates/uv/src/commands/project/environment.rs +++ b/crates/uv/src/commands/project/environment.rs @@ -47,7 +47,7 @@ impl CachedEnvironment { resolve_environment( spec, &interpreter, - settings.as_ref().into(), + &settings.resolver, network_settings, state, resolve, @@ -99,7 +99,7 @@ impl CachedEnvironment { venv, &resolution, Modifications::Exact, - settings.as_ref().into(), + settings.into(), network_settings, state, install, diff --git a/crates/uv/src/commands/project/export.rs b/crates/uv/src/commands/project/export.rs index f7d5e0bba..93af04dab 100644 --- a/crates/uv/src/commands/project/export.rs +++ b/crates/uv/src/commands/project/export.rs @@ -172,7 +172,7 @@ pub(crate) async fn export( let lock = match do_safe_lock( mode, (&target).into(), - settings.as_ref(), + &settings, &network_settings, &state, Box::new(DefaultResolveLogger), diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index 3e7090dc8..6b3576d82 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -48,7 +48,7 @@ use crate::commands::project::{ use crate::commands::reporters::{PythonDownloadReporter, ResolverReporter}; use crate::commands::{diagnostics, pip, ExitStatus, ScriptPath}; use crate::printer::Printer; -use crate::settings::{NetworkSettings, ResolverSettings, ResolverSettingsRef}; +use crate::settings::{NetworkSettings, ResolverSettings}; /// The result of running a lock operation. #[derive(Debug, Clone)] @@ -187,7 +187,7 @@ pub(crate) async fn lock( match do_safe_lock( mode, target, - settings.as_ref(), + &settings, &network_settings, &state, Box::new(DefaultResolveLogger), @@ -251,7 +251,7 @@ pub(super) enum LockMode<'env> { pub(super) async fn do_safe_lock( mode: LockMode<'_>, target: LockTarget<'_>, - settings: ResolverSettingsRef<'_>, + settings: &ResolverSettings, network_settings: &NetworkSettings, state: &UniversalState, logger: Box, @@ -346,7 +346,7 @@ async fn do_lock( target: LockTarget<'_>, interpreter: &Interpreter, existing_lock: Option, - settings: ResolverSettingsRef<'_>, + settings: &ResolverSettings, network_settings: &NetworkSettings, state: &UniversalState, logger: Box, @@ -358,7 +358,7 @@ async fn do_lock( let start = std::time::Instant::now(); // Extract the project settings. - let ResolverSettingsRef { + let ResolverSettings { index_locations, index_strategy, keyring_provider, @@ -387,14 +387,14 @@ async fn do_lock( let source_trees = vec![]; // If necessary, lower the overrides and constraints. - let requirements = target.lower(requirements, index_locations, sources)?; - let overrides = target.lower(overrides, index_locations, sources)?; - let constraints = target.lower(constraints, index_locations, sources)?; - let build_constraints = target.lower(build_constraints, index_locations, sources)?; + let requirements = target.lower(requirements, index_locations, *sources)?; + let overrides = target.lower(overrides, index_locations, *sources)?; + let constraints = target.lower(constraints, index_locations, *sources)?; + let build_constraints = target.lower(build_constraints, index_locations, *sources)?; let dependency_groups = dependency_groups .into_iter() .map(|(name, requirements)| { - let requirements = target.lower(requirements, index_locations, sources)?; + let requirements = target.lower(requirements, index_locations, *sources)?; Ok((name, requirements)) }) .collect::, ProjectError>>()?; @@ -553,15 +553,15 @@ async fn do_lock( .allow_insecure_host(network_settings.allow_insecure_host.clone()) .url_auth_policies(UrlAuthPolicies::from(index_locations)) .index_urls(index_locations.index_urls()) - .index_strategy(index_strategy) - .keyring(keyring_provider) + .index_strategy(*index_strategy) + .keyring(*keyring_provider) .markers(interpreter.markers()) .platform(interpreter.platform()) .build(); // Determine whether to enable build isolation. let environment; - let build_isolation = if no_build_isolation { + let build_isolation = if *no_build_isolation { environment = PythonEnvironment::from_interpreter(interpreter.clone()); BuildIsolation::Shared(&environment) } else if no_build_isolation_package.is_empty() { @@ -572,11 +572,11 @@ async fn do_lock( }; let options = OptionsBuilder::new() - .resolution_mode(resolution) - .prerelease_mode(prerelease) - .fork_strategy(fork_strategy) - .exclude_newer(exclude_newer) - .index_strategy(index_strategy) + .resolution_mode(*resolution) + .prerelease_mode(*prerelease) + .fork_strategy(*fork_strategy) + .exclude_newer(*exclude_newer) + .index_strategy(*index_strategy) .build_options(build_options.clone()) .required_environments(required_environments.cloned().unwrap_or_default()) .build(); @@ -611,14 +611,14 @@ async fn do_lock( &flat_index, dependency_metadata, state.fork().into_inner(), - index_strategy, + *index_strategy, config_setting, build_isolation, - link_mode, + *link_mode, build_options, &build_hasher, - exclude_newer, - sources, + *exclude_newer, + *sources, workspace_cache, concurrency, preview, diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index dfa8cdc75..ec1f42548 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -54,7 +54,7 @@ use crate::commands::reporters::{PythonDownloadReporter, ResolverReporter}; use crate::commands::{capitalize, conjunction, pip}; use crate::printer::Printer; use crate::settings::{ - InstallerSettingsRef, NetworkSettings, ResolverInstallerSettings, ResolverSettingsRef, + InstallerSettingsRef, NetworkSettings, ResolverInstallerSettings, ResolverSettings, }; pub(crate) mod add; @@ -1504,23 +1504,26 @@ pub(crate) async fn resolve_names( // Extract the project settings. let ResolverInstallerSettings { - index_locations, - index_strategy, - keyring_provider, - resolution: _, - prerelease: _, - fork_strategy: _, - dependency_metadata, - config_setting, - no_build_isolation, - no_build_isolation_package, - exclude_newer, - link_mode, + resolver: + ResolverSettings { + build_options, + config_setting, + dependency_metadata, + exclude_newer, + fork_strategy: _, + index_locations, + index_strategy, + keyring_provider, + link_mode, + no_build_isolation, + no_build_isolation_package, + prerelease: _, + resolution: _, + sources, + upgrade: _, + }, compile_bytecode: _, - sources, - upgrade: _, reinstall: _, - build_options, } = settings; // Add all authenticated sources to the cache. @@ -1632,7 +1635,7 @@ impl<'lock> EnvironmentSpecification<'lock> { pub(crate) async fn resolve_environment( spec: EnvironmentSpecification<'_>, interpreter: &Interpreter, - settings: ResolverSettingsRef<'_>, + settings: &ResolverSettings, network_settings: &NetworkSettings, state: &PlatformState, logger: Box, @@ -1643,7 +1646,7 @@ pub(crate) async fn resolve_environment( ) -> Result { warn_on_requirements_txt_setting(&spec.requirements, settings); - let ResolverSettingsRef { + let ResolverSettings { index_locations, index_strategy, keyring_provider, @@ -1694,15 +1697,15 @@ pub(crate) async fn resolve_environment( .allow_insecure_host(network_settings.allow_insecure_host.clone()) .url_auth_policies(UrlAuthPolicies::from(index_locations)) .index_urls(index_locations.index_urls()) - .index_strategy(index_strategy) - .keyring(keyring_provider) + .index_strategy(*index_strategy) + .keyring(*keyring_provider) .markers(interpreter.markers()) .platform(interpreter.platform()) .build(); // Determine whether to enable build isolation. let environment; - let build_isolation = if no_build_isolation { + let build_isolation = if *no_build_isolation { environment = PythonEnvironment::from_interpreter(interpreter.clone()); BuildIsolation::Shared(&environment) } else if no_build_isolation_package.is_empty() { @@ -1713,11 +1716,11 @@ pub(crate) async fn resolve_environment( }; let options = OptionsBuilder::new() - .resolution_mode(resolution) - .prerelease_mode(prerelease) - .fork_strategy(fork_strategy) - .exclude_newer(exclude_newer) - .index_strategy(index_strategy) + .resolution_mode(*resolution) + .prerelease_mode(*prerelease) + .fork_strategy(*fork_strategy) + .exclude_newer(*exclude_newer) + .index_strategy(*index_strategy) .build_options(build_options.clone()) .build(); @@ -1768,14 +1771,14 @@ pub(crate) async fn resolve_environment( &flat_index, dependency_metadata, state.clone().into_inner(), - index_strategy, + *index_strategy, config_setting, build_isolation, - link_mode, + *link_mode, build_options, &build_hasher, - exclude_newer, - sources, + *exclude_newer, + *sources, workspace_cache, concurrency, preview, @@ -1988,26 +1991,29 @@ pub(crate) async fn update_environment( printer: Printer, preview: PreviewMode, ) -> Result { - warn_on_requirements_txt_setting(&spec, settings.as_ref().into()); + warn_on_requirements_txt_setting(&spec, &settings.resolver); let ResolverInstallerSettings { - index_locations, - index_strategy, - keyring_provider, - resolution, - prerelease, - fork_strategy, - dependency_metadata, - config_setting, - no_build_isolation, - no_build_isolation_package, - exclude_newer, - link_mode, + resolver: + ResolverSettings { + build_options, + config_setting, + dependency_metadata, + exclude_newer, + fork_strategy, + index_locations, + index_strategy, + keyring_provider, + link_mode, + no_build_isolation, + no_build_isolation_package, + prerelease, + resolution, + sources, + upgrade, + }, compile_bytecode, - sources, - upgrade, reinstall, - build_options, } = settings; // Respect all requirements from the provided sources. @@ -2333,7 +2339,7 @@ pub(crate) fn detect_conflicts( #[allow(clippy::result_large_err)] pub(crate) fn script_specification( script: Pep723ItemRef<'_>, - settings: ResolverSettingsRef, + settings: &ResolverSettings, ) -> Result, ProjectError> { let Some(dependencies) = script.metadata().dependencies.as_ref() else { return Ok(None); @@ -2383,7 +2389,7 @@ pub(crate) fn script_specification( script_dir.as_ref(), script_sources, script_indexes, - settings.index_locations, + &settings.index_locations, ) .map_ok(LoweredRequirement::into_inner) }) @@ -2403,7 +2409,7 @@ pub(crate) fn script_specification( script_dir.as_ref(), script_sources, script_indexes, - settings.index_locations, + &settings.index_locations, ) .map_ok(LoweredRequirement::into_inner) }) @@ -2423,7 +2429,7 @@ pub(crate) fn script_specification( script_dir.as_ref(), script_sources, script_indexes, - settings.index_locations, + &settings.index_locations, ) .map_ok(LoweredRequirement::into_inner) }) @@ -2437,10 +2443,7 @@ pub(crate) fn script_specification( } /// Warn if the user provides (e.g.) an `--index-url` in a requirements file. -fn warn_on_requirements_txt_setting( - spec: &RequirementsSpecification, - settings: ResolverSettingsRef<'_>, -) { +fn warn_on_requirements_txt_setting(spec: &RequirementsSpecification, settings: &ResolverSettings) { let RequirementsSpecification { index_url, extra_index_urls, diff --git a/crates/uv/src/commands/project/remove.rs b/crates/uv/src/commands/project/remove.rs index 19c6e1ec3..d3d7cec97 100644 --- a/crates/uv/src/commands/project/remove.rs +++ b/crates/uv/src/commands/project/remove.rs @@ -279,7 +279,7 @@ pub(crate) async fn remove( let lock = match project::lock::do_safe_lock( mode, (&target).into(), - settings.as_ref().into(), + &settings.resolver, &network_settings, &state, Box::new(DefaultResolveLogger), @@ -340,7 +340,7 @@ pub(crate) async fn remove( EditableMode::Editable, install_options, Modifications::Exact, - settings.as_ref().into(), + (&settings).into(), &network_settings, &state, Box::new(DefaultInstallLogger), diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index 9e2460630..05be2ca09 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -245,7 +245,7 @@ hint: If you are running a script with `{}` in the shebang, you may need to incl let lock = match project::lock::do_safe_lock( mode, target, - settings.as_ref().into(), + &settings.resolver, &network_settings, &lock_state, if show_resolution { @@ -288,7 +288,7 @@ hint: If you are running a script with `{}` in the shebang, you may need to incl editable, install_options, modifications, - settings.as_ref().into(), + (&settings).into(), &network_settings, &sync_state, if show_resolution { @@ -334,7 +334,7 @@ hint: If you are running a script with `{}` in the shebang, you may need to incl } // Install the script requirements, if necessary. Otherwise, use an isolated environment. - if let Some(spec) = script_specification((&script).into(), settings.as_ref().into())? { + if let Some(spec) = script_specification((&script).into(), &settings.resolver)? { let environment = ScriptEnvironment::get_or_init( (&script).into(), python.as_deref().map(PythonRequest::parse), @@ -664,7 +664,7 @@ hint: If you are running a script with `{}` in the shebang, you may need to incl let result = match project::lock::do_safe_lock( mode, project.workspace().into(), - settings.as_ref().into(), + &settings.resolver, &network_settings, &lock_state, if show_resolution { @@ -750,7 +750,7 @@ hint: If you are running a script with `{}` in the shebang, you may need to incl editable, install_options, modifications, - settings.as_ref().into(), + (&settings).into(), &network_settings, &sync_state, if show_resolution { diff --git a/crates/uv/src/commands/project/sync.rs b/crates/uv/src/commands/project/sync.rs index 9e992e122..6b742d213 100644 --- a/crates/uv/src/commands/project/sync.rs +++ b/crates/uv/src/commands/project/sync.rs @@ -274,9 +274,8 @@ pub(crate) async fn sync( )); } - let spec = - script_specification(Pep723ItemRef::Script(script), settings.as_ref().into())? - .unwrap_or_default(); + let spec = script_specification(Pep723ItemRef::Script(script), &settings.resolver)? + .unwrap_or_default(); match update_environment( Deref::deref(&environment).clone(), spec, @@ -331,7 +330,7 @@ pub(crate) async fn sync( let lock = match do_safe_lock( mode, lock_target, - settings.as_ref().into(), + &settings.resolver, &network_settings, &state, Box::new(DefaultResolveLogger), @@ -454,7 +453,7 @@ pub(crate) async fn sync( editable, install_options, modifications, - settings.as_ref().into(), + (&settings).into(), &network_settings, &state, Box::new(DefaultInstallLogger), diff --git a/crates/uv/src/commands/project/tree.rs b/crates/uv/src/commands/project/tree.rs index 746ad8deb..a94bc0acf 100644 --- a/crates/uv/src/commands/project/tree.rs +++ b/crates/uv/src/commands/project/tree.rs @@ -135,7 +135,7 @@ pub(crate) async fn tree( let lock = match do_safe_lock( mode, target, - settings.as_ref(), + &settings, network_settings, &state, Box::new(DefaultResolveLogger), diff --git a/crates/uv/src/commands/tool/install.rs b/crates/uv/src/commands/tool/install.rs index b480a47a0..8dd6c427d 100644 --- a/crates/uv/src/commands/tool/install.rs +++ b/crates/uv/src/commands/tool/install.rs @@ -34,7 +34,7 @@ use crate::commands::tool::{Target, ToolRequest}; use crate::commands::ExitStatus; use crate::commands::{diagnostics, reporters::PythonDownloadReporter}; use crate::printer::Printer; -use crate::settings::{NetworkSettings, ResolverInstallerSettings}; +use crate::settings::{NetworkSettings, ResolverInstallerSettings, ResolverSettings}; /// Install a tool. #[allow(clippy::fn_params_excessive_bools)] @@ -208,9 +208,13 @@ pub(crate) async fn install( // If the user passed, e.g., `ruff@latest`, we need to mark it as upgradable. let settings = if request.is_latest() { ResolverInstallerSettings { - upgrade: settings - .upgrade - .combine(Upgrade::package(from.name.clone())), + resolver: ResolverSettings { + upgrade: settings + .resolver + .upgrade + .combine(Upgrade::package(from.name.clone())), + ..settings.resolver + }, ..settings } } else { @@ -340,7 +344,9 @@ pub(crate) async fn install( .as_ref() .filter(|_| { // And the user didn't request a reinstall or upgrade... - !request.is_latest() && settings.reinstall.is_none() && settings.upgrade.is_none() + !request.is_latest() + && settings.reinstall.is_none() + && settings.resolver.upgrade.is_none() }) .is_some() { @@ -435,7 +441,7 @@ pub(crate) async fn install( let resolution = resolve_environment( spec.clone(), &interpreter, - settings.as_ref().into(), + &settings.resolver, &network_settings, &state, Box::new(DefaultResolveLogger), @@ -487,7 +493,7 @@ pub(crate) async fn install( match resolve_environment( spec, &interpreter, - settings.as_ref().into(), + &settings.resolver, &network_settings, &state, Box::new(DefaultResolveLogger), @@ -526,7 +532,7 @@ pub(crate) async fn install( environment, &resolution.into(), Modifications::Exact, - settings.as_ref().into(), + (&settings).into(), &network_settings, &state, Box::new(DefaultInstallLogger), diff --git a/crates/uv/src/commands/tool/run.rs b/crates/uv/src/commands/tool/run.rs index 46f0d22d4..0d6c6e470 100644 --- a/crates/uv/src/commands/tool/run.rs +++ b/crates/uv/src/commands/tool/run.rs @@ -166,6 +166,7 @@ pub(crate) async fn run( && invocation_source == ToolRunCommand::Uvx && target == "run" && settings + .resolver .index_locations .indexes() .all(|index| matches!(index.url, IndexUrl::Pypi(..))) diff --git a/crates/uv/src/commands/tool/upgrade.rs b/crates/uv/src/commands/tool/upgrade.rs index 5979e48d1..8d83f871a 100644 --- a/crates/uv/src/commands/tool/upgrade.rs +++ b/crates/uv/src/commands/tool/upgrade.rs @@ -293,7 +293,7 @@ async fn upgrade_tool( let resolution = resolve_environment( spec.into(), interpreter, - settings.as_ref().into(), + &settings.resolver, network_settings, &state, Box::new(SummaryResolveLogger), @@ -310,7 +310,7 @@ async fn upgrade_tool( environment, &resolution.into(), Modifications::Exact, - settings.as_ref().into(), + (&settings).into(), network_settings, &state, Box::new(DefaultInstallLogger), diff --git a/crates/uv/src/lib.rs b/crates/uv/src/lib.rs index 83803d338..0c27b352d 100644 --- a/crates/uv/src/lib.rs +++ b/crates/uv/src/lib.rs @@ -816,7 +816,7 @@ async fn run(mut cli: Cli) -> Result { args.hash_checking, args.python, args.install_mirrors, - args.settings, + &args.settings, &globals.network_settings, cli.top_level.no_config, globals.python_preference, @@ -966,7 +966,7 @@ async fn run(mut cli: Cli) -> Result { let cache = cache.init()?.with_refresh( args.refresh .combine(Refresh::from(args.settings.reinstall.clone())) - .combine(Refresh::from(args.settings.upgrade.clone())), + .combine(Refresh::from(args.settings.resolver.upgrade.clone())), ); let requirements = { @@ -1034,7 +1034,7 @@ async fn run(mut cli: Cli) -> Result { let cache = cache.init()?.with_refresh( args.refresh .combine(Refresh::from(args.settings.reinstall.clone())) - .combine(Refresh::from(args.settings.upgrade.clone())), + .combine(Refresh::from(args.settings.resolver.upgrade.clone())), ); let mut requirements = Vec::with_capacity( @@ -1456,7 +1456,7 @@ async fn run_project( let cache = cache.init()?.with_refresh( args.refresh .combine(Refresh::from(args.settings.reinstall.clone())) - .combine(Refresh::from(args.settings.upgrade.clone())), + .combine(Refresh::from(args.settings.resolver.upgrade.clone())), ); let mut requirements = Vec::with_capacity( @@ -1521,7 +1521,7 @@ async fn run_project( let cache = cache.init()?.with_refresh( args.refresh .combine(Refresh::from(args.settings.reinstall.clone())) - .combine(Refresh::from(args.settings.upgrade.clone())), + .combine(Refresh::from(args.settings.resolver.upgrade.clone())), ); // Unwrap the script. @@ -1613,7 +1613,7 @@ async fn run_project( let cache = cache.init()?.with_refresh( args.refresh .combine(Refresh::from(args.settings.reinstall.clone())) - .combine(Refresh::from(args.settings.upgrade.clone())), + .combine(Refresh::from(args.settings.resolver.upgrade.clone())), ); // If the script already exists, use it; otherwise, propagate the file path and we'll @@ -1682,7 +1682,7 @@ async fn run_project( let cache = cache.init()?.with_refresh( args.refresh .combine(Refresh::from(args.settings.reinstall.clone())) - .combine(Refresh::from(args.settings.upgrade.clone())), + .combine(Refresh::from(args.settings.resolver.upgrade.clone())), ); // Unwrap the script. diff --git a/crates/uv/src/settings.rs b/crates/uv/src/settings.rs index d98f0e918..c1053f5b9 100644 --- a/crates/uv/src/settings.rs +++ b/crates/uv/src/settings.rs @@ -2404,40 +2404,21 @@ pub(crate) struct InstallerSettingsRef<'a> { #[allow(clippy::struct_excessive_bools)] #[derive(Debug, Clone, Default)] pub(crate) struct ResolverSettings { + pub(crate) build_options: BuildOptions, + pub(crate) config_setting: ConfigSettings, + pub(crate) dependency_metadata: DependencyMetadata, + pub(crate) exclude_newer: Option, + pub(crate) fork_strategy: ForkStrategy, pub(crate) index_locations: IndexLocations, pub(crate) index_strategy: IndexStrategy, pub(crate) keyring_provider: KeyringProviderType, - pub(crate) resolution: ResolutionMode, - pub(crate) prerelease: PrereleaseMode, - pub(crate) fork_strategy: ForkStrategy, - pub(crate) dependency_metadata: DependencyMetadata, - pub(crate) config_setting: ConfigSettings, + pub(crate) link_mode: LinkMode, pub(crate) no_build_isolation: bool, pub(crate) no_build_isolation_package: Vec, - pub(crate) exclude_newer: Option, - pub(crate) link_mode: LinkMode, - pub(crate) upgrade: Upgrade, - pub(crate) build_options: BuildOptions, - pub(crate) sources: SourceStrategy, -} - -#[derive(Debug, Clone, Copy)] -pub(crate) struct ResolverSettingsRef<'a> { - pub(crate) index_locations: &'a IndexLocations, - pub(crate) index_strategy: IndexStrategy, - pub(crate) keyring_provider: KeyringProviderType, - pub(crate) resolution: ResolutionMode, pub(crate) prerelease: PrereleaseMode, - pub(crate) fork_strategy: ForkStrategy, - pub(crate) dependency_metadata: &'a DependencyMetadata, - pub(crate) config_setting: &'a ConfigSettings, - pub(crate) no_build_isolation: bool, - pub(crate) no_build_isolation_package: &'a [PackageName], - pub(crate) exclude_newer: Option, - pub(crate) link_mode: LinkMode, - pub(crate) upgrade: &'a Upgrade, - pub(crate) build_options: &'a BuildOptions, + pub(crate) resolution: ResolutionMode, pub(crate) sources: SourceStrategy, + pub(crate) upgrade: Upgrade, } impl ResolverSettings { @@ -2452,26 +2433,6 @@ impl ResolverSettings { Self::from(options) } - - pub(crate) fn as_ref(&self) -> ResolverSettingsRef { - ResolverSettingsRef { - index_locations: &self.index_locations, - index_strategy: self.index_strategy, - keyring_provider: self.keyring_provider, - resolution: self.resolution, - prerelease: self.prerelease, - fork_strategy: self.fork_strategy, - dependency_metadata: &self.dependency_metadata, - config_setting: &self.config_setting, - no_build_isolation: self.no_build_isolation, - no_build_isolation_package: &self.no_build_isolation_package, - exclude_newer: self.exclude_newer, - link_mode: self.link_mode, - upgrade: &self.upgrade, - build_options: &self.build_options, - sources: self.sources, - } - } } impl From for ResolverSettings { @@ -2525,27 +2486,6 @@ impl From for ResolverSettings { } } -#[derive(Debug, Clone, Copy)] -pub(crate) struct ResolverInstallerSettingsRef<'a> { - pub(crate) index_locations: &'a IndexLocations, - pub(crate) index_strategy: IndexStrategy, - pub(crate) keyring_provider: KeyringProviderType, - pub(crate) resolution: ResolutionMode, - pub(crate) prerelease: PrereleaseMode, - pub(crate) fork_strategy: ForkStrategy, - pub(crate) dependency_metadata: &'a DependencyMetadata, - pub(crate) config_setting: &'a ConfigSettings, - pub(crate) no_build_isolation: bool, - pub(crate) no_build_isolation_package: &'a [PackageName], - pub(crate) exclude_newer: Option, - pub(crate) link_mode: LinkMode, - pub(crate) compile_bytecode: bool, - pub(crate) sources: SourceStrategy, - pub(crate) upgrade: &'a Upgrade, - pub(crate) reinstall: &'a Reinstall, - pub(crate) build_options: &'a BuildOptions, -} - /// The resolved settings to use for an invocation of the uv CLI with both resolver and installer /// capabilities. /// @@ -2554,23 +2494,9 @@ pub(crate) struct ResolverInstallerSettingsRef<'a> { #[allow(clippy::struct_excessive_bools)] #[derive(Debug, Clone, Default)] pub(crate) struct ResolverInstallerSettings { - pub(crate) index_locations: IndexLocations, - pub(crate) index_strategy: IndexStrategy, - pub(crate) keyring_provider: KeyringProviderType, - pub(crate) resolution: ResolutionMode, - pub(crate) prerelease: PrereleaseMode, - pub(crate) fork_strategy: ForkStrategy, - pub(crate) dependency_metadata: DependencyMetadata, - pub(crate) config_setting: ConfigSettings, - pub(crate) no_build_isolation: bool, - pub(crate) no_build_isolation_package: Vec, - pub(crate) exclude_newer: Option, - pub(crate) link_mode: LinkMode, + pub(crate) resolver: ResolverSettings, pub(crate) compile_bytecode: bool, - pub(crate) sources: SourceStrategy, - pub(crate) upgrade: Upgrade, pub(crate) reinstall: Reinstall, - pub(crate) build_options: BuildOptions, } impl ResolverInstallerSettings { @@ -2588,28 +2514,6 @@ impl ResolverInstallerSettings { Self::from(options) } - - pub(crate) fn as_ref(&self) -> ResolverInstallerSettingsRef { - ResolverInstallerSettingsRef { - index_locations: &self.index_locations, - index_strategy: self.index_strategy, - keyring_provider: self.keyring_provider, - resolution: self.resolution, - prerelease: self.prerelease, - fork_strategy: self.fork_strategy, - dependency_metadata: &self.dependency_metadata, - config_setting: &self.config_setting, - no_build_isolation: self.no_build_isolation, - no_build_isolation_package: &self.no_build_isolation_package, - exclude_newer: self.exclude_newer, - link_mode: self.link_mode, - compile_bytecode: self.compile_bytecode, - sources: self.sources, - upgrade: &self.upgrade, - reinstall: &self.reinstall, - build_options: &self.build_options, - } - } } impl From for ResolverInstallerSettings { @@ -2631,39 +2535,44 @@ impl From for ResolverInstallerSettings { value.no_index.unwrap_or_default(), ); Self { - index_locations, - resolution: value.resolution.unwrap_or_default(), - prerelease: value.prerelease.unwrap_or_default(), - fork_strategy: value.fork_strategy.unwrap_or_default(), - dependency_metadata: DependencyMetadata::from_entries( - value.dependency_metadata.into_iter().flatten(), - ), - index_strategy: value.index_strategy.unwrap_or_default(), - keyring_provider: value.keyring_provider.unwrap_or_default(), - config_setting: value.config_settings.unwrap_or_default(), - no_build_isolation: value.no_build_isolation.unwrap_or_default(), - no_build_isolation_package: value.no_build_isolation_package.unwrap_or_default(), - exclude_newer: value.exclude_newer, - link_mode: value.link_mode.unwrap_or_default(), - sources: SourceStrategy::from_args(value.no_sources.unwrap_or_default()), + resolver: ResolverSettings { + 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()), + ), + config_setting: value.config_settings.unwrap_or_default(), + dependency_metadata: DependencyMetadata::from_entries( + value.dependency_metadata.into_iter().flatten(), + ), + exclude_newer: value.exclude_newer, + fork_strategy: value.fork_strategy.unwrap_or_default(), + index_locations, + index_strategy: value.index_strategy.unwrap_or_default(), + keyring_provider: value.keyring_provider.unwrap_or_default(), + link_mode: value.link_mode.unwrap_or_default(), + no_build_isolation: value.no_build_isolation.unwrap_or_default(), + no_build_isolation_package: value.no_build_isolation_package.unwrap_or_default(), + 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_args( + value.upgrade, + value + .upgrade_package + .into_iter() + .flatten() + .map(Requirement::from) + .collect(), + ), + }, compile_bytecode: value.compile_bytecode.unwrap_or_default(), - upgrade: Upgrade::from_args( - value.upgrade, - value - .upgrade_package - .into_iter() - .flatten() - .map(Requirement::from) - .collect(), - ), reinstall: Reinstall::from_args( value.reinstall, value.reinstall_package.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()), - ), } } } @@ -3028,44 +2937,22 @@ impl PipSettings { } } -impl<'a> From> for ResolverSettingsRef<'a> { - fn from(settings: ResolverInstallerSettingsRef<'a>) -> Self { +impl<'a> From<&'a ResolverInstallerSettings> for InstallerSettingsRef<'a> { + fn from(settings: &'a ResolverInstallerSettings) -> Self { Self { - index_locations: settings.index_locations, - index_strategy: settings.index_strategy, - keyring_provider: settings.keyring_provider, - resolution: settings.resolution, - prerelease: settings.prerelease, - fork_strategy: settings.fork_strategy, - dependency_metadata: settings.dependency_metadata, - config_setting: settings.config_setting, - no_build_isolation: settings.no_build_isolation, - no_build_isolation_package: settings.no_build_isolation_package, - exclude_newer: settings.exclude_newer, - link_mode: settings.link_mode, - upgrade: settings.upgrade, - build_options: settings.build_options, - sources: settings.sources, - } - } -} - -impl<'a> From> for InstallerSettingsRef<'a> { - fn from(settings: ResolverInstallerSettingsRef<'a>) -> Self { - Self { - index_locations: settings.index_locations, - index_strategy: settings.index_strategy, - keyring_provider: settings.keyring_provider, - dependency_metadata: settings.dependency_metadata, - config_setting: settings.config_setting, - no_build_isolation: settings.no_build_isolation, - no_build_isolation_package: settings.no_build_isolation_package, - exclude_newer: settings.exclude_newer, - link_mode: settings.link_mode, + index_locations: &settings.resolver.index_locations, + index_strategy: settings.resolver.index_strategy, + keyring_provider: settings.resolver.keyring_provider, + dependency_metadata: &settings.resolver.dependency_metadata, + config_setting: &settings.resolver.config_setting, + no_build_isolation: settings.resolver.no_build_isolation, + no_build_isolation_package: &settings.resolver.no_build_isolation_package, + exclude_newer: settings.resolver.exclude_newer, + link_mode: settings.resolver.link_mode, compile_bytecode: settings.compile_bytecode, - reinstall: settings.reinstall, - build_options: settings.build_options, - sources: settings.sources, + reinstall: &settings.reinstall, + build_options: &settings.resolver.build_options, + sources: settings.resolver.sources, } } } diff --git a/crates/uv/tests/it/show_settings.rs b/crates/uv/tests/it/show_settings.rs index 063482043..e57eb2693 100644 --- a/crates/uv/tests/it/show_settings.rs +++ b/crates/uv/tests/it/show_settings.rs @@ -3114,34 +3114,36 @@ fn resolve_tool() -> anyhow::Result<()> { no_binary_package: None, }, settings: ResolverInstallerSettings { - index_locations: IndexLocations { - indexes: [], - flat_index: [], - no_index: false, + resolver: ResolverSettings { + build_options: BuildOptions { + no_binary: None, + no_build: None, + }, + config_setting: ConfigSettings( + {}, + ), + dependency_metadata: DependencyMetadata( + {}, + ), + exclude_newer: None, + fork_strategy: RequiresPython, + index_locations: IndexLocations { + indexes: [], + flat_index: [], + no_index: false, + }, + index_strategy: FirstIndex, + keyring_provider: Disabled, + link_mode: Clone, + no_build_isolation: false, + no_build_isolation_package: [], + prerelease: IfNecessaryOrExplicit, + resolution: LowestDirect, + sources: Enabled, + upgrade: None, }, - index_strategy: FirstIndex, - keyring_provider: Disabled, - resolution: LowestDirect, - prerelease: IfNecessaryOrExplicit, - fork_strategy: RequiresPython, - dependency_metadata: DependencyMetadata( - {}, - ), - config_setting: ConfigSettings( - {}, - ), - no_build_isolation: false, - no_build_isolation_package: [], - exclude_newer: None, - link_mode: Clone, compile_bytecode: false, - sources: Enabled, - upgrade: None, reinstall: None, - build_options: BuildOptions { - no_binary: None, - no_build: None, - }, }, force: false, editable: false,