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`
  • Loading branch information
zanieb committed Feb 4, 2025
1 parent d9907f6 commit b8e1816
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 b8e1816

Please sign in to comment.