Skip to content

Commit

Permalink
Allow discovering virtual environments from the first interpreter fou…
Browse files Browse the repository at this point in the history
…nd on the `PATH` (#11218)

Closes #11214

Special-cases the first Python executable we find on the `PATH`,
allowing it to be considered during searches for virtual environments.

For some context, there are two stages to Python interpreter discovery

1. We find possible Python executables in various sources
2. We query the executables to determine canonical metadata about the
interpreter

We can't really be "sure" if an executable is a complaint virtual
environment during (1), we need to query the interpreter first. This
means that if you're only allowed to installed into virtual
environments, we'll query every interpreter on your PATH. This is not
performant, and causes confusion for users. Notably, I recently improved
error messaging when we can't find any valid interpreters, by showing
the error message we encounter while querying an interpreter (if any).
However, this is problematic when there's an error for an interpreter
that is not relevant to your search. In
#11143, I added filtering to avoid
querying additional interpreters, but that regressed some user
experiences where they were relying on us finding implicitly active
virtual environments via the PATH.
  • Loading branch information
zanieb authored Feb 4, 2025
1 parent 04374b0 commit ec480bd
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 11 deletions.
37 changes: 32 additions & 5 deletions crates/uv-python/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ pub enum PythonSource {
DiscoveredEnvironment,
/// An executable was found in the search path i.e. `PATH`
SearchPath,
/// The first executable found in the search path i.e. `PATH`
SearchPathFirst,
/// An executable was found in the Windows registry via PEP 514
Registry,
/// An executable was found in the known Microsoft Store locations
Expand Down Expand Up @@ -331,7 +333,14 @@ fn python_executables_from_installed<'a>(

let from_search_path = iter::once_with(move || {
python_executables_from_search_path(version, implementation)
.map(|path| Ok((PythonSource::SearchPath, path)))
.enumerate()
.map(|(i, path)| {
if i == 0 {
Ok((PythonSource::SearchPathFirst, path))
} else {
Ok((PythonSource::SearchPath, path))
}
})
})
.flatten();

Expand Down Expand Up @@ -1049,7 +1058,10 @@ pub(crate) fn find_python_installation(
// 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;
&& matches!(
installation.source,
PythonSource::SearchPath | PythonSource::SearchPathFirst
);

// 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.
Expand Down Expand Up @@ -1601,6 +1613,7 @@ impl PythonSource {
match self {
Self::Managed | Self::Registry | Self::MicrosoftStore => false,
Self::SearchPath
| Self::SearchPathFirst
| Self::CondaPrefix
| Self::BaseCondaPrefix
| Self::ProvidedPath
Expand All @@ -1613,7 +1626,13 @@ impl PythonSource {
/// 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::Managed
| Self::Registry
| Self::SearchPath
// TODO(zanieb): We may want to allow this at some point, but when adding this variant
// we want compatibility with existing behavior
| Self::SearchPathFirst
| Self::MicrosoftStore => false,
Self::CondaPrefix
| Self::BaseCondaPrefix
| Self::ProvidedPath
Expand All @@ -1629,15 +1648,20 @@ impl PythonSource {
/// 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.
/// patterns. Instead, we special case the first Python executable found on the `PATH` with
/// [`PythonSource::SearchPathFirst`], allowing us to check if that's a virtual environment.
/// This enables targeting the virtual environment with uv by putting its `bin/` on the `PATH`
/// without setting `VIRTUAL_ENV` — but if there's another interpreter before it we will ignore
/// it.
pub(crate) fn is_maybe_virtualenv(self) -> bool {
match self {
Self::ProvidedPath
| Self::ActiveEnvironment
| Self::DiscoveredEnvironment
| Self::CondaPrefix
| Self::BaseCondaPrefix
| Self::ParentInterpreter => true,
| Self::ParentInterpreter
| Self::SearchPathFirst => true,
Self::Managed | Self::SearchPath | Self::Registry | Self::MicrosoftStore => false,
}
}
Expand All @@ -1651,6 +1675,7 @@ impl PythonSource {
| Self::ProvidedPath
| Self::Managed
| Self::SearchPath
| Self::SearchPathFirst
| Self::Registry
| Self::MicrosoftStore => true,
Self::ActiveEnvironment | Self::DiscoveredEnvironment => false,
Expand Down Expand Up @@ -2062,6 +2087,7 @@ impl VersionRequest {
| PythonSource::DiscoveredEnvironment
| PythonSource::ActiveEnvironment => Self::Any,
PythonSource::SearchPath
| PythonSource::SearchPathFirst
| PythonSource::Registry
| PythonSource::MicrosoftStore
| PythonSource::Managed => Self::Default,
Expand Down Expand Up @@ -2473,6 +2499,7 @@ impl fmt::Display for PythonSource {
Self::CondaPrefix | Self::BaseCondaPrefix => f.write_str("conda prefix"),
Self::DiscoveredEnvironment => f.write_str("virtual environment"),
Self::SearchPath => f.write_str("search path"),
Self::SearchPathFirst => f.write_str("first executable in the search path"),
Self::Registry => f.write_str("registry"),
Self::MicrosoftStore => f.write_str("Microsoft Store"),
Self::Managed => f.write_str("managed installations"),
Expand Down
8 changes: 4 additions & 4 deletions crates/uv-python/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ mod tests {
matches!(
interpreter,
PythonInstallation {
source: PythonSource::SearchPath,
source: PythonSource::SearchPathFirst,
interpreter: _
}
),
Expand Down Expand Up @@ -936,7 +936,7 @@ mod tests {
matches!(
python,
PythonInstallation {
source: PythonSource::SearchPath,
source: PythonSource::SearchPathFirst,
interpreter: _
}
),
Expand Down Expand Up @@ -2427,7 +2427,7 @@ mod tests {
matches!(
python,
PythonInstallation {
source: PythonSource::SearchPath,
source: PythonSource::SearchPathFirst,
interpreter: _
}
),
Expand Down Expand Up @@ -2479,7 +2479,7 @@ mod tests {
matches!(
python,
PythonInstallation {
source: PythonSource::SearchPath,
source: PythonSource::SearchPathFirst,
interpreter: _
}
),
Expand Down
40 changes: 38 additions & 2 deletions crates/uv/tests/it/pip_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7608,12 +7608,48 @@ fn install_incompatible_python_version_interpreter_broken_in_path() -> Result<()
perms.set_mode(0o755);
fs_err::set_permissions(&python, perms)?;

// Request Python 3.12; which should fail
// Put the broken interpreter _before_ the other interpreters in the PATH
let path = std::env::join_paths(
std::iter::once(context.bin_dir.to_path_buf())
.chain(std::env::split_paths(&context.python_path())),
)
.unwrap();

// Request Python 3.12, which should fail since the virtual environment does not have a matching
// version.
// Since the broken interpreter is at the front of the PATH, this query error should be raised
uv_snapshot!(context.filters(), context.pip_install()
.arg("-p").arg("3.12")
.arg("anyio")
// In tests, we ignore `PATH` during Python discovery so we need to add the context `bin`
.env("UV_TEST_PYTHON_PATH", path.as_os_str()), @r###"
success: false
exit_code: 2
----- stdout -----
----- stderr -----
error: Failed to inspect Python interpreter from first executable in the search path at `[BIN]/python3`
Caused by: Querying Python at `[BIN]/python3` failed with exit status exit status: 1
[stderr]
error: intentionally broken python executable
"###
);

// Put the broken interpreter _after_ the other interpreters in the PATH
let path = std::env::join_paths(
std::env::split_paths(&context.python_path())
.chain(std::iter::once(context.bin_dir.to_path_buf())),
)
.unwrap();

// Since the broken interpreter is not at the front of the PATH, the query error should not be
// raised
uv_snapshot!(context.filters(), context.pip_install()
.arg("-p").arg("3.12")
.arg("anyio")
// In tests, we ignore `PATH` during Python discovery so we need to add the context `bin`
.env("UV_TEST_PYTHON_PATH", context.bin_dir.as_os_str()), @r###"
.env("UV_TEST_PYTHON_PATH", path.as_os_str()), @r###"
success: false
exit_code: 2
----- stdout -----
Expand Down
50 changes: 50 additions & 0 deletions crates/uv/tests/it/python_find.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,56 @@ fn python_find_venv() {
----- stderr -----
"###);

// Or at the front of the PATH
#[cfg(not(windows))]
uv_snapshot!(context.filters(), context.python_find().env(EnvVars::UV_TEST_PYTHON_PATH, child_dir.join(".venv").join("bin").as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
[TEMP_DIR]/child/.venv/[BIN]/python
----- stderr -----
"###);

// This holds even if there are other directories before it in the path, as long as they do
// not contain a Python executable
#[cfg(not(windows))]
{
let path = std::env::join_paths(&[
context.temp_dir.to_path_buf(),
child_dir.join(".venv").join("bin"),
])
.unwrap();

uv_snapshot!(context.filters(), context.python_find().env(EnvVars::UV_TEST_PYTHON_PATH, path.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
[TEMP_DIR]/child/.venv/[BIN]/python
----- stderr -----
"###);
}

// But, if there's an executable _before_ the virtual environment — we prefer that
#[cfg(not(windows))]
{
let path = std::env::join_paths(
std::env::split_paths(&context.python_path())
.chain(std::iter::once(child_dir.join(".venv").join("bin"))),
)
.unwrap();

uv_snapshot!(context.filters(), context.python_find().env(EnvVars::UV_TEST_PYTHON_PATH, path.as_os_str()), @r###"
success: true
exit_code: 0
----- stdout -----
[PYTHON-3.11]
----- stderr -----
"###);
}
}

#[cfg(unix)]
Expand Down

0 comments on commit ec480bd

Please sign in to comment.