From 77e93157fb829dee83ff29582e4852a3731477b9 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 3 Jun 2024 18:37:15 -0400 Subject: [PATCH] Make target Python version an optional field (#4000) ## Summary Instead of checking if the target and installed version are the same, we model the data such that the target version is only present if it was specified by the user. This also means that we correctly say "requested version" even if the two happen to be the same. --- crates/bench/benches/uv.rs | 2 +- crates/uv-dispatch/src/lib.rs | 3 +-- crates/uv-resolver/src/pubgrub/report.rs | 16 +++++++++---- crates/uv-resolver/src/python_requirement.rs | 24 +++++++++++++------- crates/uv-resolver/src/resolver/mod.rs | 10 ++++---- crates/uv-resolver/src/version_map.rs | 23 +++++++++++++------ crates/uv-resolver/tests/resolver.rs | 2 +- crates/uv/src/commands/pip/compile.rs | 9 ++++++-- crates/uv/src/commands/pip/operations.rs | 7 ++---- 9 files changed, 61 insertions(+), 35 deletions(-) diff --git a/crates/bench/benches/uv.rs b/crates/bench/benches/uv.rs index a6f554442f6..e53d8a8e103 100644 --- a/crates/bench/benches/uv.rs +++ b/crates/bench/benches/uv.rs @@ -136,7 +136,7 @@ mod resolver { let index_locations = IndexLocations::default(); let installed_packages = EmptyInstalledPackages; let interpreter = venv.interpreter(); - let python_requirement = PythonRequirement::from_marker_environment(interpreter, &MARKERS); + let python_requirement = PythonRequirement::from_interpreter(interpreter); let build_context = BuildDispatch::new( client, diff --git a/crates/uv-dispatch/src/lib.rs b/crates/uv-dispatch/src/lib.rs index 456c5e44a3b..62ef5368037 100644 --- a/crates/uv-dispatch/src/lib.rs +++ b/crates/uv-dispatch/src/lib.rs @@ -150,9 +150,8 @@ impl<'a> BuildContext for BuildDispatch<'a> { } async fn resolve<'data>(&'data self, requirements: &'data [Requirement]) -> Result { + let python_requirement = PythonRequirement::from_interpreter(self.interpreter); let markers = self.interpreter.markers(); - let python_requirement = - PythonRequirement::from_marker_environment(self.interpreter, markers); let tags = self.interpreter.tags()?; let resolver = Resolver::new( Manifest::simple(requirements.to_vec()), diff --git a/crates/uv-resolver/src/pubgrub/report.rs b/crates/uv-resolver/src/pubgrub/report.rs index e3897e11718..aaa742158e2 100644 --- a/crates/uv-resolver/src/pubgrub/report.rs +++ b/crates/uv-resolver/src/pubgrub/report.rs @@ -49,11 +49,17 @@ impl ReportFormatter, UnavailableReason> &**package, PubGrubPackageInner::Python(PubGrubPython::Target) ) { - return format!( - "the requested {package} version ({}) does not satisfy {}", - python.target(), - PackageRange::compatibility(package, set) - ); + return if let Some(target) = python.target() { + format!( + "the requested {package} version ({target}) does not satisfy {}", + PackageRange::compatibility(package, set) + ) + } else { + format!( + "the requested {package} version does not satisfy {}", + PackageRange::compatibility(package, set) + ) + }; } if matches!( &**package, diff --git a/crates/uv-resolver/src/python_requirement.rs b/crates/uv-resolver/src/python_requirement.rs index 84a6dd5568d..55f43ebb9fa 100644 --- a/crates/uv-resolver/src/python_requirement.rs +++ b/crates/uv-resolver/src/python_requirement.rs @@ -1,5 +1,5 @@ -use pep508_rs::{MarkerEnvironment, StringVersion}; -use uv_interpreter::Interpreter; +use pep508_rs::StringVersion; +use uv_interpreter::{Interpreter, PythonVersion}; #[derive(Debug, Clone, Eq, PartialEq)] pub struct PythonRequirement { @@ -8,21 +8,29 @@ pub struct PythonRequirement { /// The target version of Python; that is, the version of Python for which we are resolving /// dependencies. This is typically the same as the installed version, but may be different /// when specifying an alternate Python version for the resolution. - target: StringVersion, + /// + /// If `None`, the target version is the same as the installed version. + target: Option, } impl PythonRequirement { - pub fn from_marker_environment(interpreter: &Interpreter, env: &MarkerEnvironment) -> Self { + /// Create a [`PythonRequirement`] to resolve against both an [`Interpreter`] and a + /// [`PythonVersion`]. + pub fn from_python_version(interpreter: &Interpreter, python_version: &PythonVersion) -> Self { Self { installed: interpreter.python_full_version().clone(), - target: env.python_full_version().clone(), + target: Some(StringVersion { + string: python_version.to_string(), + version: python_version.python_full_version(), + }), } } + /// Create a [`PythonRequirement`] to resolve against an [`Interpreter`]. pub fn from_interpreter(interpreter: &Interpreter) -> Self { Self { installed: interpreter.python_full_version().clone(), - target: interpreter.python_full_version().clone(), + target: None, } } @@ -32,7 +40,7 @@ impl PythonRequirement { } /// Return the target version of Python. - pub fn target(&self) -> &StringVersion { - &self.target + pub fn target(&self) -> Option<&StringVersion> { + self.target.as_ref() } } diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index ed972d8abab..de01e8f2010 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -308,7 +308,9 @@ impl ResolverState ResolverState ResolverState Cow::Owned(Tags::from_env( @@ -251,8 +258,6 @@ pub(crate) async fn pip_compile( (None, None) => Cow::Borrowed(interpreter.markers()), }; - let python_requirement = PythonRequirement::from_marker_environment(&interpreter, &markers); - // Generate, but don't enforce hashes for the requirements. let hasher = if generate_hashes { HashStrategy::Generate diff --git a/crates/uv/src/commands/pip/operations.rs b/crates/uv/src/commands/pip/operations.rs index cbab850c765..7c805c5982d 100644 --- a/crates/uv/src/commands/pip/operations.rs +++ b/crates/uv/src/commands/pip/operations.rs @@ -182,11 +182,6 @@ pub(crate) async fn resolve( // Collect constraints and overrides. let constraints = Constraints::from_requirements(constraints); let overrides = Overrides::from_requirements(overrides); - let python_requirement = if let Some(markers) = markers { - PythonRequirement::from_marker_environment(interpreter, markers) - } else { - PythonRequirement::from_interpreter(interpreter) - }; // Determine any lookahead requirements. let lookaheads = match options.dependency_mode { @@ -206,6 +201,8 @@ pub(crate) async fn resolve( DependencyMode::Direct => Vec::new(), }; + let python_requirement = PythonRequirement::from_interpreter(interpreter); + // TODO(zanieb): Consider consuming these instead of cloning let exclusions = Exclusions::new(reinstall.clone(), upgrade.clone());