From 6e18e5678926122b85df4fb92b7620fff4982e75 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 14 Jan 2024 10:39:15 -0500 Subject: [PATCH] Adjust markers to match target Python version (#909) ## Summary This PR ensures that when the user passes in `--python-version`, we adjust the _markers_ to match the target version, thus forcing us to select compatible wheels for the `--python-version`, rather than the installed version. ## Context Let's call Python 3.10 the "installed" environment and Python 3.12 the "target" environment. For each version, we have _both_ a Python version (to match against `Requires-Python`) and a set of tags (to match against wheels). The rules for resolution are as follows... - For each package, for each version, we try to find the "best candidate" for resolution and installation. - We first look for a wheel that's compatible with the _target_ environment. This requires testing against both the `Requires-Python` and the markers. (We won't have to build or run this code, so the _installed_ version is irrelevant.) **(This PR corrects _this_ bullet -- previously, we validated against the _installed_ markers, rather than the target markers.)** - If we can't find a compatible wheel, we accept any _incompatible_ wheel as long as there's a source distribution. The source distribution _must_ be compatible with the target environment. (We won't have to build or run this code, so the _installed_ version is irrelevant.) - If there are no wheels, then the source distribution must be compatible with _both_ the installed and target environments, since we need to build it. This is all true for the top-level resolution. When we perform a sub-resolution (when resolving the build dependencies of a source distribution), we should _only_ use the installed environment, and ignore the target environment, since we assume that the dependencies will be the same in both environments once built -- so our goal is "just" to build the distribution, without concern for which build dependencies it uses. Closes https://github.com/astral-sh/puffin/issues/883. --- crates/puffin-cli/src/commands/pip_compile.rs | 14 ++++++++--- crates/puffin-interpreter/src/interpreter.rs | 24 ++++++++++++++----- .../puffin-interpreter/src/python_version.rs | 13 ++++++---- .../puffin-resolver/src/candidate_selector.rs | 17 ++++++++++--- 4 files changed, 52 insertions(+), 16 deletions(-) diff --git a/crates/puffin-cli/src/commands/pip_compile.rs b/crates/puffin-cli/src/commands/pip_compile.rs index cee3b08c3..a5bbf0986 100644 --- a/crates/puffin-cli/src/commands/pip_compile.rs +++ b/crates/puffin-cli/src/commands/pip_compile.rs @@ -16,6 +16,7 @@ use tracing::debug; use distribution_types::{IndexUrls, LocalEditable}; use pep508_rs::Requirement; use platform_host::Platform; +use platform_tags::Tags; use puffin_cache::Cache; use puffin_client::RegistryClientBuilder; use puffin_dispatch::BuildDispatch; @@ -128,7 +129,14 @@ pub(crate) async fn pip_compile( ); // Determine the tags, markers, and interpreter to use for resolution. - let tags = interpreter.tags()?; + let tags = if let Some(python_version) = python_version.as_ref() { + Cow::Owned(Tags::from_env( + interpreter.platform(), + python_version.simple_version(), + )?) + } else { + Cow::Borrowed(interpreter.tags()?) + }; let markers = python_version.map_or_else( || Cow::Borrowed(interpreter.markers()), |python_version| Cow::Owned(python_version.markers(interpreter.markers())), @@ -171,7 +179,7 @@ pub(crate) async fn pip_compile( }) .collect::>()?; - let downloader = Downloader::new(&cache, tags, &client, &build_dispatch) + let downloader = Downloader::new(&cache, &tags, &client, &build_dispatch) .with_reporter(DownloadReporter::from(printer).with_length(editables.len() as u64)); let editable_wheel_dir = tempdir_in(cache.root())?; @@ -217,7 +225,7 @@ pub(crate) async fn pip_compile( options, &markers, &interpreter, - tags, + &tags, &client, &build_dispatch, ) diff --git a/crates/puffin-interpreter/src/interpreter.rs b/crates/puffin-interpreter/src/interpreter.rs index 2fd9488dd..5aeb0aa29 100644 --- a/crates/puffin-interpreter/src/interpreter.rs +++ b/crates/puffin-interpreter/src/interpreter.rs @@ -144,12 +144,14 @@ impl Interpreter { } /// Returns the path to the Python virtual environment. + #[inline] pub fn platform(&self) -> &Platform { &self.platform } /// Returns the [`MarkerEnvironment`] for this Python executable. - pub fn markers(&self) -> &MarkerEnvironment { + #[inline] + pub const fn markers(&self) -> &MarkerEnvironment { &self.markers } @@ -160,16 +162,26 @@ impl Interpreter { } /// Returns the Python version. - pub fn version(&self) -> &Version { + #[inline] + pub const fn version(&self) -> &Version { &self.markers.python_full_version.version } + /// Return the major version of this Python version. + pub fn major(&self) -> u8 { + let major = self.version().release()[0]; + u8::try_from(major).expect("invalid major version") + } + + /// Return the minor version of this Python version. + pub fn minor(&self) -> u8 { + let minor = self.version().release()[1]; + u8::try_from(minor).expect("invalid minor version") + } + /// Returns the Python version as a simple tuple. pub fn simple_version(&self) -> (u8, u8) { - ( - u8::try_from(self.version().release()[0]).expect("invalid major version"), - u8::try_from(self.version().release()[1]).expect("invalid minor version"), - ) + (self.major(), self.minor()) } pub fn base_exec_prefix(&self) -> &Path { diff --git a/crates/puffin-interpreter/src/python_version.rs b/crates/puffin-interpreter/src/python_version.rs index 754558b38..126a27f1b 100644 --- a/crates/puffin-interpreter/src/python_version.rs +++ b/crates/puffin-interpreter/src/python_version.rs @@ -84,12 +84,17 @@ impl PythonVersion { } /// Return the major version of this Python version. - pub fn major(&self) -> u64 { - self.0.release()[0] + pub fn major(&self) -> u8 { + u8::try_from(self.0.release()[0]).expect("invalid major version") } /// Return the minor version of this Python version. - pub fn minor(&self) -> u64 { - self.0.release()[1] + pub fn minor(&self) -> u8 { + u8::try_from(self.0.release()[1]).expect("invalid minor version") + } + + /// Returns the Python version as a simple tuple. + pub fn simple_version(&self) -> (u8, u8) { + (self.major(), self.minor()) } } diff --git a/crates/puffin-resolver/src/candidate_selector.rs b/crates/puffin-resolver/src/candidate_selector.rs index f4925fadf..017d5aef3 100644 --- a/crates/puffin-resolver/src/candidate_selector.rs +++ b/crates/puffin-resolver/src/candidate_selector.rs @@ -256,9 +256,7 @@ impl<'a> Candidate<'a> { /// If the candidate doesn't match the given requirement, return the version specifiers. pub(crate) fn validate(&self, requirement: &PythonRequirement) -> Option<&VersionSpecifiers> { - // Validate against the _installed_ file. It's fine if the _resolved_ file is incompatible, - // since it could be an incompatible wheel. (If the resolved file is an incompatible source - // distribution, then the resolved and installed file will be the same anyway.) + // Validate the _installed_ file. let requires_python = self.install().requires_python.as_ref()?; // If the candidate doesn't support the target Python version, return the failing version @@ -275,6 +273,19 @@ impl<'a> Candidate<'a> { } } + // Validate the resolved file. + let requires_python = self.resolve().requires_python.as_ref()?; + + // If the candidate is a source distribution, and doesn't support the installed Python + // version, return the failing version specifiers, since we won't be able to build it. + // This isn't strictly necessary, since if `self.resolve()` is a source distribution, it + // should be the same file as `self.install()` (validated above). + if matches!(self.resolve().dist, Dist::Source(_)) { + if !requires_python.contains(requirement.installed()) { + return Some(requires_python); + } + } + None } }