Filter discovered Python interpreters by system preference (#3739)

Previously, we enforced `SystemPython` outside of the interpreter
discovery exclusively with source selection. Now, we perform additional
filtering of interpreters depending on if they are a virtual
environment. This should not change any existing behavior, but will make
it much easier to have consistent behavior in ambiguous cases like
https://github.com/astral-sh/uv/pull/3736#discussion_r1610072262 where a
source could provide either a system interpreter or virtual environment
interpreter.

crates/uv-interpreter/src/discovery.rs

@@ -322,24 +322,50 @@ fn python_executables_from_search_path<'a>(
 fn python_interpreters<'a>(
     version: Option<&'a VersionRequest>,
     implementation: Option<&'a ImplementationName>,
+    system: SystemPython,
     sources: &SourceSelector,
     cache: &'a Cache,
 ) -> impl Iterator<Item = Result<(InterpreterSource, Interpreter), Error>> + 'a {
-    python_executables(version, implementation, sources).map(|result| match result {
+    python_executables(version, implementation, sources)
-        Ok((source, path)) => Interpreter::query(&path, cache)
+        .map(|result| match result {
-            .map(|interpreter| (source, interpreter))
+            Ok((source, path)) => Interpreter::query(&path, cache)
-            .inspect(|(source, interpreter)| {
+                .map(|interpreter| (source, interpreter))
-                trace!(
+                .inspect(|(source, interpreter)| {
-                    "Found Python interpreter {} {} at {} from {source}",
+                    trace!(
-                    interpreter.implementation_name(),
+                        "Found Python interpreter {} {} at {} from {source}",
-                    interpreter.python_full_version(),
+                        interpreter.implementation_name(),
-                    path.display()
+                        interpreter.python_full_version(),
-                );
+                        path.display()
-            })
+                    );
-            .map_err(Error::from)
+                })
-            .inspect_err(|err| trace!("{err}")),
+                .map_err(Error::from)
-        Err(err) => Err(err),
+                .inspect_err(|err| trace!("{err}")),
-    })
+            Err(err) => Err(err),
+        })
+        .filter(move |result| match result {
+            // Filter the returned interpreters to conform to the system request
+            Ok((_, interpreter)) => match (system, interpreter.is_virtualenv()) {
+                (SystemPython::Allowed, _) => true,
+                (SystemPython::Disallowed, false) => {
+                    debug!(
+                        "Ignoring Python interpreter at `{}`: system intepreter not allowed",
+                        interpreter.sys_executable().display()
+                    );
+                    false
+                }
+                (SystemPython::Disallowed, true) => true,
+                (SystemPython::Required, true) => {
+                    debug!(
+                        "Ignoring Python interpreter at `{}`: system intepreter required",
+                        interpreter.sys_executable().display()
+                    );
+                    false
+                }
+                (SystemPython::Required, false) => true,
+            },
+            // Do not drop any errors
+            Err(_) => true,
+        })
 }
 
 /// Check if an encountered error should stop discovery.
