From 78c8c711fa6d86b95f3e4b749da4573c01d2aae0 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Wed, 13 Aug 2025 09:02:55 -0500 Subject: [PATCH] Refactor os, arch, and libc information into a shared `Platform` type (#15027) Addresses this outstanding item from a previous review https://github.com/astral-sh/uv/pull/13724#discussion_r2114867288 I'm interested in this now for consolidating some logic in #12731 --- crates/uv-platform/src/arch.rs | 108 ++++--- crates/uv-platform/src/lib.rs | 408 +++++++++++++++++++++++++++ crates/uv-platform/src/os.rs | 4 + crates/uv-python/src/discovery.rs | 2 +- crates/uv-python/src/downloads.rs | 81 +++--- crates/uv-python/src/installation.rs | 232 ++++++++++----- crates/uv-python/src/interpreter.rs | 7 +- crates/uv-python/src/managed.rs | 28 +- 8 files changed, 698 insertions(+), 172 deletions(-) diff --git a/crates/uv-platform/src/arch.rs b/crates/uv-platform/src/arch.rs index 92c6b1e41..39ea6ca0d 100644 --- a/crates/uv-platform/src/arch.rs +++ b/crates/uv-platform/src/arch.rs @@ -1,7 +1,5 @@ use crate::Error; -use std::fmt::Display; use std::str::FromStr; -use std::{cmp, fmt}; /// Architecture variants, e.g., with support for different instruction sets #[derive(Debug, Eq, PartialEq, Clone, Copy, Hash, Ord, PartialOrd)] @@ -24,7 +22,7 @@ pub struct Arch { } impl Ord for Arch { - fn cmp(&self, other: &Self) -> cmp::Ordering { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { if self.family == other.family { return self.variant.cmp(&other.variant); } @@ -55,8 +53,8 @@ impl Ord for Arch { other.family == preferred.family, ) { (true, true) => unreachable!(), - (true, false) => cmp::Ordering::Less, - (false, true) => cmp::Ordering::Greater, + (true, false) => std::cmp::Ordering::Less, + (false, true) => std::cmp::Ordering::Greater, (false, false) => { // Both non-preferred, fallback to lexicographic order self.family.to_string().cmp(&other.family.to_string()) @@ -66,48 +64,29 @@ impl Ord for Arch { } impl PartialOrd for Arch { - fn partial_cmp(&self, other: &Self) -> Option { + fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } } - impl Arch { pub fn new(family: target_lexicon::Architecture, variant: Option) -> Self { Self { family, variant } } pub fn from_env() -> Self { + #[cfg(test)] + { + if let Some(arch) = test_support::get_mock_arch() { + return arch; + } + } + Self { family: target_lexicon::HOST.architecture, variant: None, } } - /// Does the current architecture support running the other? - /// - /// When the architecture is equal, this is always true. Otherwise, this is true if the - /// architecture is transparently emulated or is a microarchitecture with worse performance - /// characteristics. - pub fn supports(self, other: Self) -> bool { - if self == other { - return true; - } - - // TODO: Implement `variant` support checks - - // Windows ARM64 runs emulated x86_64 binaries transparently - // Similarly, macOS aarch64 runs emulated x86_64 binaries transparently if you have Rosetta - // installed. We don't try to be clever and check if that's the case here, we just assume - // that if x86_64 distributions are available, they're usable. - if (cfg!(windows) || cfg!(target_os = "macos")) - && matches!(self.family, target_lexicon::Architecture::Aarch64(_)) - { - return other.family == target_lexicon::Architecture::X86_64; - } - - false - } - pub fn family(&self) -> target_lexicon::Architecture { self.family } @@ -117,8 +96,8 @@ impl Arch { } } -impl Display for Arch { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +impl std::fmt::Display for Arch { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self.family { target_lexicon::Architecture::X86_32(target_lexicon::X86_32Architecture::I686) => { write!(f, "x86")?; @@ -192,8 +171,8 @@ impl FromStr for ArchVariant { } } -impl Display for ArchVariant { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +impl std::fmt::Display for ArchVariant { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { Self::V2 => write!(f, "v2"), Self::V3 => write!(f, "v3"), @@ -247,3 +226,60 @@ impl From<&uv_platform_tags::Arch> for Arch { } } } + +#[cfg(test)] +pub(crate) mod test_support { + use super::*; + use std::cell::RefCell; + + thread_local! { + static MOCK_ARCH: RefCell> = const { RefCell::new(None) }; + } + + pub(crate) fn get_mock_arch() -> Option { + MOCK_ARCH.with(|arch| *arch.borrow()) + } + + fn set_mock_arch(arch: Option) { + MOCK_ARCH.with(|mock| *mock.borrow_mut() = arch); + } + + pub(crate) struct MockArchGuard { + previous: Option, + } + + impl MockArchGuard { + pub(crate) fn new(arch: Arch) -> Self { + let previous = get_mock_arch(); + set_mock_arch(Some(arch)); + Self { previous } + } + } + + impl Drop for MockArchGuard { + fn drop(&mut self) { + set_mock_arch(self.previous); + } + } + + /// Run a function with a mocked architecture. + /// The mock is automatically cleaned up after the function returns. + pub(crate) fn run_with_arch(arch: Arch, f: F) -> R + where + F: FnOnce() -> R, + { + let _guard = MockArchGuard::new(arch); + f() + } + + pub(crate) fn x86_64() -> Arch { + Arch::new(target_lexicon::Architecture::X86_64, None) + } + + pub(crate) fn aarch64() -> Arch { + Arch::new( + target_lexicon::Architecture::Aarch64(target_lexicon::Aarch64Architecture::Aarch64), + None, + ) + } +} diff --git a/crates/uv-platform/src/lib.rs b/crates/uv-platform/src/lib.rs index 7eb23875a..3d1f89f81 100644 --- a/crates/uv-platform/src/lib.rs +++ b/crates/uv-platform/src/lib.rs @@ -1,5 +1,8 @@ //! Platform detection for operating system, architecture, and libc. +use std::cmp; +use std::fmt; +use std::str::FromStr; use thiserror::Error; pub use crate::arch::{Arch, ArchVariant}; @@ -23,4 +26,409 @@ pub enum Error { UnsupportedVariant(String, String), #[error(transparent)] LibcDetectionError(#[from] crate::libc::LibcDetectionError), + #[error("Invalid platform format: {0}")] + InvalidPlatformFormat(String), +} + +/// A platform identifier that combines operating system, architecture, and libc. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct Platform { + pub os: Os, + pub arch: Arch, + pub libc: Libc, +} + +impl Platform { + /// Create a new platform with the given components. + pub fn new(os: Os, arch: Arch, libc: Libc) -> Self { + Self { os, arch, libc } + } + + /// Create a platform from string parts (os, arch, libc). + pub fn from_parts(os: &str, arch: &str, libc: &str) -> Result { + Ok(Self { + os: Os::from_str(os)?, + arch: Arch::from_str(arch)?, + libc: Libc::from_str(libc)?, + }) + } + + /// Detect the platform from the current environment. + pub fn from_env() -> Result { + let os = Os::from_env(); + let arch = Arch::from_env(); + let libc = Libc::from_env()?; + Ok(Self { os, arch, libc }) + } + + /// Check if this platform supports running another platform. + pub fn supports(&self, other: &Self) -> bool { + // If platforms are exactly equal, they're compatible + if self == other { + return true; + } + + // OS must match exactly + if self.os != other.os { + return false; + } + + // Libc must match exactly + if self.libc != other.libc { + return false; + } + + // Check architecture compatibility + if self.arch == other.arch { + return true; + } + + // Windows ARM64 runs emulated x86_64 binaries transparently + // Similarly, macOS aarch64 runs emulated x86_64 binaries transparently if you have Rosetta + // installed. We don't try to be clever and check if that's the case here, we just assume + // that if x86_64 distributions are available, they're usable. + if (self.os.is_windows() || self.os.is_macos()) + && matches!(self.arch.family(), target_lexicon::Architecture::Aarch64(_)) + && matches!(other.arch.family(), target_lexicon::Architecture::X86_64) + { + return true; + } + + // TODO: Allow inequal variants, as we don't implement variant support checks yet. + // See https://github.com/astral-sh/uv/pull/9788 + // For now, allow same architecture family as a fallback + self.arch.family() == other.arch.family() + } +} + +impl fmt::Display for Platform { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}-{}-{}", self.os, self.arch, self.libc) + } +} + +impl FromStr for Platform { + type Err = Error; + + fn from_str(s: &str) -> Result { + let parts: Vec<&str> = s.split('-').collect(); + + if parts.len() != 3 { + return Err(Error::InvalidPlatformFormat(format!( + "expected exactly 3 parts separated by '-', got {}", + parts.len() + ))); + } + + Self::from_parts(parts[0], parts[1], parts[2]) + } +} + +impl Ord for Platform { + fn cmp(&self, other: &Self) -> cmp::Ordering { + self.os + .to_string() + .cmp(&other.os.to_string()) + // Then architecture + .then_with(|| { + if self.arch.family == other.arch.family { + return self.arch.variant.cmp(&other.arch.variant); + } + + // For the time being, manually make aarch64 windows disfavored on its own host + // platform, because most packages don't have wheels for aarch64 windows, making + // emulation more useful than native execution! + // + // The reason we do this in "sorting" and not "supports" is so that we don't + // *refuse* to use an aarch64 windows pythons if they happen to be installed and + // nothing else is available. + // + // Similarly if someone manually requests an aarch64 windows install, we should + // respect that request (this is the way users should "override" this behaviour). + let preferred = if self.os.is_windows() { + Arch { + family: target_lexicon::Architecture::X86_64, + variant: None, + } + } else { + // Prefer native architectures + Arch::from_env() + }; + + match ( + self.arch.family == preferred.family, + other.arch.family == preferred.family, + ) { + (true, true) => unreachable!(), + (true, false) => cmp::Ordering::Less, + (false, true) => cmp::Ordering::Greater, + (false, false) => { + // Both non-preferred, fallback to lexicographic order + self.arch + .family + .to_string() + .cmp(&other.arch.family.to_string()) + } + } + }) + // Finally compare libc + .then_with(|| self.libc.to_string().cmp(&other.libc.to_string())) + } +} + +impl PartialOrd for Platform { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl From<&uv_platform_tags::Platform> for Platform { + fn from(value: &uv_platform_tags::Platform) -> Self { + Self { + os: Os::from(value.os()), + arch: Arch::from(&value.arch()), + libc: Libc::from(value.os()), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_platform_display() { + let platform = Platform { + os: Os::from_str("linux").unwrap(), + arch: Arch::from_str("x86_64").unwrap(), + libc: Libc::from_str("gnu").unwrap(), + }; + assert_eq!(platform.to_string(), "linux-x86_64-gnu"); + } + + #[test] + fn test_platform_from_str() { + let platform = Platform::from_str("macos-aarch64-none").unwrap(); + assert_eq!(platform.os.to_string(), "macos"); + assert_eq!(platform.arch.to_string(), "aarch64"); + assert_eq!(platform.libc.to_string(), "none"); + } + + #[test] + fn test_platform_from_parts() { + let platform = Platform::from_parts("linux", "x86_64", "gnu").unwrap(); + assert_eq!(platform.os.to_string(), "linux"); + assert_eq!(platform.arch.to_string(), "x86_64"); + assert_eq!(platform.libc.to_string(), "gnu"); + + // Test with arch variant + let platform = Platform::from_parts("linux", "x86_64_v3", "musl").unwrap(); + assert_eq!(platform.os.to_string(), "linux"); + assert_eq!(platform.arch.to_string(), "x86_64_v3"); + assert_eq!(platform.libc.to_string(), "musl"); + + // Test error cases + assert!(Platform::from_parts("invalid_os", "x86_64", "gnu").is_err()); + assert!(Platform::from_parts("linux", "invalid_arch", "gnu").is_err()); + assert!(Platform::from_parts("linux", "x86_64", "invalid_libc").is_err()); + } + + #[test] + fn test_platform_from_str_with_arch_variant() { + let platform = Platform::from_str("linux-x86_64_v3-gnu").unwrap(); + assert_eq!(platform.os.to_string(), "linux"); + assert_eq!(platform.arch.to_string(), "x86_64_v3"); + assert_eq!(platform.libc.to_string(), "gnu"); + } + + #[test] + fn test_platform_from_str_error() { + // Too few parts + assert!(Platform::from_str("linux-x86_64").is_err()); + assert!(Platform::from_str("invalid").is_err()); + + // Too many parts (would have been accepted by the old code) + assert!(Platform::from_str("linux-x86-64-gnu").is_err()); + assert!(Platform::from_str("linux-x86_64-gnu-extra").is_err()); + } + + #[test] + fn test_platform_sorting_os_precedence() { + let linux = Platform::from_str("linux-x86_64-gnu").unwrap(); + let macos = Platform::from_str("macos-x86_64-none").unwrap(); + let windows = Platform::from_str("windows-x86_64-none").unwrap(); + + // OS sorting takes precedence (alphabetical) + assert!(linux < macos); + assert!(macos < windows); + } + + #[test] + fn test_platform_sorting_libc() { + let gnu = Platform::from_str("linux-x86_64-gnu").unwrap(); + let musl = Platform::from_str("linux-x86_64-musl").unwrap(); + + // Same OS and arch, libc comparison (alphabetical) + assert!(gnu < musl); + } + + #[test] + fn test_platform_sorting_arch_linux() { + // Test that Linux prefers the native architecture + use crate::arch::test_support::{aarch64, run_with_arch, x86_64}; + + let linux_x86_64 = Platform::from_str("linux-x86_64-gnu").unwrap(); + let linux_aarch64 = Platform::from_str("linux-aarch64-gnu").unwrap(); + + // On x86_64 Linux, x86_64 should be preferred over aarch64 + run_with_arch(x86_64(), || { + assert!(linux_x86_64 < linux_aarch64); + }); + + // On aarch64 Linux, aarch64 should be preferred over x86_64 + run_with_arch(aarch64(), || { + assert!(linux_aarch64 < linux_x86_64); + }); + } + + #[test] + fn test_platform_sorting_arch_macos() { + use crate::arch::test_support::{aarch64, run_with_arch, x86_64}; + + let macos_x86_64 = Platform::from_str("macos-x86_64-none").unwrap(); + let macos_aarch64 = Platform::from_str("macos-aarch64-none").unwrap(); + + // On x86_64 macOS, x86_64 should be preferred over aarch64 + run_with_arch(x86_64(), || { + assert!(macos_x86_64 < macos_aarch64); + }); + + // On aarch64 macOS, aarch64 should be preferred over x86_64 + run_with_arch(aarch64(), || { + assert!(macos_aarch64 < macos_x86_64); + }); + } + + #[test] + fn test_platform_supports() { + let native = Platform::from_str("linux-x86_64-gnu").unwrap(); + let same = Platform::from_str("linux-x86_64-gnu").unwrap(); + let different_arch = Platform::from_str("linux-aarch64-gnu").unwrap(); + let different_os = Platform::from_str("macos-x86_64-none").unwrap(); + let different_libc = Platform::from_str("linux-x86_64-musl").unwrap(); + + // Exact match + assert!(native.supports(&same)); + + // Different OS - not supported + assert!(!native.supports(&different_os)); + + // Different libc - not supported + assert!(!native.supports(&different_libc)); + + // Different architecture but same family + // x86_64 doesn't support aarch64 on Linux + assert!(!native.supports(&different_arch)); + + // Test architecture family support + let x86_64_v2 = Platform::from_str("linux-x86_64_v2-gnu").unwrap(); + let x86_64_v3 = Platform::from_str("linux-x86_64_v3-gnu").unwrap(); + + // These have the same architecture family (both x86_64) + assert_eq!(native.arch.family(), x86_64_v2.arch.family()); + assert_eq!(native.arch.family(), x86_64_v3.arch.family()); + + // Due to the family check, these should support each other + assert!(native.supports(&x86_64_v2)); + assert!(native.supports(&x86_64_v3)); + } + + #[test] + fn test_windows_aarch64_platform_sorting() { + // Test that on Windows, x86_64 is preferred over aarch64 + let windows_x86_64 = Platform::from_str("windows-x86_64-none").unwrap(); + let windows_aarch64 = Platform::from_str("windows-aarch64-none").unwrap(); + + // x86_64 should sort before aarch64 on Windows (preferred) + assert!(windows_x86_64 < windows_aarch64); + + // Test with multiple Windows platforms + let mut platforms = [ + Platform::from_str("windows-aarch64-none").unwrap(), + Platform::from_str("windows-x86_64-none").unwrap(), + Platform::from_str("windows-x86-none").unwrap(), + ]; + + platforms.sort(); + + // After sorting on Windows, the order should be: x86_64 (preferred), aarch64, x86 + // x86_64 is preferred on Windows regardless of native architecture + assert_eq!(platforms[0].arch.to_string(), "x86_64"); + assert_eq!(platforms[1].arch.to_string(), "aarch64"); + assert_eq!(platforms[2].arch.to_string(), "x86"); + } + + #[test] + fn test_windows_sorting_always_prefers_x86_64() { + // Test that Windows always prefers x86_64 regardless of host architecture + use crate::arch::test_support::{aarch64, run_with_arch, x86_64}; + + let windows_x86_64 = Platform::from_str("windows-x86_64-none").unwrap(); + let windows_aarch64 = Platform::from_str("windows-aarch64-none").unwrap(); + + // Even with aarch64 as host, Windows should still prefer x86_64 + run_with_arch(aarch64(), || { + assert!(windows_x86_64 < windows_aarch64); + }); + + // With x86_64 as host, Windows should still prefer x86_64 + run_with_arch(x86_64(), || { + assert!(windows_x86_64 < windows_aarch64); + }); + } + + #[test] + fn test_windows_aarch64_supports() { + // Test that Windows aarch64 can run x86_64 binaries through emulation + let windows_aarch64 = Platform::from_str("windows-aarch64-none").unwrap(); + let windows_x86_64 = Platform::from_str("windows-x86_64-none").unwrap(); + + // aarch64 Windows supports x86_64 through transparent emulation + assert!(windows_aarch64.supports(&windows_x86_64)); + + // But x86_64 doesn't support aarch64 + assert!(!windows_x86_64.supports(&windows_aarch64)); + + // Self-support should always work + assert!(windows_aarch64.supports(&windows_aarch64)); + assert!(windows_x86_64.supports(&windows_x86_64)); + } + + #[test] + fn test_from_platform_tags_platform() { + // Test conversion from uv_platform_tags::Platform to uv_platform::Platform + let tags_platform = uv_platform_tags::Platform::new( + uv_platform_tags::Os::Windows, + uv_platform_tags::Arch::X86_64, + ); + let platform = Platform::from(&tags_platform); + + assert_eq!(platform.os.to_string(), "windows"); + assert_eq!(platform.arch.to_string(), "x86_64"); + assert_eq!(platform.libc.to_string(), "none"); + + // Test with manylinux + let tags_platform_linux = uv_platform_tags::Platform::new( + uv_platform_tags::Os::Manylinux { + major: 2, + minor: 17, + }, + uv_platform_tags::Arch::Aarch64, + ); + let platform_linux = Platform::from(&tags_platform_linux); + + assert_eq!(platform_linux.os.to_string(), "linux"); + assert_eq!(platform_linux.arch.to_string(), "aarch64"); + assert_eq!(platform_linux.libc.to_string(), "gnu"); + } } diff --git a/crates/uv-platform/src/os.rs b/crates/uv-platform/src/os.rs index 245d799f3..89493adc0 100644 --- a/crates/uv-platform/src/os.rs +++ b/crates/uv-platform/src/os.rs @@ -19,6 +19,10 @@ impl Os { pub fn is_windows(&self) -> bool { matches!(self.0, target_lexicon::OperatingSystem::Windows) } + + pub fn is_macos(&self) -> bool { + matches!(self.0, target_lexicon::OperatingSystem::Darwin(_)) + } } impl Display for Os { diff --git a/crates/uv-python/src/discovery.rs b/crates/uv-python/src/discovery.rs index abd8c8ac1..a658e6b25 100644 --- a/crates/uv-python/src/discovery.rs +++ b/crates/uv-python/src/discovery.rs @@ -350,7 +350,7 @@ fn python_executables_from_installed<'a>( debug!("Skipping managed installation `{installation}`: does not satisfy `{version}`"); return false; } - if !platform.matches(installation.key()) { + if !platform.matches(installation.platform()) { debug!("Skipping managed installation `{installation}`: does not satisfy `{platform}`"); return false; } diff --git a/crates/uv-python/src/downloads.rs b/crates/uv-python/src/downloads.rs index f4579f869..4104e1d49 100644 --- a/crates/uv-python/src/downloads.rs +++ b/crates/uv-python/src/downloads.rs @@ -25,7 +25,7 @@ use uv_client::{BaseClient, WrappedReqwestError, is_extended_transient_error}; use uv_distribution_filename::{ExtensionError, SourceDistExtension}; use uv_extract::hash::Hasher; use uv_fs::{Simplified, rename_with_retry}; -use uv_platform::{self as platform, Arch, Libc, Os}; +use uv_platform::{self as platform, Arch, Libc, Os, Platform}; use uv_pypi_types::{HashAlgorithm, HashDigest}; use uv_redacted::DisplaySafeUrl; use uv_static::EnvVars; @@ -171,22 +171,22 @@ pub struct PlatformRequest { } impl PlatformRequest { - /// Check if this platform request is satisfied by an installation key. - pub fn matches(&self, key: &PythonInstallationKey) -> bool { + /// Check if this platform request is satisfied by a platform. + pub fn matches(&self, platform: &Platform) -> bool { if let Some(os) = self.os { - if key.os != os { + if platform.os != os { return false; } } if let Some(arch) = self.arch { - if !arch.satisfied_by(key.arch) { + if !arch.satisfied_by(platform) { return false; } } if let Some(libc) = self.libc { - if key.libc != libc { + if platform.libc != libc { return false; } } @@ -220,10 +220,14 @@ impl Display for ArchRequest { } impl ArchRequest { - pub(crate) fn satisfied_by(self, arch: Arch) -> bool { + pub(crate) fn satisfied_by(self, platform: &Platform) -> bool { match self { - Self::Explicit(request) => request == arch, - Self::Environment(env) => env.supports(arch), + Self::Explicit(request) => request == platform.arch, + Self::Environment(env) => { + // Check if the environment's platform can run the target platform + let env_platform = Platform::new(platform.os, env, platform.libc); + env_platform.supports(platform) + } } } @@ -327,14 +331,15 @@ impl PythonDownloadRequest { /// /// Platform information is pulled from the environment. pub fn fill_platform(mut self) -> Result { + let platform = Platform::from_env()?; if self.arch.is_none() { - self.arch = Some(ArchRequest::Environment(Arch::from_env())); + self.arch = Some(ArchRequest::Environment(platform.arch)); } if self.os.is_none() { - self.os = Some(Os::from_env()); + self.os = Some(platform.os); } if self.libc.is_none() { - self.libc = Some(Libc::from_env()?); + self.libc = Some(platform.libc); } Ok(self) } @@ -378,23 +383,16 @@ impl PythonDownloadRequest { /// Whether this request is satisfied by an installation key. pub fn satisfied_by_key(&self, key: &PythonInstallationKey) -> bool { - if let Some(os) = &self.os { - if key.os != *os { - return false; - } + // Check platform requirements + let request = PlatformRequest { + os: self.os, + arch: self.arch, + libc: self.libc, + }; + if !request.matches(key.platform()) { + return false; } - if let Some(arch) = &self.arch { - if !arch.satisfied_by(key.arch) { - return false; - } - } - - if let Some(libc) = &self.libc { - if key.libc != *libc { - return false; - } - } if let Some(implementation) = &self.implementation { if key.implementation != LenientImplementationName::from(*implementation) { return false; @@ -453,19 +451,20 @@ impl PythonDownloadRequest { } } if let Some(os) = self.os() { - let interpreter_os = Os::from(interpreter.platform().os()); - if &interpreter_os != os { + if &interpreter.os() != os { debug!( - "Skipping interpreter at `{executable}`: operating system `{interpreter_os}` does not match request `{os}`" + "Skipping interpreter at `{executable}`: operating system `{}` does not match request `{os}`", + interpreter.os() ); return false; } } if let Some(arch) = self.arch() { - let interpreter_arch = Arch::from(&interpreter.platform().arch()); - if !arch.satisfied_by(interpreter_arch) { + let interpreter_platform = Platform::from(interpreter.platform()); + if !arch.satisfied_by(&interpreter_platform) { debug!( - "Skipping interpreter at `{executable}`: architecture `{interpreter_arch}` does not match request `{arch}`" + "Skipping interpreter at `{executable}`: architecture `{}` does not match request `{arch}`", + interpreter.arch() ); return false; } @@ -482,10 +481,10 @@ impl PythonDownloadRequest { } } if let Some(libc) = self.libc() { - let interpreter_libc = Libc::from(interpreter.platform().os()); - if &interpreter_libc != libc { + if &interpreter.libc() != libc { debug!( - "Skipping interpreter at `{executable}`: libc `{interpreter_libc}` does not match request `{libc}`" + "Skipping interpreter at `{executable}`: libc `{}` does not match request `{libc}`", + interpreter.libc() ); return false; } @@ -514,9 +513,9 @@ impl From<&ManagedPythonInstallation> for PythonDownloadRequest { "Managed Python installations are expected to always have known implementation names, found {name}" ), }, - Some(ArchRequest::Explicit(key.arch)), - Some(key.os), - Some(key.libc), + Some(ArchRequest::Explicit(*key.arch())), + Some(*key.os()), + Some(*key.libc()), Some(key.prerelease.is_some()), ) } @@ -1184,9 +1183,7 @@ fn parse_json_downloads( key: PythonInstallationKey::new_from_version( implementation, &version, - os, - arch, - libc, + Platform::new(os, arch, libc), variant, ), url, diff --git a/crates/uv-python/src/installation.rs b/crates/uv-python/src/installation.rs index 09810e6f1..9ac1e6284 100644 --- a/crates/uv-python/src/installation.rs +++ b/crates/uv-python/src/installation.rs @@ -10,7 +10,7 @@ use uv_cache::Cache; use uv_client::BaseClientBuilder; use uv_configuration::Preview; use uv_pep440::{Prerelease, Version}; -use uv_platform::{Arch, Libc, Os}; +use uv_platform::{Arch, Libc, Os, Platform}; use crate::discovery::{ EnvironmentPreference, PythonRequest, find_best_python_installation, find_python_installation, @@ -352,9 +352,7 @@ pub struct PythonInstallationKey { pub(crate) minor: u8, pub(crate) patch: u8, pub(crate) prerelease: Option, - pub(crate) os: Os, - pub(crate) arch: Arch, - pub(crate) libc: Libc, + pub(crate) platform: Platform, pub(crate) variant: PythonVariant, } @@ -365,9 +363,7 @@ impl PythonInstallationKey { minor: u8, patch: u8, prerelease: Option, - os: Os, - arch: Arch, - libc: Libc, + platform: Platform, variant: PythonVariant, ) -> Self { Self { @@ -376,9 +372,7 @@ impl PythonInstallationKey { minor, patch, prerelease, - os, - arch, - libc, + platform, variant, } } @@ -386,9 +380,7 @@ impl PythonInstallationKey { pub fn new_from_version( implementation: LenientImplementationName, version: &PythonVersion, - os: Os, - arch: Arch, - libc: Libc, + platform: Platform, variant: PythonVariant, ) -> Self { Self { @@ -397,9 +389,7 @@ impl PythonInstallationKey { minor: version.minor(), patch: version.patch().unwrap_or_default(), prerelease: version.pre(), - os, - arch, - libc, + platform, variant, } } @@ -434,16 +424,20 @@ impl PythonInstallationKey { self.minor } + pub fn platform(&self) -> &Platform { + &self.platform + } + pub fn arch(&self) -> &Arch { - &self.arch + &self.platform.arch } pub fn os(&self) -> &Os { - &self.os + &self.platform.os } pub fn libc(&self) -> &Libc { - &self.libc + &self.platform.libc } pub fn variant(&self) -> &PythonVariant { @@ -489,7 +483,7 @@ impl fmt::Display for PythonInstallationKey { }; write!( f, - "{}-{}.{}.{}{}{}-{}-{}-{}", + "{}-{}.{}.{}{}{}-{}", self.implementation, self.major, self.minor, @@ -498,9 +492,7 @@ impl fmt::Display for PythonInstallationKey { .map(|pre| pre.to_string()) .unwrap_or_default(), variant, - self.os, - self.arch, - self.libc + self.platform ) } } @@ -510,31 +502,25 @@ impl FromStr for PythonInstallationKey { fn from_str(key: &str) -> Result { let parts = key.split('-').collect::>(); - let [implementation, version, os, arch, libc] = parts.as_slice() else { + + // We need exactly implementation-version-os-arch-libc + if parts.len() != 5 { return Err(PythonInstallationKeyError::ParseError( key.to_string(), - "not enough `-`-separated values".to_string(), + format!( + "expected exactly 5 `-`-separated values, got {}", + parts.len() + ), )); + } + + let [implementation_str, version_str, os, arch, libc] = parts.as_slice() else { + unreachable!() }; - let implementation = LenientImplementationName::from(*implementation); + let implementation = LenientImplementationName::from(*implementation_str); - let os = Os::from_str(os).map_err(|err| { - PythonInstallationKeyError::ParseError(key.to_string(), format!("invalid OS: {err}")) - })?; - - let arch = Arch::from_str(arch).map_err(|err| { - PythonInstallationKeyError::ParseError( - key.to_string(), - format!("invalid architecture: {err}"), - ) - })?; - - let libc = Libc::from_str(libc).map_err(|err| { - PythonInstallationKeyError::ParseError(key.to_string(), format!("invalid libc: {err}")) - })?; - - let (version, variant) = match version.split_once('+') { + let (version, variant) = match version_str.split_once('+') { Some((version, variant)) => { let variant = PythonVariant::from_str(variant).map_err(|()| { PythonInstallationKeyError::ParseError( @@ -544,7 +530,7 @@ impl FromStr for PythonInstallationKey { })?; (version, variant) } - None => (*version, PythonVariant::Default), + None => (*version_str, PythonVariant::Default), }; let version = PythonVersion::from_str(version).map_err(|err| { @@ -554,14 +540,22 @@ impl FromStr for PythonInstallationKey { ) })?; - Ok(Self::new_from_version( + let platform = Platform::from_parts(os, arch, libc).map_err(|err| { + PythonInstallationKeyError::ParseError( + key.to_string(), + format!("invalid platform: {err}"), + ) + })?; + + Ok(Self { implementation, - &version, - os, - arch, - libc, + major: version.major(), + minor: version.minor(), + patch: version.patch().unwrap_or_default(), + prerelease: version.pre(), + platform, variant, - )) + }) } } @@ -576,10 +570,8 @@ impl Ord for PythonInstallationKey { self.implementation .cmp(&other.implementation) .then_with(|| self.version().cmp(&other.version())) - .then_with(|| self.os.to_string().cmp(&other.os.to_string())) - // Architectures are sorted in preferred order, with native architectures first - .then_with(|| self.arch.cmp(&other.arch).reverse()) - .then_with(|| self.libc.to_string().cmp(&other.libc.to_string())) + // Platforms are sorted in preferred order for the target + .then_with(|| self.platform.cmp(&other.platform).reverse()) // Python variants are sorted in preferred order, with `Default` first .then_with(|| self.variant.cmp(&other.variant).reverse()) } @@ -632,14 +624,8 @@ impl fmt::Display for PythonInstallationMinorVersionKey { }; write!( f, - "{}-{}.{}{}-{}-{}-{}", - self.0.implementation, - self.0.major, - self.0.minor, - variant, - self.0.os, - self.0.arch, - self.0.libc, + "{}-{}.{}{}-{}", + self.0.implementation, self.0.major, self.0.minor, variant, self.0.platform, ) } } @@ -653,9 +639,9 @@ impl fmt::Debug for PythonInstallationMinorVersionKey { .field("major", &self.0.major) .field("minor", &self.0.minor) .field("variant", &self.0.variant) - .field("os", &self.0.os) - .field("arch", &self.0.arch) - .field("libc", &self.0.libc) + .field("os", &self.0.platform.os) + .field("arch", &self.0.platform.arch) + .field("libc", &self.0.platform.libc) .finish() } } @@ -667,9 +653,7 @@ impl PartialEq for PythonInstallationMinorVersionKey { self.0.implementation == other.0.implementation && self.0.major == other.0.major && self.0.minor == other.0.minor - && self.0.os == other.0.os - && self.0.arch == other.0.arch - && self.0.libc == other.0.libc + && self.0.platform == other.0.platform && self.0.variant == other.0.variant } } @@ -681,9 +665,7 @@ impl Hash for PythonInstallationMinorVersionKey { self.0.implementation.hash(state); self.0.major.hash(state); self.0.minor.hash(state); - self.0.os.hash(state); - self.0.arch.hash(state); - self.0.libc.hash(state); + self.0.platform.hash(state); self.0.variant.hash(state); } } @@ -693,3 +675,113 @@ impl From for PythonInstallationMinorVersionKey { Self(key) } } + +#[cfg(test)] +mod tests { + use super::*; + use uv_platform::ArchVariant; + + #[test] + fn test_python_installation_key_from_str() { + // Test basic parsing + let key = PythonInstallationKey::from_str("cpython-3.12.0-linux-x86_64-gnu").unwrap(); + assert_eq!( + key.implementation, + LenientImplementationName::Known(ImplementationName::CPython) + ); + assert_eq!(key.major, 3); + assert_eq!(key.minor, 12); + assert_eq!(key.patch, 0); + assert_eq!( + key.platform.os, + Os::new(target_lexicon::OperatingSystem::Linux) + ); + assert_eq!( + key.platform.arch, + Arch::new(target_lexicon::Architecture::X86_64, None) + ); + assert_eq!( + key.platform.libc, + Libc::Some(target_lexicon::Environment::Gnu) + ); + + // Test with architecture variant + let key = PythonInstallationKey::from_str("cpython-3.11.2-linux-x86_64_v3-musl").unwrap(); + assert_eq!( + key.implementation, + LenientImplementationName::Known(ImplementationName::CPython) + ); + assert_eq!(key.major, 3); + assert_eq!(key.minor, 11); + assert_eq!(key.patch, 2); + assert_eq!( + key.platform.os, + Os::new(target_lexicon::OperatingSystem::Linux) + ); + assert_eq!( + key.platform.arch, + Arch::new(target_lexicon::Architecture::X86_64, Some(ArchVariant::V3)) + ); + assert_eq!( + key.platform.libc, + Libc::Some(target_lexicon::Environment::Musl) + ); + + // Test with Python variant (freethreaded) + let key = PythonInstallationKey::from_str("cpython-3.13.0+freethreaded-macos-aarch64-none") + .unwrap(); + assert_eq!( + key.implementation, + LenientImplementationName::Known(ImplementationName::CPython) + ); + assert_eq!(key.major, 3); + assert_eq!(key.minor, 13); + assert_eq!(key.patch, 0); + assert_eq!(key.variant, PythonVariant::Freethreaded); + assert_eq!( + key.platform.os, + Os::new(target_lexicon::OperatingSystem::Darwin(None)) + ); + assert_eq!( + key.platform.arch, + Arch::new( + target_lexicon::Architecture::Aarch64(target_lexicon::Aarch64Architecture::Aarch64), + None + ) + ); + assert_eq!(key.platform.libc, Libc::None); + + // Test error cases + assert!(PythonInstallationKey::from_str("cpython-3.12.0-linux-x86_64").is_err()); + assert!(PythonInstallationKey::from_str("cpython-3.12.0").is_err()); + assert!(PythonInstallationKey::from_str("cpython").is_err()); + } + + #[test] + fn test_python_installation_key_display() { + let key = PythonInstallationKey { + implementation: LenientImplementationName::from("cpython"), + major: 3, + minor: 12, + patch: 0, + prerelease: None, + platform: Platform::from_str("linux-x86_64-gnu").unwrap(), + variant: PythonVariant::Default, + }; + assert_eq!(key.to_string(), "cpython-3.12.0-linux-x86_64-gnu"); + + let key_with_variant = PythonInstallationKey { + implementation: LenientImplementationName::from("cpython"), + major: 3, + minor: 13, + patch: 0, + prerelease: None, + platform: Platform::from_str("macos-aarch64-none").unwrap(), + variant: PythonVariant::Freethreaded, + }; + assert_eq!( + key_with_variant.to_string(), + "cpython-3.13.0+freethreaded-macos-aarch64-none" + ); + } +} diff --git a/crates/uv-python/src/interpreter.rs b/crates/uv-python/src/interpreter.rs index 827199bf1..37b644f9f 100644 --- a/crates/uv-python/src/interpreter.rs +++ b/crates/uv-python/src/interpreter.rs @@ -22,8 +22,7 @@ use uv_install_wheel::Layout; use uv_pep440::Version; use uv_pep508::{MarkerEnvironment, StringVersion}; use uv_platform::{Arch, Libc, Os}; -use uv_platform_tags::Platform; -use uv_platform_tags::{Tags, TagsError}; +use uv_platform_tags::{Platform, Tags, TagsError}; use uv_pypi_types::{ResolverMarkerEnvironment, Scheme}; use crate::implementation::LenientImplementationName; @@ -205,9 +204,7 @@ impl Interpreter { self.python_minor(), self.python_patch(), self.python_version().pre(), - self.os(), - self.arch(), - self.libc(), + uv_platform::Platform::new(self.os(), self.arch(), self.libc()), self.variant(), ) } diff --git a/crates/uv-python/src/managed.rs b/crates/uv-python/src/managed.rs index 5ab44be08..7207e354a 100644 --- a/crates/uv-python/src/managed.rs +++ b/crates/uv-python/src/managed.rs @@ -18,7 +18,7 @@ use windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT; use uv_fs::{LockedFile, Simplified, replace_symlink, symlink_or_copy_file}; use uv_platform::Error as PlatformError; -use uv_platform::{Arch, Libc, LibcDetectionError, Os}; +use uv_platform::{LibcDetectionError, Platform}; use uv_state::{StateBucket, StateStore}; use uv_static::EnvVars; use uv_trampoline_builder::{Launcher, windows_python_launcher}; @@ -259,20 +259,11 @@ impl ManagedPythonInstallations { pub fn find_matching_current_platform( &self, ) -> Result + use<>, Error> { - let os = Os::from_env(); - let arch = Arch::from_env(); - let libc = Libc::from_env()?; + let platform = Platform::from_env()?; let iter = Self::from_settings(None)? .find_all()? - .filter(move |installation| { - installation.key.os == os - && (arch.supports(installation.key.arch) - // TODO(zanieb): Allow inequal variants, as `Arch::supports` does not - // implement this yet. See https://github.com/astral-sh/uv/pull/9788 - || arch.family() == installation.key.arch.family()) - && installation.key.libc == libc - }); + .filter(move |installation| platform.supports(installation.platform())); Ok(iter) } @@ -451,6 +442,10 @@ impl ManagedPythonInstallation { &self.key } + pub fn platform(&self) -> &Platform { + self.key.platform() + } + pub fn minor_version_key(&self) -> &PythonInstallationMinorVersionKey { PythonInstallationMinorVersionKey::ref_cast(&self.key) } @@ -544,7 +539,7 @@ impl ManagedPythonInstallation { /// standard `EXTERNALLY-MANAGED` file. pub fn ensure_externally_managed(&self) -> Result<(), Error> { // Construct the path to the `stdlib` directory. - let stdlib = if self.key.os.is_windows() { + let stdlib = if self.key.os().is_windows() { self.python_dir().join("Lib") } else { let lib_suffix = self.key.variant.suffix(); @@ -588,7 +583,7 @@ impl ManagedPythonInstallation { /// See for more information. pub fn ensure_dylib_patched(&self) -> Result<(), macos_dylib::Error> { if cfg!(target_os = "macos") { - if self.key().os.is_like_darwin() { + if self.key().os().is_like_darwin() { if *self.implementation() == ImplementationName::CPython { let dylib_path = self.python_dir().join("lib").join(format!( "{}python{}{}{}", @@ -890,10 +885,7 @@ pub fn create_link_to_executable(link: &Path, executable: &Path) -> Result<(), E // TODO(zanieb): Only used in tests now. /// Generate a platform portion of a key from the environment. pub fn platform_key_from_env() -> Result { - let os = Os::from_env(); - let arch = Arch::from_env(); - let libc = Libc::from_env()?; - Ok(format!("{os}-{arch}-{libc}").to_lowercase()) + Ok(Platform::from_env()?.to_string().to_lowercase()) } impl fmt::Display for ManagedPythonInstallation {