Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Treat the base Conda environment as a system environment #7691

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 32 additions & 16 deletions crates/uv-python/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::microsoft_store::find_microsoft_store_pythons;
#[cfg(windows)]
use crate::py_launcher::{registry_pythons, WindowsPython};
use crate::virtualenv::{
conda_prefix_from_env, virtualenv_from_env, virtualenv_from_working_dir,
conda_environment_from_env, virtualenv_from_env, virtualenv_from_working_dir,
virtualenv_python_executable,
};
use crate::which::is_executable;
Expand Down Expand Up @@ -171,6 +171,8 @@ pub enum PythonSource {
ActiveEnvironment,
/// A conda environment was active e.g. via `CONDA_PREFIX`
CondaPrefix,
/// A base conda environment was active e.g. via `CONDA_PREFIX`
BaseCondaPrefix,
/// An environment was discovered e.g. via `.venv`
DiscoveredEnvironment,
/// An executable was found in the search path i.e. `PATH`
Expand Down Expand Up @@ -215,27 +217,27 @@ pub enum Error {
SourceNotAllowed(PythonRequest, PythonSource, PythonPreference),
}

/// Lazily iterate over Python executables in mutable environments.
/// Lazily iterate over Python executables in mutable virtual environments.
///
/// The following sources are supported:
///
/// - Active virtual environment (via `VIRTUAL_ENV`)
/// - Active conda environment (via `CONDA_PREFIX`)
/// - Discovered virtual environment (e.g. `.venv` in a parent directory)
///
/// Notably, "system" environments are excluded. See [`python_executables_from_installed`].
fn python_executables_from_environments<'a>(
fn python_executables_from_virtual_environments<'a>(
) -> impl Iterator<Item = Result<(PythonSource, PathBuf), Error>> + 'a {
let from_virtual_environment = std::iter::once_with(|| {
let from_active_environment = std::iter::once_with(|| {
virtualenv_from_env()
.into_iter()
.map(virtualenv_python_executable)
.map(|path| Ok((PythonSource::ActiveEnvironment, path)))
})
.flatten();

// N.B. we prefer the conda environment over discovered virtual environments
let from_conda_environment = std::iter::once_with(|| {
conda_prefix_from_env()
conda_environment_from_env(false)
.into_iter()
.map(virtualenv_python_executable)
.map(|path| Ok((PythonSource::CondaPrefix, path)))
Expand All @@ -253,7 +255,7 @@ fn python_executables_from_environments<'a>(
})
.flatten_ok();

from_virtual_environment
from_active_environment
.chain(from_conda_environment)
.chain(from_discovered_environment)
}
Expand Down Expand Up @@ -396,23 +398,35 @@ fn python_executables<'a>(
})
.flatten();

let from_environments = python_executables_from_environments();
// Check if the the base conda environment is active
let from_base_conda_environment = std::iter::once_with(|| {
conda_environment_from_env(true)
.into_iter()
.map(virtualenv_python_executable)
.map(|path| Ok((PythonSource::BaseCondaPrefix, path)))
})
.flatten();

let from_virtual_environments = python_executables_from_virtual_environments();
let from_installed = python_executables_from_installed(version, implementation, preference);

// 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_environments))
Box::new(from_parent_interpreter.chain(from_virtual_environments))
}
EnvironmentPreference::ExplicitSystem | EnvironmentPreference::Any => Box::new(
from_parent_interpreter
.chain(from_environments)
.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),
),
EnvironmentPreference::OnlySystem => {
Box::new(from_parent_interpreter.chain(from_installed))
}
}
}

