Skip to content

Commit

Permalink
Filter discovered Python executables by source before querying
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Jan 31, 2025
1 parent c9c7b81 commit 4bf6a6b
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 27 deletions.
127 changes: 105 additions & 22 deletions crates/uv-python/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,16 +397,17 @@ fn python_executables_from_installed<'a>(

/// Lazily iterate over all discoverable Python executables.
///
/// Note that Python executables may be excluded by the given [`EnvironmentPreference`] and [`PythonPreference`].
/// Note that Python executables may be excluded by the given [`EnvironmentPreference`] and
/// [`PythonPreference`].
///
/// See [`python_executables_from_installed`] and [`python_executables_from_environments`]
/// See [`python_executables_from_installed`] and [`python_executables_from_virtual_environments`]
/// for more information on discovery.
fn python_executables<'a>(
version: &'a VersionRequest,
implementation: Option<&'a ImplementationName>,
environments: EnvironmentPreference,
preference: PythonPreference,
) -> Box<dyn Iterator<Item = Result<(PythonSource, PathBuf), Error>> + 'a> {
) -> impl Iterator<Item = Result<(PythonSource, PathBuf), Error>> + 'a {
// Always read from `UV_INTERNAL__PARENT_INTERPRETER` — it could be a system interpreter
let from_parent_interpreter = iter::once_with(|| {
env::var_os(EnvVars::UV_INTERNAL__PARENT_INTERPRETER)
Expand All @@ -429,22 +430,16 @@ fn python_executables<'a>(

// Limit the search to the relevant environment preference; we later validate that they match
// the preference but queries are expensive and we query less interpreters this way.
match environments {
EnvironmentPreference::OnlyVirtual => {
Box::new(from_parent_interpreter.chain(from_virtual_environments))
}
EnvironmentPreference::ExplicitSystem | EnvironmentPreference::Any => Box::new(
from_parent_interpreter
.chain(from_virtual_environments)
.chain(from_base_conda_environment)
.chain(from_installed),
),
EnvironmentPreference::OnlySystem => Box::new(
from_parent_interpreter
.chain(from_base_conda_environment)
.chain(from_installed),
),
}
from_parent_interpreter
.chain(from_virtual_environments)
.chain(from_base_conda_environment)
.chain(from_installed)
.filter(move |result| {
result.is_err()
|| result.as_ref().is_ok_and(|(source, path)| {
source_satisfies_environment_preference(*source, path, environments)
})
})
}

/// Lazily iterate over Python executables in the `PATH`.
Expand Down Expand Up @@ -630,8 +625,13 @@ fn python_interpreters_from_executables<'a>(
})
}

/// Returns true if a Python interpreter matches the [`EnvironmentPreference`].
fn satisfies_environment_preference(
/// Whether a [`Interpreter`] matches the [`EnvironmentPreference`].
///
/// This is the correct way to determine if an interpreter matches the preference. In contrast,
/// [`source_satisfies_environment_preference`] only checks if a [`PythonSource`] **could** satisfy
/// preference as a pre-filtering step. We cannot definitely know if a Python interpreter is in
/// a virtual environment until we query it.
fn interpreter_satisfies_environment_preference(
source: PythonSource,
interpreter: &Interpreter,
preference: EnvironmentPreference,
Expand Down Expand Up @@ -680,6 +680,55 @@ fn satisfies_environment_preference(
}
}

/// Returns true if a [`PythonSource`] could satisfy the [`EnvironmentPreference`].
///
/// This is useful as a pre-filtering step. Use of [`interpreter_satisfies_environment_preference`]
/// is required to determine if an [`Interpreter`] satisfies the preference.
///
/// The interpreter path is only used for debug messages.
fn source_satisfies_environment_preference(
source: PythonSource,
interpreter_path: &Path,
preference: EnvironmentPreference,
) -> bool {
match preference {
EnvironmentPreference::Any => true,
EnvironmentPreference::OnlyVirtual => {
if source.is_maybe_virtualenv() {
true
} else {
debug!(
"Ignoring Python interpreter at `{}`: only virtual environments allowed",
interpreter_path.display()
);
false
}
}
EnvironmentPreference::ExplicitSystem => {
if source.is_maybe_virtualenv() {
true
} else {
debug!(
"Ignoring Python interpreter at `{}`: system interpreter not explicitly requested",
interpreter_path.display()
);
false
}
}
EnvironmentPreference::OnlySystem => {
if source.is_maybe_system() {
true
} else {
debug!(
"Ignoring Python interpreter at `{}`: system interpreter required",
interpreter_path.display()
);
false
}
}
}
}

/// Utility for applying [`VersionRequest::matches_interpreter`] to a result type.
fn result_satisfies_version_request(
result: &Result<(PythonSource, Interpreter), Error>,
Expand All @@ -705,7 +754,7 @@ fn result_satisfies_environment_preference(
preference: EnvironmentPreference,
) -> bool {
result.as_ref().ok().map_or(true, |(source, interpreter)| {
satisfies_environment_preference(*source, interpreter, preference)
interpreter_satisfies_environment_preference(*source, interpreter, preference)
})
}

Expand Down Expand Up @@ -1575,6 +1624,40 @@ impl PythonSource {
| Self::DiscoveredEnvironment => true,
}
}

/// Whether this source **could** be a virtual environment.
///
/// This excludes the [`PythonSource::SearchPath`] although it could be in a virtual
/// environment; pragmatically, that's not common and saves us from querying a bunch of system
/// interpreters for no reason. It seems dubious to consider an interpreter in the `PATH` as a
/// target virtual environment if it's not discovered through our virtual environment-specific
/// patterns.
pub(crate) fn is_maybe_virtualenv(self) -> bool {
match self {
Self::ProvidedPath
| Self::ActiveEnvironment
| Self::DiscoveredEnvironment
| Self::CondaPrefix
| Self::BaseCondaPrefix
| Self::ParentInterpreter => true,
Self::Managed | Self::SearchPath | Self::Registry | Self::MicrosoftStore => false,
}
}

/// Whether this source **could** be a system interpreter.
pub(crate) fn is_maybe_system(self) -> bool {
match self {
Self::CondaPrefix
| Self::BaseCondaPrefix
| Self::ParentInterpreter
| Self::ProvidedPath
| Self::Managed
| Self::SearchPath
| Self::Registry
| Self::MicrosoftStore => true,
Self::ActiveEnvironment | Self::DiscoveredEnvironment => false,
}
}
}

impl PythonPreference {
Expand Down
6 changes: 1 addition & 5 deletions crates/uv/tests/it/pip_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7542,11 +7542,7 @@ fn install_incompatible_python_version_interpreter_broken_in_path() -> Result<()
----- stdout -----
----- stderr -----
error: Failed to inspect Python interpreter from search path at `/Users/zb/.local/share/uv/tests/.tmpDRf1pq/bin/python3`
Caused by: Querying Python at `/Users/zb/.local/share/uv/tests/.tmpDRf1pq/bin/python3` failed with exit status exit status: 1
[stderr]
error: intentionally broken python executable
error: No virtual environment found for Python 3.12; run `uv venv` to create an environment, or pass `--system` to install into a non-virtual environment
"###
);

Expand Down

0 comments on commit 4bf6a6b

Please sign in to comment.