@@ -370,6 +396,7 @@ fn should_stop_discovery(err: &Error) -> bool {
 /// the error will raised instead of attempting further candidates.
 pub fn find_interpreter(
     request: &InterpreterRequest,
+    system: SystemPython,
     sources: &SourceSelector,
     cache: &Cache,
 ) -> Result<InterpreterResult, Error> {
@@ -433,7 +460,7 @@ pub fn find_interpreter(
         }
         InterpreterRequest::Implementation(implementation) => {
             let Some((source, interpreter)) =
-                python_interpreters(None, Some(implementation), sources, cache)
+                python_interpreters(None, Some(implementation), system, sources, cache)
                     .find(|result| {
                         match result {
                             // Return the first critical error or matching interpreter
@@ -456,7 +483,7 @@ pub fn find_interpreter(
         }
         InterpreterRequest::ImplementationVersion(implementation, version) => {
             let Some((source, interpreter)) =
-                python_interpreters(Some(version), Some(implementation), sources, cache)
+                python_interpreters(Some(version), Some(implementation), system, sources, cache)
                     .find(|result| {
                         match result {
                             // Return the first critical error or matching interpreter
@@ -486,7 +513,7 @@ pub fn find_interpreter(
         }
         InterpreterRequest::Version(version) => {
             let Some((source, interpreter)) =
-                python_interpreters(Some(version), None, sources, cache)
+                python_interpreters(Some(version), None, system, sources, cache)
                     .find(|result| {
                         match result {
                             // Return the first critical error or matching interpreter
@@ -526,7 +553,7 @@ pub fn find_default_interpreter(cache: &Cache) -> Result<InterpreterResult, Erro
         InterpreterSource::PyLauncher,
     ]);
 
-    let result = find_interpreter(&request, &sources, cache)?;
+    let result = find_interpreter(&request, SystemPython::Required, &sources, cache)?;
     if let Ok(ref found) = result {
         warn_on_unsupported_python(found.interpreter());
     }
@@ -557,7 +584,7 @@ pub fn find_best_interpreter(
 
     // First, check for an exact match (or the first available version if no Python versfion was provided)
     debug!("Looking for exact match for request {request}");
-    let result = find_interpreter(request, &sources, cache)?;
+    let result = find_interpreter(request, system, &sources, cache)?;
     if let Ok(ref found) = result {
         warn_on_unsupported_python(found.interpreter());
         return Ok(result);
@@ -579,7 +606,7 @@ pub fn find_best_interpreter(
         _ => None,
     } {
         debug!("Looking for relaxed patch version {request}");
-        let result = find_interpreter(&request, &sources, cache)?;
+        let result = find_interpreter(&request, system, &sources, cache)?;
         if let Ok(ref found) = result {
             warn_on_unsupported_python(found.interpreter());
             return Ok(result);
@@ -591,7 +618,7 @@ pub fn find_best_interpreter(
     let request = InterpreterRequest::Version(VersionRequest::Default);
     Ok(find_interpreter(
         // TODO(zanieb): Add a dedicated `Default` variant to `InterpreterRequest`
-        &request, &sources, cache,
+        &request, system, &sources, cache,
     )?
     .map_err(|err| {
         // Use a more general error in this case since we looked for multiple versions

crates/uv-interpreter/src/environment.rs

@@ -43,7 +43,7 @@ impl PythonEnvironment {
     pub fn from_virtualenv(cache: &Cache) -> Result<Self, Error> {
         let sources = SourceSelector::virtualenvs();
         let request = InterpreterRequest::Version(VersionRequest::Default);
-        let found = find_interpreter(&request, &sources, cache)??;
+        let found = find_interpreter(&request, SystemPython::Disallowed, &sources, cache)??;
 
         debug_assert!(
             found.interpreter().base_prefix() == found.interpreter().base_exec_prefix(),
@@ -86,7 +86,7 @@ impl PythonEnvironment {
     ) -> Result<Self, Error> {
         let sources = SourceSelector::from_env(system);
         let request = InterpreterRequest::parse(request);
-        let interpreter = find_interpreter(&request, &sources, cache)??.into_interpreter();
+        let interpreter = find_interpreter(&request, system, &sources, cache)??.into_interpreter();
         Ok(Self(Arc::new(PythonEnvironmentShared {
             root: interpreter.prefix().to_path_buf(),
             interpreter,

crates/uv-interpreter/src/lib.rs

@@ -80,7 +80,7 @@ mod tests {
         implementation::ImplementationName,
         virtualenv::virtualenv_python_executable,
         Error, InterpreterNotFound, InterpreterSource, PythonEnvironment, PythonVersion,
-        SourceSelector,
+        SourceSelector, SystemPython,
     };
 
     /// Create a fake Python interpreter executable which returns fixed metadata mocking our interpreter
@@ -89,6 +89,7 @@ mod tests {
         path: &Path,
         version: &PythonVersion,
         implementation: ImplementationName,
+        system: bool,
     ) -> Result<()> {
         let json = indoc! {r##"
             {
@@ -116,7 +117,7 @@ mod tests {
                 },
                 "base_exec_prefix": "/home/ferris/.pyenv/versions/{FULL_VERSION}",
                 "base_prefix": "/home/ferris/.pyenv/versions/{FULL_VERSION}",
-                "prefix": "/home/ferris/projects/uv/.venv",
+                "prefix": "{PREFIX}",
                 "sys_executable": "{PATH}",
                 "sys_path": [
                     "/home/ferris/.pyenv/versions/{FULL_VERSION}/lib/python{VERSION}/lib/python{VERSION}",
@@ -140,11 +141,22 @@ mod tests {
                 "pointer_size": "64",
                 "gil_disabled": true
             }
-        "##}
+        "##};
-        .replace("{PATH}", path.to_str().expect("Path can be represented as string"))
+
-        .replace("{FULL_VERSION}", &version.to_string())
+        let json = if system {
-        .replace("{VERSION}", &version.without_patch().to_string())
+            json.replace("{PREFIX}", "/home/ferris/.pyenv/versions/{FULL_VERSION}")
-        .replace("{IMPLEMENTATION}", implementation.as_str());
+        } else {
+            json.replace("{PREFIX}", "/home/ferris/projects/uv/.venv")
+        };
+
+        let json = json
+            .replace(
+                "{PATH}",
+                path.to_str().expect("Path can be represented as string"),
+            )
+            .replace("{FULL_VERSION}", &version.to_string())
+            .replace("{VERSION}", &version.without_patch().to_string())
+            .replace("{IMPLEMENTATION}", implementation.as_str());
 
         fs_err::write(
             path,
@@ -199,7 +211,7 @@ mod tests {
     fn simple_mock_interpreters(tempdir: &TempDir, versions: &[&'static str]) -> Result<OsString> {
         let kinds: Vec<_> = versions
             .iter()
-            .map(|version| (ImplementationName::default(), "python", *version))
+            .map(|version| (true, ImplementationName::default(), "python", *version))
             .collect();
         mock_interpreters(tempdir, kinds.as_slice())
     }
@@ -209,18 +221,21 @@ mod tests {
     /// Returns a search path for the mock interpreters.
     fn mock_interpreters(
         tempdir: &TempDir,
-        kinds: &[(ImplementationName, &'static str, &'static str)],
+        kinds: &[(bool, ImplementationName, &'static str, &'static str)],
     ) -> Result<OsString> {
         let names: Vec<OsString> = (0..kinds.len())
             .map(|i| OsString::from(i.to_string()))
             .collect();
         let paths = create_children(tempdir, names.as_slice())?;
-        for (path, (implementation, executable, version)) in itertools::zip_eq(&paths, kinds) {
+        for (path, (system, implementation, executable, version)) in
+            itertools::zip_eq(&paths, kinds)
+        {
             let python = format!("{executable}{}", std::env::consts::EXE_SUFFIX);
             create_mock_interpreter(
                 &path.join(python),
                 &PythonVersion::from_str(version).unwrap(),
                 *implementation,
+                *system,
             )?;
         }
         Ok(env::join_paths(paths)?)
@@ -241,6 +256,7 @@ mod tests {
             &executable,
             &PythonVersion::from_str(version).expect("A valid Python version is used for tests"),
             ImplementationName::default(),
+            false,
         )?;
         venv.child("pyvenv.cfg").touch()?;
         Ok(venv.to_path_buf())
@@ -326,6 +342,7 @@ mod tests {
             &python,
             &PythonVersion::from_str("3.12.1").unwrap(),
             ImplementationName::default(),
+            true,
         )?;
 
         with_vars(
@@ -394,6 +411,7 @@ mod tests {
             &python,
             &PythonVersion::from_str("3.12.1").unwrap(),
             ImplementationName::default(),
+            true,
         )?;
 
         with_vars(
@@ -484,6 +502,7 @@ mod tests {
             &python3,
             &PythonVersion::from_str("3.12.1").unwrap(),
             ImplementationName::default(),
+            true,
         )?;
 
         with_vars(
@@ -521,6 +540,162 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn find_interpreter_system_python_allowed() -> Result<()> {
+        let tempdir = TempDir::new()?;
+        let cache = Cache::temp()?;
+
+        with_vars(
+            [
+                ("UV_TEST_PYTHON_PATH", None::<OsString>),
+                ("UV_BOOTSTRAP_DIR", None),
+                (
+                    "PATH",
+                    Some(mock_interpreters(
+                        &tempdir,
+                        &[
+                            (false, ImplementationName::CPython, "python", "3.10.0"),
+                            (true, ImplementationName::CPython, "python", "3.10.1"),
+                        ],
+                    )?),
+                ),
+                ("PWD", Some(tempdir.path().into())),
+            ],
+            || {
+                let result = find_interpreter(
+                    &InterpreterRequest::Version(VersionRequest::Default),
+                    SystemPython::Allowed,
+                    &SourceSelector::All,
+                    &cache,
+                )
+                .unwrap()
+                .unwrap();
+                assert_eq!(
+                    result.interpreter().python_full_version().to_string(),
+                    "3.10.0",
+                    "Should find the first interpreter regardless of system"
+                );
+            },
+        );
+
+        // Reverse the order of the virtual environment and system
+        with_vars(
+            [
+                ("UV_TEST_PYTHON_PATH", None::<OsString>),
+                ("UV_BOOTSTRAP_DIR", None),
+                (
+                    "PATH",
+                    Some(mock_interpreters(
+                        &tempdir,
+                        &[
+                            (true, ImplementationName::CPython, "python", "3.10.0"),
+                            (false, ImplementationName::CPython, "python", "3.10.1"),
+                        ],
+                    )?),
+                ),
+                ("PWD", Some(tempdir.path().into())),
+            ],
+            || {
+                let result = find_interpreter(
+                    &InterpreterRequest::Version(VersionRequest::Default),
+                    SystemPython::Allowed,
+                    &SourceSelector::All,
+                    &cache,
+                )
+                .unwrap()
+                .unwrap();
+                assert_eq!(
+                    result.interpreter().python_full_version().to_string(),
+                    "3.10.0",
+                    "Should find the first interpreter regardless of system"
+                );
+            },
+        );
+
+        Ok(())
+    }
+
+    #[test]
+    fn find_interpreter_system_python_required() -> Result<()> {
+        let tempdir = TempDir::new()?;
+        let cache = Cache::temp()?;
+
+        with_vars(
+            [
+                ("UV_TEST_PYTHON_PATH", None::<OsString>),
+                ("UV_BOOTSTRAP_DIR", None),
+                (
+                    "PATH",
+                    Some(mock_interpreters(
+                        &tempdir,
+                        &[
+                            (false, ImplementationName::CPython, "python", "3.10.0"),
+                            (true, ImplementationName::CPython, "python", "3.10.1"),
+                        ],
+                    )?),
+                ),
+                ("PWD", Some(tempdir.path().into())),
+            ],
+            || {
+                let result = find_interpreter(
+                    &InterpreterRequest::Version(VersionRequest::Default),
+                    SystemPython::Required,
+                    &SourceSelector::All,
+                    &cache,
+                )
+                .unwrap()
+                .unwrap();
+                assert_eq!(
+                    result.interpreter().python_full_version().to_string(),
+                    "3.10.1",
+                    "Should skip the virtual environment"
+                );
+            },
+        );
+
+        Ok(())
+    }
+
+    #[test]
+    fn find_interpreter_system_python_disallowed() -> Result<()> {
+        let tempdir = TempDir::new()?;
+        let cache = Cache::temp()?;
+
+        with_vars(
+            [
+                ("UV_TEST_PYTHON_PATH", None::<OsString>),
+                (
+                    "PATH",
+                    Some(mock_interpreters(
+                        &tempdir,
+                        &[
+                            (true, ImplementationName::CPython, "python", "3.10.0"),
+                            (false, ImplementationName::CPython, "python", "3.10.1"),
+                        ],
+                    )?),
+                ),
+                ("PWD", Some(tempdir.path().into())),
+            ],
+            || {
+                let result = find_interpreter(
+                    &InterpreterRequest::Version(VersionRequest::Default),
+                    SystemPython::Disallowed,
+                    &SourceSelector::All,
+                    &cache,
+                )
+                .unwrap()
+                .unwrap();
+                assert_eq!(
+                    result.interpreter().python_full_version().to_string(),
+                    "3.10.1",
+                    "Should skip the system Python"
+                );
+            },
+        );
+
+        Ok(())
+    }
+
     #[test]
     fn find_interpreter_version_minor() -> Result<()> {
         let tempdir = TempDir::new()?;
@@ -546,7 +721,12 @@ mod tests {
                 ("PWD", Some(tempdir.path().into())),
             ],
             || {
-                let result = find_interpreter(&InterpreterRequest::parse("3.11"), &sources, &cache);
+                let result = find_interpreter(
+                    &InterpreterRequest::parse("3.11"),
+                    SystemPython::Allowed,
+                    &sources,
+                    &cache,
+                );
                 assert!(
                     matches!(
                         result,
@@ -598,8 +778,12 @@ mod tests {
                 ("PWD", Some(tempdir.path().into())),
             ],
             || {
-                let result =
+                let result = find_interpreter(
-                    find_interpreter(&InterpreterRequest::parse("3.11.2"), &sources, &cache);
+                    &InterpreterRequest::parse("3.11.2"),
+                    SystemPython::Allowed,
+                    &sources,
+                    &cache,
+                );
                 assert!(
                     matches!(
                         result,
@@ -651,7 +835,12 @@ mod tests {
                 ("PWD", Some(tempdir.path().into())),
             ],
             || {
-                let result = find_interpreter(&InterpreterRequest::parse("3.9"), &sources, &cache);
+                let result = find_interpreter(
+                    &InterpreterRequest::parse("3.9"),
+                    SystemPython::Allowed,
+                    &sources,
+                    &cache,
+                );
                 assert!(
                     matches!(
                         result,
@@ -693,8 +882,12 @@ mod tests {
                 ("PWD", Some(tempdir.path().into())),
             ],
             || {
-                let result =
+                let result = find_interpreter(
-                    find_interpreter(&InterpreterRequest::parse("3.11.9"), &sources, &cache);
+                    &InterpreterRequest::parse("3.11.9"),
+                    SystemPython::Allowed,
+                    &sources,
+                    &cache,
+                );
                 assert!(
                     matches!(
                         result,
@@ -1281,6 +1474,7 @@ mod tests {
             &python,
             &PythonVersion::from_str("3.10.0").unwrap(),
             ImplementationName::default(),
+            true,
         )?;
 
         with_vars(
@@ -1314,6 +1508,7 @@ mod tests {
             &python,
             &PythonVersion::from_str("3.10.0").unwrap(),
             ImplementationName::default(),
+            true,
         )?;
 
         with_vars(
@@ -1350,6 +1545,7 @@ mod tests {
             &python,
             &PythonVersion::from_str("3.10.0").unwrap(),
             ImplementationName::default(),
+            true,
         )?;
 
         with_vars(
@@ -1418,6 +1614,7 @@ mod tests {
             &python,
             &PythonVersion::from_str("3.10.0").unwrap(),
             ImplementationName::default(),
+            true,
         )?;
 
         with_vars(
@@ -1491,6 +1688,7 @@ mod tests {
             &python,
             &PythonVersion::from_str("3.10.0").unwrap(),
             ImplementationName::default(),
+            true,
         )?;
 
         with_vars(
@@ -1526,7 +1724,7 @@ mod tests {
                     "PATH",
                     Some(mock_interpreters(
                         &tempdir,
-                        &[(ImplementationName::PyPy, "pypy", "3.10.1")],
+                        &[(true, ImplementationName::PyPy, "pypy", "3.10.1")],
                     )?),
                 ),
                 ("PWD", Some(tempdir.path().into())),
@@ -1559,8 +1757,8 @@ mod tests {
                     Some(mock_interpreters(
                         &tempdir,
                         &[
-                            (ImplementationName::CPython, "python", "3.10.0"),
+                            (true, ImplementationName::CPython, "python", "3.10.0"),
-                            (ImplementationName::PyPy, "pypy", "3.10.1"),
+                            (true, ImplementationName::PyPy, "pypy", "3.10.1"),
                         ],
                     )?),
                 ),
@@ -1595,8 +1793,8 @@ mod tests {
                     Some(mock_interpreters(
                         &tempdir,
                         &[
-                            (ImplementationName::PyPy, "pypy", "3.9"),
+                            (true, ImplementationName::PyPy, "pypy", "3.9"),
-                            (ImplementationName::PyPy, "pypy", "3.10.1"),
+                            (true, ImplementationName::PyPy, "pypy", "3.10.1"),
                         ],
                     )?),
                 ),
@@ -1631,9 +1829,9 @@ mod tests {
                     Some(mock_interpreters(
                         &tempdir,
                         &[
-                            (ImplementationName::PyPy, "pypy3.9", "3.10.0"), // We don't consider this one because of the executable name
+                            (true, ImplementationName::PyPy, "pypy3.9", "3.10.0"), // We don't consider this one because of the executable name
-                            (ImplementationName::PyPy, "pypy3.10", "3.10.1"),
+                            (true, ImplementationName::PyPy, "pypy3.10", "3.10.1"),
-                            (ImplementationName::PyPy, "pypy", "3.10.2"),
+                            (true, ImplementationName::PyPy, "pypy", "3.10.2"),
                         ],
                     )?),
                 ),
@@ -1667,11 +1865,13 @@ mod tests {
             &tempdir.path().join("python"),
             &PythonVersion::from_str("3.10.0").unwrap(),
             ImplementationName::PyPy,
+            true,
         )?;
         create_mock_interpreter(
             &tempdir.path().join("pypy"),
             &PythonVersion::from_str("3.10.1").unwrap(),
             ImplementationName::PyPy,
+            true,
         )?;
         with_vars(
             [
@@ -1703,8 +1903,8 @@ mod tests {
                     Some(mock_interpreters(
                         &tempdir,
                         &[
-                            (ImplementationName::PyPy, "python", "3.10.0"),
+                            (true, ImplementationName::PyPy, "python", "3.10.0"),
-                            (ImplementationName::PyPy, "pypy", "3.10.1"),
+                            (true, ImplementationName::PyPy, "pypy", "3.10.1"),
                         ],
                     )?),
                 ),
@@ -1737,11 +1937,13 @@ mod tests {
             &tempdir.path().join("pypy3.10"),
             &PythonVersion::from_str("3.10.0").unwrap(),
             ImplementationName::PyPy,
+            true,
         )?;
         create_mock_interpreter(
             &tempdir.path().join("pypy"),
             &PythonVersion::from_str("3.10.1").unwrap(),
             ImplementationName::PyPy,
+            true,
         )?;
         with_vars(
             [
@@ -1769,11 +1971,13 @@ mod tests {
             &tempdir.path().join("python3.10"),
             &PythonVersion::from_str("3.10.0").unwrap(),
             ImplementationName::PyPy,
+            true,
         )?;
         create_mock_interpreter(
             &tempdir.path().join("pypy"),
             &PythonVersion::from_str("3.10.1").unwrap(),
             ImplementationName::PyPy,
+            true,
         )?;
         with_vars(
             [

crates/uv/src/commands/pip/compile.rs

@@ -171,7 +171,7 @@ pub(crate) async fn pip_compile(
     let interpreter = if let Some(python) = python.as_ref() {
         let request = InterpreterRequest::parse(python);
         let sources = SourceSelector::from_env(system);
-        find_interpreter(&request, &sources, &cache)??
+        find_interpreter(&request, system, &sources, &cache)??
     } else {
         let request = if let Some(version) = python_version.as_ref() {
             // TODO(zanieb): We should consolidate `VersionRequest` and `PythonVersion`

crates/uv/src/commands/venv.rs

@@ -119,9 +119,10 @@ async fn venv_impl(
 ) -> miette::Result<ExitStatus> {
     // Locate the Python interpreter.
     let interpreter = if let Some(python) = python_request.as_ref() {
+        let system = uv_interpreter::SystemPython::Required;
         let request = InterpreterRequest::parse(python);
-        let sources = SourceSelector::from_env(uv_interpreter::SystemPython::Required);
+        let sources = SourceSelector::from_env(system);
-        find_interpreter(&request, &sources, cache)
+        find_interpreter(&request, system, &sources, cache)
     } else {
         find_default_interpreter(cache)
     }

crates/uv/tests/common/mod.rs

@@ -401,7 +401,14 @@ pub fn python_path_with_versions(
                         .expect("The test version request must be valid"),
                 );
                 let sources = SourceSelector::All;
-                if let Ok(found) = find_interpreter(&request, &sources, &cache).unwrap() {
+                if let Ok(found) = find_interpreter(
+                    &request,
+                    uv_interpreter::SystemPython::Allowed,
+                    &sources,
+                    &cache,
+                )
+                .unwrap()
+                {
                     vec![found
                         .into_interpreter()
                         .sys_executable()