From b5d280dc403f7c88feb54b39301e700ba3c872f5 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Mon, 17 Jun 2024 12:16:15 -0400 Subject: [PATCH] Refactor `Toolchain` API to always take `ToolchainRequest` instead of `str` (#4341) This API was taking an `Option<&str>` for caller convenience in some places but we ought to just take a `ToolchainRequest` consistently. --- crates/uv-toolchain/src/lib.rs | 52 +++++++++++++------------ crates/uv-toolchain/src/toolchain.rs | 11 +++--- crates/uv/src/commands/pip/check.rs | 10 +++-- crates/uv/src/commands/pip/freeze.rs | 10 +++-- crates/uv/src/commands/pip/install.rs | 6 ++- crates/uv/src/commands/pip/list.rs | 10 +++-- crates/uv/src/commands/pip/show.rs | 10 +++-- crates/uv/src/commands/pip/sync.rs | 6 ++- crates/uv/src/commands/pip/uninstall.rs | 3 +- crates/uv/src/commands/project/run.rs | 4 +- crates/uv/src/commands/tool/run.rs | 4 +- crates/uv/src/commands/venv.rs | 4 +- crates/uv/tests/common/mod.rs | 4 +- 13 files changed, 78 insertions(+), 56 deletions(-) diff --git a/crates/uv-toolchain/src/lib.rs b/crates/uv-toolchain/src/lib.rs index 0e2e592fe..8c9eb6613 100644 --- a/crates/uv-toolchain/src/lib.rs +++ b/crates/uv-toolchain/src/lib.rs @@ -1205,7 +1205,7 @@ mod tests { let toolchain = context.run_with_vars(&[("VIRTUAL_ENV", Some(venv.as_os_str()))], || { Toolchain::find( - Some("3.12"), + Some(ToolchainRequest::parse("3.12")), SystemPython::Required, PreviewMode::Disabled, &context.cache, @@ -1220,7 +1220,7 @@ mod tests { // With a patch version that cannot be toolchain let result = context.run_with_vars(&[("VIRTUAL_ENV", Some(venv.as_os_str()))], || { Toolchain::find( - Some("3.12.3"), + Some(ToolchainRequest::parse("3.12.3")), SystemPython::Required, PreviewMode::Disabled, &context.cache, @@ -1263,7 +1263,7 @@ mod tests { &[("VIRTUAL_ENV", Some(context.tempdir.as_os_str()))], || { Toolchain::find( - Some("3.12.3"), + Some(ToolchainRequest::parse("3.12.3")), SystemPython::Required, PreviewMode::Disabled, &context.cache, @@ -1290,7 +1290,7 @@ mod tests { let toolchain = context.run(|| { Toolchain::find( - Some("foobar"), + Some(ToolchainRequest::parse("foobar")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1317,7 +1317,7 @@ mod tests { let result = context.run(|| { Toolchain::find( - Some("3.10.0"), + Some(ToolchainRequest::parse("3.10.0")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1344,7 +1344,7 @@ mod tests { let toolchain = context.run(|| { Toolchain::find( - Some("./foo/bar"), + Some(ToolchainRequest::parse("./foo/bar")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1359,7 +1359,7 @@ mod tests { context.add_python_versions(&["3.11.1"])?; let toolchain = context.run(|| { Toolchain::find( - Some("./foo/bar"), + Some(ToolchainRequest::parse("./foo/bar")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1387,7 +1387,7 @@ mod tests { let toolchain = context.run(|| { Toolchain::find( - Some(python.to_str().unwrap()), + Some(ToolchainRequest::parse(python.to_str().unwrap())), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1402,7 +1402,7 @@ mod tests { // With `SystemPython::Explicit let toolchain = context.run(|| { Toolchain::find( - Some(python.to_str().unwrap()), + Some(ToolchainRequest::parse(python.to_str().unwrap())), SystemPython::Explicit, PreviewMode::Disabled, &context.cache, @@ -1416,7 +1416,7 @@ mod tests { let result = context.run(|| { Toolchain::find( - Some(python.to_str().unwrap()), + Some(ToolchainRequest::parse(python.to_str().unwrap())), SystemPython::Disallowed, PreviewMode::Disabled, &context.cache, @@ -1436,7 +1436,7 @@ mod tests { context.add_python_versions(&["3.11.1"])?; let toolchain = context.run(|| { Toolchain::find( - Some(python.to_str().unwrap()), + Some(ToolchainRequest::parse(python.to_str().unwrap())), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1459,7 +1459,7 @@ mod tests { TestContext::mock_venv(&venv, "3.10.0")?; let toolchain = context.run(|| { Toolchain::find( - Some("../foo/.venv"), + Some(ToolchainRequest::parse("../foo/.venv")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1473,7 +1473,7 @@ mod tests { let toolchain = context.run(|| { Toolchain::find( - Some(venv.to_str().unwrap()), + Some(ToolchainRequest::parse(venv.to_str().unwrap())), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1495,7 +1495,9 @@ mod tests { )?; let toolchain = context.run(|| { Toolchain::find( - Some(context.tempdir.child("bar").to_str().unwrap()), + Some(ToolchainRequest::parse( + context.tempdir.child("bar").to_str().unwrap(), + )), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1513,7 +1515,7 @@ mod tests { let toolchain = context.run_with_vars(&[("VIRTUAL_ENV", Some(other_venv.as_os_str()))], || { Toolchain::find( - Some(venv.to_str().unwrap()), + Some(ToolchainRequest::parse(venv.to_str().unwrap())), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1535,7 +1537,7 @@ mod tests { let result = context.run(|| { Toolchain::find( - Some("./foo/bar"), + Some(ToolchainRequest::parse("./foo/bar")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1567,7 +1569,7 @@ mod tests { let toolchain = context .run(|| { Toolchain::find( - Some("bar"), + Some(ToolchainRequest::parse("bar")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1623,7 +1625,7 @@ mod tests { let toolchain = context .run(|| { Toolchain::find( - Some("pypy"), + Some(ToolchainRequest::parse("pypy")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1650,7 +1652,7 @@ mod tests { let toolchain = context .run(|| { Toolchain::find( - Some("pypy"), + Some(ToolchainRequest::parse("pypy")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1692,7 +1694,7 @@ mod tests { let toolchain = context.run(|| { Toolchain::find( - Some("pypy3.10"), + Some(ToolchainRequest::parse("pypy3.10")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1718,7 +1720,7 @@ mod tests { let toolchain = context.run(|| { Toolchain::find( - Some("pypy@3.10"), + Some(ToolchainRequest::parse("pypy@3.10")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1755,7 +1757,7 @@ mod tests { let toolchain = context.run(|| { Toolchain::find( - Some("pypy@3.10"), + Some(ToolchainRequest::parse("pypy@3.10")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1774,7 +1776,7 @@ mod tests { ])?; let toolchain = context.run(|| { Toolchain::find( - Some("pypy@3.10"), + Some(ToolchainRequest::parse("pypy@3.10")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1807,7 +1809,7 @@ mod tests { let toolchain = context.run(|| { Toolchain::find( - Some("pypy@3.10"), + Some(ToolchainRequest::parse("pypy@3.10")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, @@ -1836,7 +1838,7 @@ mod tests { let toolchain = context.run(|| { Toolchain::find( - Some("pypy@3.10"), + Some(ToolchainRequest::parse("pypy@3.10")), SystemPython::Allowed, PreviewMode::Disabled, &context.cache, diff --git a/crates/uv-toolchain/src/toolchain.rs b/crates/uv-toolchain/src/toolchain.rs index 2655836e2..128292951 100644 --- a/crates/uv-toolchain/src/toolchain.rs +++ b/crates/uv-toolchain/src/toolchain.rs @@ -39,13 +39,12 @@ impl Toolchain { /// /// See [`uv-toolchain::discovery`] for implementation details. pub fn find( - python: Option<&str>, + python: Option, system: SystemPython, preview: PreviewMode, cache: &Cache, ) -> Result { - if let Some(python) = python { - let request = ToolchainRequest::parse(python); + if let Some(request) = python { Self::find_requested(&request, system, preview, cache) } else if system.is_preferred() { Self::find_default(preview, cache) @@ -134,19 +133,19 @@ impl Toolchain { /// /// Unlike [`Toolchain::find`], if the toolchain is not installed it will be installed automatically. pub async fn find_or_fetch<'a>( - python: Option<&str>, + python: Option, system: SystemPython, preview: PreviewMode, client_builder: BaseClientBuilder<'a>, cache: &Cache, ) -> Result { // Perform a find first - match Self::find(python, system, preview, cache) { + match Self::find(python.clone(), system, preview, cache) { Ok(venv) => Ok(venv), Err(Error::NotFound(_)) if system.is_allowed() && preview.is_enabled() => { debug!("Requested Python not found, checking for available download..."); let request = if let Some(request) = python { - ToolchainRequest::parse(request) + request } else { ToolchainRequest::default() }; diff --git a/crates/uv/src/commands/pip/check.rs b/crates/uv/src/commands/pip/check.rs index 80e533935..dea822afa 100644 --- a/crates/uv/src/commands/pip/check.rs +++ b/crates/uv/src/commands/pip/check.rs @@ -10,7 +10,7 @@ use uv_cache::Cache; use uv_configuration::PreviewMode; use uv_fs::Simplified; use uv_installer::{SitePackages, SitePackagesDiagnostic}; -use uv_toolchain::{PythonEnvironment, SystemPython, Toolchain}; +use uv_toolchain::{PythonEnvironment, SystemPython, Toolchain, ToolchainRequest}; use crate::commands::{elapsed, ExitStatus}; use crate::printer::Printer; @@ -31,8 +31,12 @@ pub(crate) fn pip_check( } else { SystemPython::Allowed }; - let environment = - PythonEnvironment::from_toolchain(Toolchain::find(python, system, preview, cache)?); + let environment = PythonEnvironment::from_toolchain(Toolchain::find( + python.map(ToolchainRequest::parse), + system, + preview, + cache, + )?); debug!( "Using Python {} environment at {}", diff --git a/crates/uv/src/commands/pip/freeze.rs b/crates/uv/src/commands/pip/freeze.rs index 3b7f78b30..1e73e82b6 100644 --- a/crates/uv/src/commands/pip/freeze.rs +++ b/crates/uv/src/commands/pip/freeze.rs @@ -10,7 +10,7 @@ use uv_cache::Cache; use uv_configuration::PreviewMode; use uv_fs::Simplified; use uv_installer::SitePackages; -use uv_toolchain::{PythonEnvironment, SystemPython, Toolchain}; +use uv_toolchain::{PythonEnvironment, SystemPython, Toolchain, ToolchainRequest}; use crate::commands::ExitStatus; use crate::printer::Printer; @@ -31,8 +31,12 @@ pub(crate) fn pip_freeze( } else { SystemPython::Allowed }; - let environment = - PythonEnvironment::from_toolchain(Toolchain::find(python, system, preview, cache)?); + let environment = PythonEnvironment::from_toolchain(Toolchain::find( + python.map(ToolchainRequest::parse), + system, + preview, + cache, + )?); debug!( "Using Python {} environment at {}", diff --git a/crates/uv/src/commands/pip/install.rs b/crates/uv/src/commands/pip/install.rs index 768b4a114..c7eab36e7 100644 --- a/crates/uv/src/commands/pip/install.rs +++ b/crates/uv/src/commands/pip/install.rs @@ -25,7 +25,9 @@ use uv_resolver::{ DependencyMode, ExcludeNewer, FlatIndex, InMemoryIndex, OptionsBuilder, PreReleaseMode, ResolutionMode, }; -use uv_toolchain::{Prefix, PythonEnvironment, PythonVersion, SystemPython, Target, Toolchain}; +use uv_toolchain::{ + Prefix, PythonEnvironment, PythonVersion, SystemPython, Target, Toolchain, ToolchainRequest, +}; use uv_types::{BuildIsolation, HashStrategy, InFlight}; use crate::commands::pip::operations::Modifications; @@ -120,7 +122,7 @@ pub(crate) async fn pip_install( SystemPython::Explicit }; let environment = PythonEnvironment::from_toolchain(Toolchain::find( - python.as_deref(), + python.as_deref().map(ToolchainRequest::parse), system, preview, &cache, diff --git a/crates/uv/src/commands/pip/list.rs b/crates/uv/src/commands/pip/list.rs index 063ff4984..6100f066b 100644 --- a/crates/uv/src/commands/pip/list.rs +++ b/crates/uv/src/commands/pip/list.rs @@ -14,8 +14,8 @@ use uv_configuration::PreviewMode; use uv_fs::Simplified; use uv_installer::SitePackages; use uv_normalize::PackageName; -use uv_toolchain::Toolchain; use uv_toolchain::{PythonEnvironment, SystemPython}; +use uv_toolchain::{Toolchain, ToolchainRequest}; use crate::commands::ExitStatus; use crate::commands::ListFormat; @@ -41,8 +41,12 @@ pub(crate) fn pip_list( } else { SystemPython::Allowed }; - let environment = - PythonEnvironment::from_toolchain(Toolchain::find(python, system, preview, cache)?); + let environment = PythonEnvironment::from_toolchain(Toolchain::find( + python.map(ToolchainRequest::parse), + system, + preview, + cache, + )?); debug!( "Using Python {} environment at {}", diff --git a/crates/uv/src/commands/pip/show.rs b/crates/uv/src/commands/pip/show.rs index 00ac58650..f2241f6d5 100644 --- a/crates/uv/src/commands/pip/show.rs +++ b/crates/uv/src/commands/pip/show.rs @@ -12,7 +12,7 @@ use uv_configuration::PreviewMode; use uv_fs::Simplified; use uv_installer::SitePackages; use uv_normalize::PackageName; -use uv_toolchain::{PythonEnvironment, SystemPython, Toolchain}; +use uv_toolchain::{PythonEnvironment, SystemPython, Toolchain, ToolchainRequest}; use crate::commands::ExitStatus; use crate::printer::Printer; @@ -46,8 +46,12 @@ pub(crate) fn pip_show( } else { SystemPython::Allowed }; - let environment = - PythonEnvironment::from_toolchain(Toolchain::find(python, system, preview, cache)?); + let environment = PythonEnvironment::from_toolchain(Toolchain::find( + python.map(ToolchainRequest::parse), + system, + preview, + cache, + )?); debug!( "Using Python {} environment at {}", diff --git a/crates/uv/src/commands/pip/sync.rs b/crates/uv/src/commands/pip/sync.rs index 9968e87a8..db53eec0c 100644 --- a/crates/uv/src/commands/pip/sync.rs +++ b/crates/uv/src/commands/pip/sync.rs @@ -24,7 +24,9 @@ use uv_resolver::{ DependencyMode, ExcludeNewer, FlatIndex, InMemoryIndex, OptionsBuilder, PreReleaseMode, ResolutionMode, }; -use uv_toolchain::{Prefix, PythonEnvironment, PythonVersion, SystemPython, Target, Toolchain}; +use uv_toolchain::{ + Prefix, PythonEnvironment, PythonVersion, SystemPython, Target, Toolchain, ToolchainRequest, +}; use uv_types::{BuildIsolation, HashStrategy, InFlight}; use crate::commands::pip::operations::Modifications; @@ -115,7 +117,7 @@ pub(crate) async fn pip_sync( SystemPython::Explicit }; let environment = PythonEnvironment::from_toolchain(Toolchain::find( - python.as_deref(), + python.as_deref().map(ToolchainRequest::parse), system, preview, &cache, diff --git a/crates/uv/src/commands/pip/uninstall.rs b/crates/uv/src/commands/pip/uninstall.rs index a0dffdc05..21b177a81 100644 --- a/crates/uv/src/commands/pip/uninstall.rs +++ b/crates/uv/src/commands/pip/uninstall.rs @@ -15,6 +15,7 @@ use uv_configuration::{KeyringProviderType, PreviewMode}; use uv_fs::Simplified; use uv_requirements::{RequirementsSource, RequirementsSpecification}; use uv_toolchain::Toolchain; +use uv_toolchain::ToolchainRequest; use uv_toolchain::{Prefix, PythonEnvironment, SystemPython, Target}; use crate::commands::{elapsed, ExitStatus}; @@ -52,7 +53,7 @@ pub(crate) async fn pip_uninstall( SystemPython::Explicit }; let environment = PythonEnvironment::from_toolchain(Toolchain::find( - python.as_deref(), + python.as_deref().map(ToolchainRequest::parse), system, preview, &cache, diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index 744727636..57ad72c98 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -12,7 +12,7 @@ use uv_configuration::{Concurrency, ExtrasSpecification, PreviewMode}; use uv_distribution::{ProjectWorkspace, Workspace}; use uv_normalize::PackageName; use uv_requirements::RequirementsSource; -use uv_toolchain::{PythonEnvironment, SystemPython, Toolchain}; +use uv_toolchain::{PythonEnvironment, SystemPython, Toolchain, ToolchainRequest}; use uv_warnings::warn_user; use crate::commands::{project, ExitStatus}; @@ -128,7 +128,7 @@ pub(crate) async fn run( } else { // Note we force preview on during `uv run` for now since the entire interface is in preview Toolchain::find_or_fetch( - python.as_deref(), + python.as_deref().map(ToolchainRequest::parse), SystemPython::Allowed, PreviewMode::Enabled, client_builder, diff --git a/crates/uv/src/commands/tool/run.rs b/crates/uv/src/commands/tool/run.rs index 8a858d3d6..af48d8aa6 100644 --- a/crates/uv/src/commands/tool/run.rs +++ b/crates/uv/src/commands/tool/run.rs @@ -10,7 +10,7 @@ use uv_cache::Cache; use uv_client::Connectivity; use uv_configuration::{Concurrency, PreviewMode}; use uv_requirements::RequirementsSource; -use uv_toolchain::{PythonEnvironment, SystemPython, Toolchain}; +use uv_toolchain::{PythonEnvironment, SystemPython, Toolchain, ToolchainRequest}; use uv_warnings::warn_user; use crate::commands::project::update_environment; @@ -55,7 +55,7 @@ pub(crate) async fn run( // Discover an interpreter. // Note we force preview on during `uv tool run` for now since the entire interface is in preview let interpreter = Toolchain::find( - python.as_deref(), + python.as_deref().map(ToolchainRequest::parse), SystemPython::Allowed, PreviewMode::Enabled, cache, diff --git a/crates/uv/src/commands/venv.rs b/crates/uv/src/commands/venv.rs index 42305b49b..118bcff22 100644 --- a/crates/uv/src/commands/venv.rs +++ b/crates/uv/src/commands/venv.rs @@ -23,7 +23,7 @@ use uv_dispatch::BuildDispatch; use uv_fs::Simplified; use uv_git::GitResolver; use uv_resolver::{ExcludeNewer, FlatIndex, InMemoryIndex, OptionsBuilder}; -use uv_toolchain::{SystemPython, Toolchain}; +use uv_toolchain::{SystemPython, Toolchain, ToolchainRequest}; use uv_types::{BuildContext, BuildIsolation, HashStrategy, InFlight}; use crate::commands::{pip, ExitStatus}; @@ -127,7 +127,7 @@ async fn venv_impl( // Locate the Python interpreter to use in the environment let interpreter = Toolchain::find_or_fetch( - python_request, + python_request.map(ToolchainRequest::parse), SystemPython::Required, preview, client_builder, diff --git a/crates/uv/tests/common/mod.rs b/crates/uv/tests/common/mod.rs index 5c341c790..54b444633 100644 --- a/crates/uv/tests/common/mod.rs +++ b/crates/uv/tests/common/mod.rs @@ -18,7 +18,7 @@ use uv_configuration::PreviewMode; use uv_cache::Cache; use uv_fs::Simplified; use uv_toolchain::managed::InstalledToolchains; -use uv_toolchain::{PythonVersion, Toolchain}; +use uv_toolchain::{PythonVersion, Toolchain, ToolchainRequest}; // Exclude any packages uploaded after this date. pub static EXCLUDE_NEWER: &str = "2024-03-25T00:00:00Z"; @@ -596,7 +596,7 @@ pub fn python_path_with_versions( if inner.is_empty() { // Fallback to a system lookup if we failed to find one in the toolchain directory if let Ok(toolchain) = Toolchain::find( - Some(python_version), + Some(ToolchainRequest::parse(python_version)), // Without required, we could pick the current venv here and the test fails // because the venv subcommand requires a system interpreter. uv_toolchain::SystemPython::Required,