From dad512818f00fc1a1cc82b1c0ce47c7ebb512c4d Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Wed, 22 May 2024 15:27:32 -0500 Subject: [PATCH 1/2] Do not treat interpereters discovered via `CONDA_PREFIX` as system interpreters --- crates/uv-interpreter/src/discovery.rs | 32 +++++++++++++++++++------ crates/uv-interpreter/src/virtualenv.rs | 9 ++++++- 2 files changed, 33 insertions(+), 8 deletions(-) diff --git a/crates/uv-interpreter/src/discovery.rs b/crates/uv-interpreter/src/discovery.rs index 05c9b16f8303..e5f022176a95 100644 --- a/crates/uv-interpreter/src/discovery.rs +++ b/crates/uv-interpreter/src/discovery.rs @@ -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; @@ -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` @@ -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() @@ -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( @@ -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( @@ -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 to read python installations from the registry instead. .chain( (sources.contains(InterpreterSource::PyLauncher) && cfg!(windows)).then(|| @@ -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!( @@ -1100,6 +1116,7 @@ impl SourceSelector { Self::VirtualEnv => [ InterpreterSource::DiscoveredEnvironment, InterpreterSource::ActiveEnvironment, + InterpreterSource::CondaPrefix, ] .contains(&source), Self::Custom(sources) => sources.contains(&source), @@ -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"), diff --git a/crates/uv-interpreter/src/virtualenv.rs b/crates/uv-interpreter/src/virtualenv.rs index f34bb0b09259..8a0824c3c363 100644 --- a/crates/uv-interpreter/src/virtualenv.rs +++ b/crates/uv-interpreter/src/virtualenv.rs @@ -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 { if let Some(dir) = env::var_os("VIRTUAL_ENV").filter(|value| !value.is_empty()) { info!( @@ -53,6 +53,13 @@ pub(crate) fn virtualenv_from_env() -> Option { 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 { if let Some(dir) = env::var_os("CONDA_PREFIX").filter(|value| !value.is_empty()) { info!( "Found active virtual environment (via CONDA_PREFIX) at: {}", From c83ba47b24cb19cacbb25205d14baae9ce67e0cc Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Wed, 22 May 2024 15:39:33 -0500 Subject: [PATCH 2/2] Add conda prefix unit tests --- crates/uv-interpreter/src/environment.rs | 5 +- crates/uv-interpreter/src/lib.rs | 91 ++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/crates/uv-interpreter/src/environment.rs b/crates/uv-interpreter/src/environment.rs index 65abab11547b..95edfe4e52be 100644 --- a/crates/uv-interpreter/src/environment.rs +++ b/crates/uv-interpreter/src/environment.rs @@ -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 { 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() diff --git a/crates/uv-interpreter/src/lib.rs b/crates/uv-interpreter/src/lib.rs index 51397ddd1fad..7325b4996b70 100644 --- a/crates/uv-interpreter/src/lib.rs +++ b/crates/uv-interpreter/src/lib.rs @@ -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 { + 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()?; @@ -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()?;