From 0c5ae1f25cb71db9cc1af94315eeab6b4accab9e Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Thu, 29 May 2025 17:04:57 -0500 Subject: [PATCH] Differentiate between implicit vs explicit architecture requests (#13723) In https://github.com/astral-sh/uv/pull/13721#issuecomment-2920530601 I presumed that all the installation problems in https://github.com/astral-sh/uv/pull/13722 were solved by https://github.com/astral-sh/uv/pull/13709 but they were not because we don't differentiate between implicit and explicit architecture requests so a request for `aarch64` is considered satisfied by an existing `x86-64` installation even if the user explicitly requested that architecture. Now, we track if it was explicit or implicit, requiring an exact match in the former case, and a `supports` in the latter. We considered doing this for other items in the request, like the operating system but we don't have a `supports()` concept there. It might make sense for libc in the future. --- crates/uv-python/src/discovery.rs | 10 ++-- crates/uv-python/src/downloads.rs | 59 +++++++++++++++--------- crates/uv/src/commands/python/install.rs | 4 +- crates/uv/tests/it/python_find.rs | 5 +- 4 files changed, 48 insertions(+), 30 deletions(-) diff --git a/crates/uv-python/src/discovery.rs b/crates/uv-python/src/discovery.rs index 155455ff9..09022f03b 100644 --- a/crates/uv-python/src/discovery.rs +++ b/crates/uv-python/src/discovery.rs @@ -2723,7 +2723,7 @@ mod tests { use crate::{ discovery::{PythonRequest, VersionRequest}, - downloads::PythonDownloadRequest, + downloads::{ArchRequest, PythonDownloadRequest}, implementation::ImplementationName, platform::{Arch, Libc, Os}, }; @@ -2813,10 +2813,10 @@ mod tests { PythonVariant::Default )), implementation: Some(ImplementationName::CPython), - arch: Some(Arch { + arch: Some(ArchRequest::Explicit(Arch { family: Architecture::Aarch64(Aarch64Architecture::Aarch64), variant: None - }), + })), os: Some(Os(target_lexicon::OperatingSystem::Darwin(None))), libc: Some(Libc::None), prereleases: None @@ -2848,10 +2848,10 @@ mod tests { PythonVariant::Default )), implementation: None, - arch: Some(Arch { + arch: Some(ArchRequest::Explicit(Arch { family: Architecture::Aarch64(Aarch64Architecture::Aarch64), variant: None - }), + })), os: None, libc: None, prereleases: None diff --git a/crates/uv-python/src/downloads.rs b/crates/uv-python/src/downloads.rs index 0659892d1..fcc39691a 100644 --- a/crates/uv-python/src/downloads.rs +++ b/crates/uv-python/src/downloads.rs @@ -116,7 +116,7 @@ pub struct ManagedPythonDownload { pub struct PythonDownloadRequest { pub(crate) version: Option, pub(crate) implementation: Option, - pub(crate) arch: Option, + pub(crate) arch: Option, pub(crate) os: Option, pub(crate) libc: Option, @@ -125,11 +125,40 @@ pub struct PythonDownloadRequest { pub(crate) prereleases: Option, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ArchRequest { + Explicit(Arch), + Environment(Arch), +} + +impl Display for ArchRequest { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Explicit(arch) | Self::Environment(arch) => write!(f, "{arch}"), + } + } +} + +impl ArchRequest { + pub(crate) fn satisfied_by(self, arch: Arch) -> bool { + match self { + Self::Explicit(request) => request == arch, + Self::Environment(env) => env.supports(arch), + } + } + + pub fn inner(&self) -> Arch { + match self { + Self::Explicit(arch) | Self::Environment(arch) => *arch, + } + } +} + impl PythonDownloadRequest { pub fn new( version: Option, implementation: Option, - arch: Option, + arch: Option, os: Option, libc: Option, prereleases: Option, @@ -158,7 +187,7 @@ impl PythonDownloadRequest { #[must_use] pub fn with_arch(mut self, arch: Arch) -> Self { - self.arch = Some(arch); + self.arch = Some(ArchRequest::Explicit(arch)); self } @@ -219,7 +248,7 @@ impl PythonDownloadRequest { /// Platform information is pulled from the environment. pub fn fill_platform(mut self) -> Result { if self.arch.is_none() { - self.arch = Some(Arch::from_env()); + self.arch = Some(ArchRequest::Environment(Arch::from_env())); } if self.os.is_none() { self.os = Some(Os::from_env()); @@ -238,18 +267,6 @@ impl PythonDownloadRequest { Ok(self) } - /// Construct a new [`PythonDownloadRequest`] with platform information from the environment. - pub fn from_env() -> Result { - Ok(Self::new( - None, - None, - Some(Arch::from_env()), - Some(Os::from_env()), - Some(Libc::from_env()?), - None, - )) - } - pub fn implementation(&self) -> Option<&ImplementationName> { self.implementation.as_ref() } @@ -258,7 +275,7 @@ impl PythonDownloadRequest { self.version.as_ref() } - pub fn arch(&self) -> Option<&Arch> { + pub fn arch(&self) -> Option<&ArchRequest> { self.arch.as_ref() } @@ -288,7 +305,7 @@ impl PythonDownloadRequest { } if let Some(arch) = &self.arch { - if !arch.supports(key.arch) { + if !arch.satisfied_by(key.arch) { return false; } } @@ -366,7 +383,7 @@ impl PythonDownloadRequest { } if let Some(arch) = self.arch() { let interpreter_arch = Arch::from(&interpreter.platform().arch()); - if &interpreter_arch != arch { + if !arch.satisfied_by(interpreter_arch) { debug!( "Skipping interpreter at `{executable}`: architecture `{interpreter_arch}` does not match request `{arch}`" ); @@ -408,7 +425,7 @@ impl From<&ManagedPythonInstallation> for PythonDownloadRequest { "Managed Python installations are expected to always have known implementation names, found {name}" ), }, - Some(key.arch), + Some(ArchRequest::Explicit(key.arch)), Some(key.os), Some(key.libc), Some(key.prerelease.is_some()), @@ -478,7 +495,7 @@ impl FromStr for PythonDownloadRequest { ); } 3 => os = Some(Os::from_str(part)?), - 4 => arch = Some(Arch::from_str(part)?), + 4 => arch = Some(ArchRequest::Explicit(Arch::from_str(part)?)), 5 => libc = Some(Libc::from_str(part)?), _ => return Err(Error::TooManyParts(s.to_string())), } diff --git a/crates/uv/src/commands/python/install.rs b/crates/uv/src/commands/python/install.rs index 7e190d31d..8f5beedc9 100644 --- a/crates/uv/src/commands/python/install.rs +++ b/crates/uv/src/commands/python/install.rs @@ -61,7 +61,9 @@ impl InstallRequest { Ok(download) => download, Err(downloads::Error::NoDownloadFound(request)) if request.libc().is_some_and(Libc::is_musl) - && request.arch().is_some_and(Arch::is_arm) => + && request + .arch() + .is_some_and(|arch| Arch::is_arm(&arch.inner())) => { return Err(anyhow::anyhow!( "uv does not yet provide musl Python distributions on aarch64." diff --git a/crates/uv/tests/it/python_find.rs b/crates/uv/tests/it/python_find.rs index 82ed37fcf..11bf421a3 100644 --- a/crates/uv/tests/it/python_find.rs +++ b/crates/uv/tests/it/python_find.rs @@ -97,15 +97,14 @@ fn python_find() { let arch = Arch::from_env(); uv_snapshot!(context.filters(), context.python_find() - .arg(format!("cpython-3.12-{os}-{arch}")) - , @r###" + .arg(format!("cpython-3.12-{os}-{arch}")), @r" success: true exit_code: 0 ----- stdout ----- [PYTHON-3.12] ----- stderr ----- - "###); + "); // Request PyPy (which should be missing) uv_snapshot!(context.filters(), context.python_find().arg("pypy"), @r"