From 2777909cda73315fcc15782ea5d51627b4839264 Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Tue, 12 Nov 2024 11:42:03 -0600 Subject: [PATCH] Prefer Python executable names that match the request over default names --- crates/uv-python/src/discovery.rs | 124 ++++++++++++++++++++---- crates/uv-python/src/discovery/tests.rs | 33 ++++--- crates/uv-python/src/implementation.rs | 4 + 3 files changed, 128 insertions(+), 33 deletions(-) diff --git a/crates/uv-python/src/discovery.rs b/crates/uv-python/src/discovery.rs index 2195b18bd980..00b015c887b8 100644 --- a/crates/uv-python/src/discovery.rs +++ b/crates/uv-python/src/discovery.rs @@ -1600,9 +1600,9 @@ impl EnvironmentPreference { } } -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone, Default, Copy, PartialEq, Eq)] pub(crate) struct ExecutableName { - name: &'static str, + implementation: Option, major: Option, minor: Option, patch: Option, @@ -1610,10 +1610,92 @@ pub(crate) struct ExecutableName { variant: PythonVariant, } +#[derive(Debug, Clone, PartialEq, Eq)] +struct ExecutableNameComparator<'a> { + name: ExecutableName, + request: &'a VersionRequest, + implementation: Option<&'a ImplementationName>, +} + +impl Ord for ExecutableNameComparator<'_> { + /// Note the comparison returns a reverse priority ordering. + /// + /// Higher priority items are "Greater" than lower priority items. + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + // Prefer the default name over a specific implementation, unless an implementation was + // requested + let name_ordering = if self.implementation.is_some() { + std::cmp::Ordering::Greater + } else { + std::cmp::Ordering::Less + }; + if self.name.implementation.is_none() && other.name.implementation.is_some() { + return name_ordering.reverse(); + } + if self.name.implementation.is_some() && other.name.implementation.is_none() { + return name_ordering; + } + // Otherwise, use the names in supported order + let ordering = self.name.implementation.cmp(&other.name.implementation); + if ordering != std::cmp::Ordering::Equal { + return ordering; + } + let ordering = self.name.major.cmp(&other.name.major); + let is_default_request = + matches!(self.request, VersionRequest::Any | VersionRequest::Default); + if ordering != std::cmp::Ordering::Equal { + return if is_default_request { + ordering.reverse() + } else { + ordering + }; + } + let ordering = self.name.minor.cmp(&other.name.minor); + if ordering != std::cmp::Ordering::Equal { + return if is_default_request { + ordering.reverse() + } else { + ordering + }; + } + let ordering = self.name.patch.cmp(&other.name.patch); + if ordering != std::cmp::Ordering::Equal { + return if is_default_request { + ordering.reverse() + } else { + ordering + }; + } + let ordering = self.name.prerelease.cmp(&other.name.prerelease); + if ordering != std::cmp::Ordering::Equal { + return if is_default_request { + ordering.reverse() + } else { + ordering + }; + } + let ordering = self.name.variant.cmp(&other.name.variant); + if ordering != std::cmp::Ordering::Equal { + return if is_default_request { + ordering.reverse() + } else { + ordering + }; + } + ordering + } +} + +impl PartialOrd for ExecutableNameComparator<'_> { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + impl ExecutableName { #[must_use] - fn with_name(mut self, name: &'static str) -> Self { - self.name = name; + fn with_implementation(mut self, implementation: ImplementationName) -> Self { + self.implementation = Some(implementation); self } @@ -1646,24 +1728,27 @@ impl ExecutableName { self.variant = variant; self } -} -impl Default for ExecutableName { - fn default() -> Self { - Self { - name: "python", - major: None, - minor: None, - patch: None, - prerelease: None, - variant: PythonVariant::Default, + fn into_comparator<'a>( + self, + request: &'a VersionRequest, + implementation: Option<&'a ImplementationName>, + ) -> ExecutableNameComparator<'a> { + ExecutableNameComparator { + name: self, + request, + implementation, } } } impl std::fmt::Display for ExecutableName { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}", self.name)?; + if let Some(implementation) = self.implementation { + write!(f, "{implementation}")?; + } else { + f.write_str("python")?; + } if let Some(major) = self.major { write!(f, "{major}")?; if let Some(minor) = self.minor { @@ -1741,15 +1826,15 @@ impl VersionRequest { // Add all the implementation-specific names if let Some(implementation) = implementation { for i in 0..names.len() { - let name = names[i].with_name(implementation.into()); + let name = names[i].with_implementation(*implementation); names.push(name); } } else { // When looking for all implementations, include all possible names if matches!(self, Self::Any) { for i in 0..names.len() { - for implementation in ImplementationName::long_names() { - let name = names[i].with_name(implementation); + for implementation in ImplementationName::iter_all() { + let name = names[i].with_implementation(implementation); names.push(name); } } @@ -1764,6 +1849,9 @@ impl VersionRequest { } } + names.sort_unstable_by_key(|name| name.into_comparator(self, implementation)); + names.reverse(); + names } diff --git a/crates/uv-python/src/discovery/tests.rs b/crates/uv-python/src/discovery/tests.rs index 9dbd688f367e..97057a3816ad 100644 --- a/crates/uv-python/src/discovery/tests.rs +++ b/crates/uv-python/src/discovery/tests.rs @@ -477,49 +477,52 @@ fn executable_names_from_request() { case( "any", &[ - "python", "python3", "cpython", "pypy", "graalpy", "cpython3", "pypy3", "graalpy3", + "python", "python3", "cpython", "cpython3", "pypy", "pypy3", "graalpy", "graalpy3", ], ); case("default", &["python", "python3"]); - case("3", &["python", "python3"]); + case("3", &["python3", "python"]); - case("4", &["python", "python4"]); + case("4", &["python4", "python"]); - case("3.13", &["python", "python3", "python3.13"]); + case("3.13", &["python3.13", "python3", "python"]); + + case("pypy", &["pypy", "pypy3", "python", "python3"]); case( "pypy@3.10", &[ - "python", - "python3", - "python3.10", - "pypy", - "pypy3", "pypy3.10", + "pypy3", + "pypy", + "python3.10", + "python3", + "python", ], ); case( "3.13t", &[ - "python", - "python3", + "python3.13t", "python3.13", - "pythont", "python3t", - "python3.13t", + "python3", + "pythont", + "python", ], ); + case("3t", &["python3t", "python3", "pythont", "python"]); case( "3.13.2", - &["python", "python3", "python3.13", "python3.13.2"], + &["python3.13.2", "python3.13", "python3", "python"], ); case( "3.13rc2", - &["python", "python3", "python3.13", "python3.13rc2"], + &["python3.13rc2", "python3.13", "python3", "python"], ); } diff --git a/crates/uv-python/src/implementation.rs b/crates/uv-python/src/implementation.rs index 7d405d48deb6..ffc61dac7ffa 100644 --- a/crates/uv-python/src/implementation.rs +++ b/crates/uv-python/src/implementation.rs @@ -33,6 +33,10 @@ impl ImplementationName { ["cpython", "pypy", "graalpy"].into_iter() } + pub(crate) fn iter_all() -> impl Iterator { + [Self::CPython, Self::PyPy, Self::GraalPy].into_iter() + } + pub fn pretty(self) -> &'static str { match self { Self::CPython => "CPython",