Skip to content

Commit

Permalink
Require opt-in to use alternative Python implementations
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Sep 24, 2024
1 parent 77c2496 commit b7524ed
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 14 deletions.
61 changes: 54 additions & 7 deletions crates/uv-python/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,7 @@ pub fn find_python_installations<'a>(
///
/// If an error is encountered while locating or inspecting a candidate installation,
/// the error will raised instead of attempting further candidates.
#[allow(clippy::nonminimal_bool)]
pub(crate) fn find_python_installation(
request: &PythonRequest,
environments: EnvironmentPreference,
Expand All @@ -934,21 +935,44 @@ pub(crate) fn find_python_installation(
return result;
};

// If it's a pre-release, and pre-releases aren't allowed skip it but store it for later
// Check if we need to skip the interpreter because it is "not allowed", e.g., if it is a
// pre-release version or an alternative implementation, using it requires opt-in.

// If the interpreter has a default executable name, e.g. `python`, and was found on the
// search path, we consider this opt-in to use it.
let has_default_executable_name = installation.interpreter.has_default_executable_name()
&& installation.source == PythonSource::SearchPath;

// If it's a pre-release and pre-releases aren't allowed, skip it — but store it for later
// since we'll use a pre-release if no other versions are available.
if installation.python_version().pre().is_some()
&& !request.allows_prereleases()
&& !installation.source.allows_prereleases()
&& !has_default_executable_name
{
debug!("Skipping pre-release {}", installation.key());
first_prerelease = Some(installation.clone());
continue;
}

// If it's an alternative implementation and alternative implementations aren't allowed,
// skip it. Note we avoid querying these interpreters at all if they're on the search path
// and are not requested, but other sources such as the managed installations will include
// them.
if installation.is_alternative_implementation()
&& !request.allows_alternative_implementations()
&& !installation.source.allows_alternative_implementations()
&& !has_default_executable_name
{
debug!("Skipping alternative implementation {}", installation.key());
continue;
}

// If we didn't skip it, this is the installation to use
return result;
}