Expand Down Expand Up @@ -607,8 +621,8 @@ fn satisfies_environment_preference(
) -> bool {
match (
preference,
// Conda environments are not conformant virtual environments but we treat them as such
interpreter.is_virtualenv() || matches!(source, PythonSource::CondaPrefix),
// Conda environments are not conformant virtual environments but we treat them as such.
interpreter.is_virtualenv() || (matches!(source, PythonSource::CondaPrefix)),
) {
(EnvironmentPreference::Any, _) => true,
(EnvironmentPreference::OnlyVirtual, true) => true,
Expand Down Expand Up @@ -1458,6 +1472,7 @@ impl PythonSource {
Self::Managed | Self::Registry | Self::MicrosoftStore => false,
Self::SearchPath
| Self::CondaPrefix
| Self::BaseCondaPrefix
| Self::ProvidedPath
| Self::ParentInterpreter
| Self::ActiveEnvironment
Expand All @@ -1470,6 +1485,7 @@ impl PythonSource {
match self {
Self::Managed | Self::Registry | Self::SearchPath | Self::MicrosoftStore => false,
Self::CondaPrefix
| Self::BaseCondaPrefix
| Self::ProvidedPath
| Self::ParentInterpreter
| Self::ActiveEnvironment
Expand Down Expand Up @@ -2076,7 +2092,7 @@ impl fmt::Display for PythonSource {
match self {
Self::ProvidedPath => f.write_str("provided path"),
Self::ActiveEnvironment => f.write_str("active virtual environment"),
Self::CondaPrefix => f.write_str("conda prefix"),
Self::CondaPrefix | Self::BaseCondaPrefix => f.write_str("conda prefix"),
Self::DiscoveredEnvironment => f.write_str("virtual environment"),
Self::SearchPath => f.write_str("search path"),
Self::Registry => f.write_str("registry"),
Expand Down
44 changes: 44 additions & 0 deletions crates/uv-python/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1037,12 +1037,56 @@ mod tests {
&context.cache,
)
})??;

assert_eq!(
python.interpreter().python_full_version().to_string(),
"3.12.0",
"We should allow the active conda python"
);

// But not if it's a base environment
let result = context.run_with_vars(
&[
("CONDA_PREFIX", Some(condaenv.as_os_str())),
("CONDA_DEFAULT_ENV", Some(&OsString::from("base"))),
],
|| {
find_python_installation(
&PythonRequest::Default,
EnvironmentPreference::OnlyVirtual,
PythonPreference::OnlySystem,
&context.cache,
)
},
)?;

assert!(
matches!(result, Err(PythonNotFound { .. })),
"We should not allow the non-virtual environment; got {result:?}"
);

// Unless, system interpreters are included...
let python = context.run_with_vars(
&[
("CONDA_PREFIX", Some(condaenv.as_os_str())),
("CONDA_DEFAULT_ENV", Some(&OsString::from("base"))),
],
|| {
find_python_installation(
&PythonRequest::Default,
EnvironmentPreference::OnlySystem,
PythonPreference::OnlySystem,
&context.cache,
)
},
)??;

assert_eq!(
python.interpreter().python_full_version().to_string(),
"3.12.0",
"We should find the base conda environment"
);

Ok(())
}

Expand Down
37 changes: 32 additions & 5 deletions crates/uv-python/src/virtualenv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,40 @@ pub(crate) fn virtualenv_from_env() -> Option<PathBuf> {

/// Locate an active conda environment by inspecting environment variables.
///
/// Supports `CONDA_PREFIX`.
pub(crate) fn conda_prefix_from_env() -> Option<PathBuf> {
if let Some(dir) = env::var_os("CONDA_PREFIX").filter(|value| !value.is_empty()) {
return Some(PathBuf::from(dir));
/// If `base` is true, the active environment must be the base environment or `None` is returned,
/// and vice-versa.
pub(crate) fn conda_environment_from_env(base: bool) -> Option<PathBuf> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might suggest making this an enum rather than a bool so that it's more self-documenting.

let dir = env::var_os("CONDA_PREFIX").filter(|value| !value.is_empty())?;
let path = PathBuf::from(dir);

if base != is_conda_base_env(&path) {
return None;
};

Some(path)
}

/// Whether the given `CONDA_PREFIX` path is the base Conda environment.
///
/// When the base environment is used, `CONDA_DEFAULT_ENV` will be set to a name, i.e., `base` or
/// `root` which does not match the prefix, e.g. `/usr/local` instead of
/// `/usr/local/conda/envs/<name>`.
fn is_conda_base_env(path: &Path) -> bool {
// If we cannot read `CONDA_DEFAULT_ENV`, there's no way to know if the base environment
let Ok(default_env) = env::var("CONDA_DEFAULT_ENV") else {
return false;
};

// These are the expected names for the base environment
if default_env != "base" && default_env != "root" {
return false;
}

None
let Some(name) = path.file_name() else {
return false;
};

name.to_string_lossy() != default_env
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can probably do name.to_str().and_then(...), since if it's not UTF-8, it can't match base or root.

}

/// Locate a virtual environment by searching the file system.
Expand Down
Loading