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

Gate discovery of managed toolchains with preview #3835

Merged
merged 1 commit into from
May 27, 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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/uv-interpreter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pep508_rs = { workspace = true }
platform-tags = { workspace = true }
pypi-types = { workspace = true }
uv-cache = { workspace = true }
uv-configuration = { workspace = true }
uv-client = { workspace = true }
uv-extract = { workspace = true }
uv-fs = { workspace = true }
Expand Down
89 changes: 59 additions & 30 deletions crates/uv-interpreter/src/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use itertools::Itertools;
use thiserror::Error;
use tracing::{debug, instrument, trace};
use uv_cache::Cache;
use uv_configuration::PreviewMode;
use uv_fs::Simplified;
use uv_warnings::warn_user_once;
use which::which;
Expand Down Expand Up @@ -46,13 +47,12 @@ pub enum InterpreterRequest {
}

/// The sources to consider when finding a Python interpreter.
#[derive(Debug, Clone, PartialEq, Eq, Default)]
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum SourceSelector {
// Consider all interpreter sources.
#[default]
All,
All(PreviewMode),
// Only consider system interpreter sources
System,
System(PreviewMode),
// Only consider virtual environment sources
VirtualEnv,
// Only consider a custom set of sources
Expand Down Expand Up @@ -632,9 +632,12 @@ pub fn find_interpreter(
/// Virtual environments are not included in discovery.
///
/// See [`find_interpreter`] for more details on interpreter discovery.
pub fn find_default_interpreter(cache: &Cache) -> Result<InterpreterResult, Error> {
pub fn find_default_interpreter(
preview: PreviewMode,
cache: &Cache,
) -> Result<InterpreterResult, Error> {
let request = InterpreterRequest::default();
let sources = SourceSelector::System;
let sources = SourceSelector::System(preview);

let result = find_interpreter(&request, SystemPython::Required, &sources, cache)?;
if let Ok(ref found) = result {
Expand All @@ -658,12 +661,13 @@ pub fn find_default_interpreter(cache: &Cache) -> Result<InterpreterResult, Erro
pub fn find_best_interpreter(
request: &InterpreterRequest,
system: SystemPython,
preview: PreviewMode,
cache: &Cache,
) -> Result<InterpreterResult, Error> {
debug!("Starting interpreter discovery for {}", request);

// Determine if we should be allowed to look outside of virtual environments.
let sources = SourceSelector::from_settings(system);
let sources = SourceSelector::from_settings(system, preview);

// 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}");
Expand Down Expand Up @@ -1136,16 +1140,23 @@ impl SourceSelector {
/// Return true if this selector includes the given [`InterpreterSource`].
fn contains(&self, source: InterpreterSource) -> bool {
match self {
Self::All => true,
Self::System => [
InterpreterSource::ProvidedPath,
InterpreterSource::SearchPath,
#[cfg(windows)]
InterpreterSource::PyLauncher,
InterpreterSource::ManagedToolchain,
InterpreterSource::ParentInterpreter,
]
.contains(&source),
Self::All(preview) => {
// Always return `true` except for `ManagedToolchain` which requires preview mode
source != InterpreterSource::ManagedToolchain || preview.is_enabled()
}
Self::System(preview) => {
[
InterpreterSource::ProvidedPath,
InterpreterSource::SearchPath,
#[cfg(windows)]
InterpreterSource::PyLauncher,
InterpreterSource::ParentInterpreter,
]
.contains(&source)
// Allow `ManagedToolchain` in preview
|| (source == InterpreterSource::ManagedToolchain
&& preview.is_enabled())
}
Self::VirtualEnv => [
InterpreterSource::DiscoveredEnvironment,
InterpreterSource::ActiveEnvironment,
Expand All @@ -1157,7 +1168,7 @@ impl SourceSelector {
}

/// Return a [`SourceSelector`] based the settings.
pub fn from_settings(system: SystemPython) -> Self {
pub fn from_settings(system: SystemPython, preview: PreviewMode) -> Self {
if env::var_os("UV_FORCE_MANAGED_PYTHON").is_some() {
debug!("Only considering managed toolchains due to `UV_FORCE_MANAGED_PYTHON`");
Self::from_sources([InterpreterSource::ManagedToolchain])
Expand All @@ -1171,8 +1182,8 @@ impl SourceSelector {
])
} else {
match system {
SystemPython::Allowed | SystemPython::Explicit => Self::All,
SystemPython::Required => Self::System,
SystemPython::Allowed | SystemPython::Explicit => Self::All(preview),
SystemPython::Required => Self::System(preview),
SystemPython::Disallowed => Self::VirtualEnv,
}
}
Expand Down Expand Up @@ -1283,19 +1294,37 @@ impl fmt::Display for InterpreterNotFound {
impl fmt::Display for SourceSelector {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
Self::All => f.write_str("all sources"),
Self::All(_) => f.write_str("all sources"),
Self::VirtualEnv => f.write_str("virtual environments"),
Self::System => {
// TODO(zanieb): We intentionally omit managed toolchains for now since they are not public
Self::System(preview) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll probably rethink this display in a future change.

if cfg!(windows) {
write!(
f,
"{} or {}",
InterpreterSource::SearchPath,
InterpreterSource::PyLauncher
)
if preview.is_disabled() {
write!(
f,
"{} or {}",
InterpreterSource::SearchPath,
InterpreterSource::PyLauncher
)
} else {
write!(
f,
"{}, {}, or {}",
InterpreterSource::SearchPath,
InterpreterSource::PyLauncher,
InterpreterSource::ManagedToolchain
)
}
} else {
write!(f, "{}", InterpreterSource::SearchPath)
if preview.is_disabled() {
write!(f, "{}", InterpreterSource::SearchPath)
} else {
write!(
f,
"{} or {}",
InterpreterSource::SearchPath,
InterpreterSource::ManagedToolchain
)
}
}
}
Self::Custom(sources) => {
Expand Down
23 changes: 16 additions & 7 deletions crates/uv-interpreter/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use itertools::Either;
use std::env;
use std::path::{Path, PathBuf};
use std::sync::Arc;
use uv_configuration::PreviewMode;

use same_file::is_same_file;

Expand All @@ -26,12 +27,17 @@ struct PythonEnvironmentShared {

impl PythonEnvironment {
/// Create a [`PythonEnvironment`] from a user request.
pub fn find(python: Option<&str>, system: SystemPython, cache: &Cache) -> Result<Self, Error> {
pub fn find(
python: Option<&str>,
system: SystemPython,
preview: PreviewMode,
cache: &Cache,
) -> Result<Self, Error> {
// Detect the current Python interpreter.
if let Some(python) = python {
Self::from_requested_python(python, system, cache)
Self::from_requested_python(python, system, preview, cache)
} else if system.is_preferred() {
Self::from_default_python(cache)
Self::from_default_python(preview, cache)
} else {
// First check for a parent intepreter
// We gate this check to avoid an extra log message when it is not set
Expand All @@ -46,7 +52,9 @@ impl PythonEnvironment {
// Then a virtual environment
match Self::from_virtualenv(cache) {
Ok(venv) => Ok(venv),
Err(Error::NotFound(_)) if system.is_allowed() => Self::from_default_python(cache),
Err(Error::NotFound(_)) if system.is_allowed() => {
Self::from_default_python(preview, cache)
}
Err(err) => Err(err),
}
}
Expand Down Expand Up @@ -110,9 +118,10 @@ impl PythonEnvironment {
pub fn from_requested_python(
request: &str,
system: SystemPython,
preview: PreviewMode,
cache: &Cache,
) -> Result<Self, Error> {
let sources = SourceSelector::from_settings(system);
let sources = SourceSelector::from_settings(system, preview);
let request = InterpreterRequest::parse(request);
let interpreter = find_interpreter(&request, system, &sources, cache)??.into_interpreter();
Ok(Self(Arc::new(PythonEnvironmentShared {
Expand All @@ -122,8 +131,8 @@ impl PythonEnvironment {
}

/// Create a [`PythonEnvironment`] for the default Python interpreter.
pub fn from_default_python(cache: &Cache) -> Result<Self, Error> {
let interpreter = find_default_interpreter(cache)??.into_interpreter();
pub fn from_default_python(preview: PreviewMode, cache: &Cache) -> Result<Self, Error> {
let interpreter = find_default_interpreter(preview, cache)??.into_interpreter();
Ok(Self(Arc::new(PythonEnvironmentShared {
root: interpreter.prefix().to_path_buf(),
interpreter,
Expand Down
Loading
Loading