// If we only found pre-releases, they're implicitly allowed and we should return the first one
// If we only found pre-releases, they're implicitly allowed and we should return the first one.
if let Some(installation) = first_prerelease {
return Ok(Ok(installation));
}
Expand Down Expand Up @@ -1205,10 +1229,7 @@ impl PythonRequest {
for implementation in
ImplementationName::long_names().chain(ImplementationName::short_names())
{
if let Some(remainder) = value
.to_ascii_lowercase()
.strip_prefix(Into::<&str>::into(implementation))
{
if let Some(remainder) = value.to_ascii_lowercase().strip_prefix(implementation) {
// e.g. `pypy`
if remainder.is_empty() {
return Self::Implementation(
Expand Down Expand Up @@ -1369,6 +1390,7 @@ impl PythonRequest {
}
}

/// Whether this request opts-in to a pre-release Python version.
pub(crate) fn allows_prereleases(&self) -> bool {
match self {
Self::Default => false,
Expand All @@ -1381,6 +1403,19 @@ impl PythonRequest {
}
}

/// Whether this request opts-in to an alternative Python implementation, e.g., PyPy.
pub(crate) fn allows_alternative_implementations(&self) -> bool {
match self {
Self::Default => false,
Self::Any => true,
Self::Version(_) => false,
Self::Directory(_) | Self::File(_) | Self::ExecutableName(_) => true,
Self::Implementation(_) => true,
Self::ImplementationVersion(_, _) => true,
Self::Key(request) => request.allows_alternative_implementations(),
}
}

pub(crate) fn is_explicit_system(&self) -> bool {
matches!(self, Self::File(_) | Self::Directory(_))
}
Expand Down Expand Up @@ -1410,7 +1445,7 @@ impl PythonSource {
matches!(self, Self::Managed)
}

/// Whether a pre-release Python installation from the source should be used without opt-in.
/// Whether a pre-release Python installation from this source can be used without opt-in.
pub(crate) fn allows_prereleases(self) -> bool {
match self {
Self::Managed | Self::Registry | Self::MicrosoftStore => false,
Expand All @@ -1422,6 +1457,18 @@ impl PythonSource {
| Self::DiscoveredEnvironment => true,
}
}

/// Whether an alternative Python implementation from this source can be used without opt-in.
pub(crate) fn allows_alternative_implementations(self) -> bool {
match self {
Self::Managed | Self::Registry | Self::SearchPath | Self::MicrosoftStore => false,
Self::CondaPrefix
| Self::ProvidedPath
| Self::ParentInterpreter
| Self::ActiveEnvironment
| Self::DiscoveredEnvironment => true,
}
}
}

impl PythonPreference {
Expand Down
6 changes: 6 additions & 0 deletions crates/uv-python/src/downloads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ impl PythonDownloadRequest {
self.satisfied_by_key(download.key())
}

/// Whether this download request opts-in to pre-release Python versions.
pub fn allows_prereleases(&self) -> bool {
self.prereleases.unwrap_or_else(|| {
self.version
Expand All @@ -289,6 +290,11 @@ impl PythonDownloadRequest {
})
}

/// Whether this download request opts-in to alternative Python implementations.
pub fn allows_alternative_implementations(&self) -> bool {
self.implementation.is_some()
}

pub fn satisfied_by_interpreter(&self, interpreter: &Interpreter) -> bool {
if let Some(version) = self.version() {
if !version.matches_interpreter(interpreter) {
Expand Down
14 changes: 10 additions & 4 deletions crates/uv-python/src/implementation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,18 +52,24 @@ impl LenientImplementationName {
}

impl From<&ImplementationName> for &'static str {
fn from(v: &ImplementationName) -> &'static str {
match v {
fn from(value: &ImplementationName) -> &'static str {
match value {
ImplementationName::CPython => "cpython",
ImplementationName::PyPy => "pypy",
ImplementationName::GraalPy => "graalpy",
}
}
}

impl From<ImplementationName> for &'static str {
fn from(value: ImplementationName) -> &'static str {
(&value).into()
}
}

impl<'a> From<&'a LenientImplementationName> for &'a str {
fn from(v: &'a LenientImplementationName) -> &'a str {
match v {
fn from(value: &'a LenientImplementationName) -> &'a str {
match value {
LenientImplementationName::Known(implementation) => implementation.into(),
LenientImplementationName::Unknown(name) => name,
}
Expand Down
13 changes: 12 additions & 1 deletion crates/uv-python/src/installation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ use crate::implementation::LenientImplementationName;
use crate::managed::{ManagedPythonInstallation, ManagedPythonInstallations};
use crate::platform::{Arch, Libc, Os};
use crate::{
downloads, Error, Interpreter, PythonDownloads, PythonPreference, PythonSource, PythonVersion,
downloads, Error, ImplementationName, Interpreter, PythonDownloads, PythonPreference,
PythonSource, PythonVersion,
};

/// A Python interpreter and accompanying tools.
Expand Down Expand Up @@ -176,6 +177,16 @@ impl PythonInstallation {
LenientImplementationName::from(self.interpreter.implementation_name())
}

/// Whether this is a CPython installation.
///
/// Returns false if it is an alternative implementation, e.g., PyPy.
pub(crate) fn is_alternative_implementation(&self) -> bool {
!matches!(
self.implementation(),
LenientImplementationName::Known(ImplementationName::CPython)
)
}

/// Return the [`Arch`] of the Python installation as reported by its interpreter.
pub fn arch(&self) -> Arch {
self.interpreter.arch()
Expand Down
19 changes: 18 additions & 1 deletion crates/uv-python/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ use uv_fs::{write_atomic_sync, PythonExt, Simplified};
use crate::implementation::LenientImplementationName;
use crate::platform::{Arch, Libc, Os};
use crate::pointer_size::PointerSize;
use crate::{Prefix, PythonInstallationKey, PythonVersion, Target, VirtualEnvironment};
use crate::{
Prefix, PythonInstallationKey, PythonVersion, Target, VersionRequest, VirtualEnvironment,
};

/// A Python executable and its associated platform markers.
#[derive(Debug, Clone)]
Expand Down Expand Up @@ -494,6 +496,21 @@ impl Interpreter {
(version.major(), version.minor()) == self.python_tuple()
}
}

/// Whether or not this Python interpreter is from a default Python executable name, like
/// `python`, `python3`, or `python.exe`.
pub(crate) fn has_default_executable_name(&self) -> bool {
let Some(file_name) = self.sys_executable().file_name() else {
return false;
};
let Some(name) = file_name.to_str() else {
return false;
};
VersionRequest::Default
.executable_names(None)
.into_iter()
.any(|default_name| name == default_name.to_string())
}
}

/// The `EXTERNALLY-MANAGED` file in a Python installation.
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-python/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1793,7 +1793,7 @@ mod tests {
})?;
assert!(
matches!(result, Err(PythonNotFound { .. })),
"We should not the pypy interpreter if not named `python` or requested; got {result:?}"
"We should not find the pypy interpreter if not named `python` or requested; got {result:?}"
);

// But we should find it
Expand Down

0 comments on commit b7524ed

Please sign in to comment.