Prefer Python executable names that match the request over default names (#9066)

This restores behavior previously removed in
https://github.com/astral-sh/uv/pull/7649.

I thought it'd be clearer (and simpler) to have a consistent Python
executable name ordering. However, we've seen some cases where this can
be surprising and, in combination with #8481, can result in incorrect
behavior. For example, see https://github.com/astral-sh/uv/issues/9046
where we prefer `python3` over `python3.12` in the same directory even
though `python3.12` was requested. While `python3` and `python3.12` both
point to valid Python 3.12 environments there, the expectation is that
when `python3.12` is requested that the `python3.12` executable is
preferred. This expectation may be less obvious if the user requests
`python@3.12`, but uv does not distinguish between these request forms.
Similarly, this may be surprising as by default uv prefers `python` over
`python3` but when requesting `python3.12` the preference will be
swapped.
This commit is contained in:
Zanie Blue 2024-11-13 10:00:23 -06:00 committed by GitHub
parent 15ef807c80
commit 2966471db2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 302 additions and 175 deletions

View file

@ -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<ImplementationName>,
major: Option<u8>,
minor: Option<u8>,
patch: Option<u8>,
@ -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<std::cmp::Ordering> {
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
}

View file

@ -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.13",
"pythont",
"python3t",
"python3.13t",
"python3.13",
"python3t",
"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"],
);
}

View file

