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

Do not treat interpereters discovered via CONDA_PREFIX as system interpreters #3771

Merged
merged 2 commits into from
May 22, 2024
Merged
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
32 changes: 25 additions & 7 deletions crates/uv-interpreter/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ use crate::interpreter::Error as InterpreterError;
use crate::managed::toolchains_for_current_platform;
use crate::py_launcher::py_list_paths;
use crate::virtualenv::{
virtualenv_from_env, virtualenv_from_working_dir, virtualenv_python_executable,
conda_prefix_from_env, virtualenv_from_env, virtualenv_from_working_dir,
virtualenv_python_executable,
};
use crate::{Interpreter, PythonVersion};
use std::borrow::Cow;
Expand Down Expand Up @@ -125,6 +126,8 @@ pub enum InterpreterSource {
ProvidedPath,
/// An environment was active e.g. via `VIRTUAL_ENV`
ActiveEnvironment,
/// A conda environment was active e.g. via `CONDA_PREFIX`
CondaPrefix,
/// An environment was discovered e.g. via `.venv`
DiscoveredEnvironment,
/// An executable was found in the search path i.e. `PATH`
Expand Down Expand Up @@ -195,7 +198,7 @@ fn python_executables<'a>(
.into_iter()
.map(|path| Ok((InterpreterSource::ParentInterpreter, PathBuf::from(path))))
).into_iter().flatten()
// (2) The active environment
// (2) An active virtual environment
.chain(
sources.contains(InterpreterSource::ActiveEnvironment).then(||
virtualenv_from_env()
Expand All @@ -204,7 +207,16 @@ fn python_executables<'a>(
.map(|path| Ok((InterpreterSource::ActiveEnvironment, path)))
).into_iter().flatten()
)
// (3) A discovered environment
// (3) An active conda environment
.chain(
sources.contains(InterpreterSource::CondaPrefix).then(||
conda_prefix_from_env()
.into_iter()
.map(virtualenv_python_executable)
.map(|path| Ok((InterpreterSource::CondaPrefix, path)))
).into_iter().flatten()
)
// (4) A discovered environment
.chain(
sources.contains(InterpreterSource::DiscoveredEnvironment).then(||
std::iter::once(
Expand All @@ -219,7 +231,7 @@ fn python_executables<'a>(
).flatten_ok()
).into_iter().flatten()
)
// (4) Managed toolchains
// (5) Managed toolchains
.chain(
sources.contains(InterpreterSource::ManagedToolchain).then(move ||
std::iter::once(
Expand All @@ -237,14 +249,14 @@ fn python_executables<'a>(
).flatten_ok()
).into_iter().flatten()
)
// (5) The search path
// (6) The search path
.chain(
sources.contains(InterpreterSource::SearchPath).then(move ||
python_executables_from_search_path(version, implementation)
.map(|path| Ok((InterpreterSource::SearchPath, path))),
).into_iter().flatten()
)
// (6) The `py` launcher (windows only)
// (7) The `py` launcher (windows only)
// TODO(konstin): Implement <https://peps.python.org/pep-0514/> to read python installations from the registry instead.
.chain(
(sources.contains(InterpreterSource::PyLauncher) && cfg!(windows)).then(||
Expand Down Expand Up @@ -362,7 +374,11 @@ fn python_interpreters<'a>(
})
.filter(move |result| match result {
// Filter the returned interpreters to conform to the system request
Ok((source, interpreter)) => match (system, interpreter.is_virtualenv()) {
Ok((source, interpreter)) => match (
system,
// Conda environments are not conformant virtual environments but we should not treat them as system interpreters
interpreter.is_virtualenv() || matches!(source, InterpreterSource::CondaPrefix),
) {
(SystemPython::Allowed, _) => true,
(SystemPython::Explicit, false) => {
if matches!(
Expand Down Expand Up @@ -1100,6 +1116,7 @@ impl SourceSelector {
Self::VirtualEnv => [
InterpreterSource::DiscoveredEnvironment,
InterpreterSource::ActiveEnvironment,
InterpreterSource::CondaPrefix,
]
.contains(&source),
Self::Custom(sources) => sources.contains(&source),
Expand Down Expand Up @@ -1163,6 +1180,7 @@ impl fmt::Display for InterpreterSource {
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::DiscoveredEnvironment => f.write_str("virtual environment"),
Self::SearchPath => f.write_str("search path"),
Self::PyLauncher => f.write_str("`py` launcher output"),
Expand Down
5 changes: 4 additions & 1 deletion crates/uv-interpreter/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,16 @@ impl PythonEnvironment {
}

/// Create a [`PythonEnvironment`] for an existing virtual environment.
///
/// Allows Conda environments (via `CONDA_PREFIX`) though they are not technically virtual environments.
pub fn from_virtualenv(cache: &Cache) -> Result<Self, Error> {
let sources = SourceSelector::VirtualEnv;
let request = InterpreterRequest::Version(VersionRequest::Default);
let found = find_interpreter(&request, SystemPython::Disallowed, &sources, cache)??;

debug_assert!(
found.interpreter().is_virtualenv(),
found.interpreter().is_virtualenv()
|| matches!(found.source(), InterpreterSource::CondaPrefix),
"Not a virtualenv (source: {}, prefix: {})",
found.source(),
found.interpreter().base_prefix().display()
Expand Down
91 changes: 91 additions & 0 deletions crates/uv-interpreter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,29 @@ mod tests {
Ok(venv.to_path_buf())
}

/// Create a mock conda prefix in the given directory.
///
/// These are like virtual environments but they look like system interpreters because `prefix` and `base_prefix` are equal.
///
/// Returns the path to the environment.
fn mock_conda_prefix(tempdir: &TempDir, version: &'static str) -> Result<PathBuf> {
let env = tempdir.child("conda");
let executable = virtualenv_python_executable(&env);
fs_err::create_dir_all(
executable
.parent()
.expect("A Python executable path should always have a parent"),
)?;
create_mock_interpreter(
&executable,
&PythonVersion::from_str(version).expect("A valid Python version is used for tests"),
ImplementationName::default(),
true,
)?;
env.child("pyvenv.cfg").touch()?;
Ok(env.to_path_buf())
}

#[test]
fn find_default_interpreter_empty_path() -> Result<()> {
let cache = Cache::temp()?;
Expand Down Expand Up @@ -1308,6 +1331,74 @@ mod tests {
Ok(())
}

#[test]
fn find_environment_from_conda_prefix() -> Result<()> {
let tempdir = TempDir::new()?;
let cache = Cache::temp()?;
let conda_prefix = mock_conda_prefix(&tempdir, "3.12.0")?;

with_vars(
[
("UV_TEST_PYTHON_PATH", None),
("UV_BOOTSTRAP_DIR", None),
(
"PATH",
Some(simple_mock_interpreters(&tempdir, &["3.10.1", "3.11.2"])?),
),
("CONDA_PREFIX", Some(conda_prefix.into())),
("PWD", Some(tempdir.path().into())),
],
|| {
let environment =
// Note this environment is not treated as a system interpreter
PythonEnvironment::find(None, SystemPython::Disallowed, &cache)
.expect("An environment is found");
assert_eq!(
environment.interpreter().python_full_version().to_string(),
"3.12.0",
"We should allow the conda environment"
);
},
);

Ok(())
}

#[test]
fn find_environment_from_conda_prefix_and_virtualenv() -> Result<()> {
let tempdir = TempDir::new()?;
let cache = Cache::temp()?;
let generic = mock_venv(&tempdir, "3.12.0")?;
let conda = mock_conda_prefix(&tempdir, "3.12.1")?;

with_vars(
[
("UV_TEST_PYTHON_PATH", None),
("UV_BOOTSTRAP_DIR", None),
(
"PATH",
Some(simple_mock_interpreters(&tempdir, &["3.10.2", "3.11.3"])?),
),
("CONDA_PREFIX", Some(conda.into())),
("VIRTUAL_ENV", Some(generic.into())),
("PWD", Some(tempdir.path().into())),
],
|| {
let environment =
// Note this environment is not treated as a system interpreter
PythonEnvironment::find(None, SystemPython::Disallowed, &cache)
.expect("An environment is found");
assert_eq!(
environment.interpreter().python_full_version().to_string(),
"3.12.0",
"We should prefer the active environment"
);
},
);

Ok(())
}

#[test]
fn find_environment_from_discovered_environment() -> Result<()> {
let tempdir = TempDir::new()?;
Expand Down
9 changes: 8 additions & 1 deletion crates/uv-interpreter/src/virtualenv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub enum Error {

/// Locate an active virtual environment by inspecting environment variables.
///
/// Supports `VIRTUAL_ENV` and `CONDA_PREFIX`.
/// Supports `VIRTUAL_ENV`.
pub(crate) fn virtualenv_from_env() -> Option<PathBuf> {
if let Some(dir) = env::var_os("VIRTUAL_ENV").filter(|value| !value.is_empty()) {
info!(
Expand All @@ -53,6 +53,13 @@ pub(crate) fn virtualenv_from_env() -> Option<PathBuf> {
return Some(PathBuf::from(dir));
}

None
}

/// 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()) {
info!(
"Found active virtual environment (via CONDA_PREFIX) at: {}",
Expand Down
Loading