@ -33,6 +33,10 @@ impl ImplementationName {
["cpython", "pypy", "graalpy"].into_iter()
}
pub(crate) fn iter_all() -> impl Iterator<Item = Self> {
[Self::CPython, Self::PyPy, Self::GraalPy].into_iter()
}
pub fn pretty(self) -> &'static str {
match self {
Self::CPython => "CPython",

View file

@ -2087,83 +2087,7 @@ fn find_python_graalpy_request_ignores_cpython() -> Result<()> {
}
#[test]
fn find_python_prefers_generic_executable_over_implementation_name() -> Result<()> {
let mut context = TestContext::new()?;
// We prefer `python` executables over `graalpy` executables in the same directory
// if they are both GraalPy
TestContext::create_mock_interpreter(
&context.tempdir.join("python"),
&PythonVersion::from_str("3.10.0").unwrap(),
ImplementationName::GraalPy,
true,
false,
)?;
TestContext::create_mock_interpreter(
&context.tempdir.join("graalpy"),
&PythonVersion::from_str("3.10.1").unwrap(),
ImplementationName::GraalPy,
true,
false,
)?;
context.add_to_search_path(context.tempdir.to_path_buf());
let python = context.run(|| {
find_python_installation(
&PythonRequest::parse("graalpy@3.10"),
EnvironmentPreference::Any,
PythonPreference::OnlySystem,
&context.cache,
)
})??;
assert_eq!(
python.interpreter().python_full_version().to_string(),
"3.10.0",
);
// And `python` executables earlier in the search path will take precedence
context.reset_search_path();
context.add_python_interpreters(&[
(true, ImplementationName::GraalPy, "python", "3.10.2"),
(true, ImplementationName::GraalPy, "graalpy", "3.10.3"),
])?;
let python = context.run(|| {
find_python_installation(
&PythonRequest::parse("graalpy@3.10"),
EnvironmentPreference::Any,
PythonPreference::OnlySystem,
&context.cache,
)
})??;
assert_eq!(
python.interpreter().python_full_version().to_string(),
"3.10.2",
);
// But `graalpy` executables earlier in the search path will take precedence
context.reset_search_path();
context.add_python_interpreters(&[
(true, ImplementationName::GraalPy, "graalpy", "3.10.3"),
(true, ImplementationName::GraalPy, "python", "3.10.2"),
])?;
let python = context.run(|| {
find_python_installation(
&PythonRequest::parse("graalpy@3.10"),
EnvironmentPreference::Any,
PythonPreference::OnlySystem,
&context.cache,
)
})??;
assert_eq!(
python.interpreter().python_full_version().to_string(),
"3.10.3",
);
Ok(())
}
#[test]
fn find_python_prefers_generic_executable_over_one_with_version() -> Result<()> {
fn find_python_executable_name_preference() -> Result<()> {
let mut context = TestContext::new()?;
TestContext::create_mock_interpreter(
&context.tempdir.join("pypy3.10"),
@ -2181,18 +2105,38 @@ fn find_python_prefers_generic_executable_over_one_with_version() -> Result<()>
)?;
context.add_to_search_path(context.tempdir.to_path_buf());
let python = context.run(|| {
find_python_installation(
&PythonRequest::parse("pypy@3.10"),
EnvironmentPreference::Any,
PythonPreference::OnlySystem,
&context.cache,
)
})??;
let python = context
.run(|| {
find_python_installation(
&PythonRequest::parse("pypy@3.10"),
EnvironmentPreference::Any,
PythonPreference::OnlySystem,
&context.cache,
)
})
.unwrap()
.unwrap();
assert_eq!(
python.interpreter().python_full_version().to_string(),
"3.10.0",
"We should prefer the versioned one when a version is requested"
);
let python = context
.run(|| {
find_python_installation(
&PythonRequest::parse("pypy"),
EnvironmentPreference::Any,
PythonPreference::OnlySystem,
&context.cache,
)
})
.unwrap()
.unwrap();
assert_eq!(
python.interpreter().python_full_version().to_string(),
"3.10.1",
"We should prefer the generic executable over one with the version number"
"We should prefer the generic one when no version is requested"
);
let mut context = TestContext::new()?;
@ -2210,20 +2154,126 @@ fn find_python_prefers_generic_executable_over_one_with_version() -> Result<()>
true,
false,
)?;
TestContext::create_mock_interpreter(
&context.tempdir.join("python"),
&PythonVersion::from_str("3.10.2").unwrap(),
ImplementationName::PyPy,
true,
false,
)?;
context.add_to_search_path(context.tempdir.to_path_buf());
let python = context.run(|| {
find_python_installation(
&PythonRequest::parse("pypy@3.10"),
EnvironmentPreference::Any,
PythonPreference::OnlySystem,
&context.cache,
)
})??;
let python = context
.run(|| {
find_python_installation(
&PythonRequest::parse("pypy@3.10"),
EnvironmentPreference::Any,
PythonPreference::OnlySystem,
&context.cache,
)
})
.unwrap()
.unwrap();
assert_eq!(
python.interpreter().python_full_version().to_string(),
"3.10.0",
"We should prefer the generic name with a version over one the implementation name"
"3.10.1",
"We should prefer the implementation name over the generic name"
);
let python = context
.run(|| {
find_python_installation(
&PythonRequest::parse("default"),
EnvironmentPreference::Any,
PythonPreference::OnlySystem,
&context.cache,
)
})
.unwrap()
.unwrap();
assert_eq!(
python.interpreter().python_full_version().to_string(),
"3.10.2",
"We should prefer the generic name over the implementation name, but not the versioned name"
);
// We prefer `python` executables over `graalpy` executables in the same directory
// if they are both GraalPy
let mut context = TestContext::new()?;
TestContext::create_mock_interpreter(
&context.tempdir.join("python"),
&PythonVersion::from_str("3.10.0").unwrap(),
ImplementationName::GraalPy,
true,
false,
)?;
TestContext::create_mock_interpreter(
&context.tempdir.join("graalpy"),
&PythonVersion::from_str("3.10.1").unwrap(),
ImplementationName::GraalPy,
true,
false,
)?;
context.add_to_search_path(context.tempdir.to_path_buf());
let python = context
.run(|| {
find_python_installation(
&PythonRequest::parse("graalpy@3.10"),
EnvironmentPreference::Any,
PythonPreference::OnlySystem,
&context.cache,
)
})
.unwrap()
.unwrap();
assert_eq!(
python.interpreter().python_full_version().to_string(),
"3.10.1",
);
// And `python` executables earlier in the search path will take precedence
context.reset_search_path();
context.add_python_interpreters(&[
(true, ImplementationName::GraalPy, "python", "3.10.2"),
(true, ImplementationName::GraalPy, "graalpy", "3.10.3"),
])?;
let python = context
.run(|| {
find_python_installation(
&PythonRequest::parse("graalpy@3.10"),
EnvironmentPreference::Any,
PythonPreference::OnlySystem,
&context.cache,
)
})
.unwrap()
.unwrap();
assert_eq!(
python.interpreter().python_full_version().to_string(),
"3.10.2",
);
// And `graalpy` executables earlier in the search path will take precedence
context.reset_search_path();
context.add_python_interpreters(&[
(true, ImplementationName::GraalPy, "graalpy", "3.10.3"),
(true, ImplementationName::GraalPy, "python", "3.10.2"),
])?;
let python = context
.run(|| {
find_python_installation(
&PythonRequest::parse("graalpy@3.10"),
EnvironmentPreference::Any,
PythonPreference::OnlySystem,
&context.cache,
)
})
.unwrap()
.unwrap();
assert_eq!(
python.interpreter().python_full_version().to_string(),
"3.10.3",
);
Ok(())