From 90947a933df2fe3f42bc9cbbf27dbce3a3b3a708 Mon Sep 17 00:00:00 2001 From: konsti Date: Mon, 10 Jun 2024 16:36:14 +0200 Subject: [PATCH] Recreate project environment if `--python` or `requires-python` doesn't match (#3945) Fixes #4131 Fixes #3895 --- crates/uv-toolchain/src/discovery.rs | 115 ++++++++++++++++++++--- crates/uv/src/commands/project/lock.rs | 21 ++++- crates/uv/src/commands/project/mod.rs | 104 +++++++++++++++------ crates/uv/src/commands/project/run.rs | 8 +- crates/uv/src/commands/project/sync.rs | 9 +- crates/uv/src/main.rs | 2 + crates/uv/tests/common/mod.rs | 55 +++++++++-- crates/uv/tests/lock.rs | 3 +- crates/uv/tests/run.rs | 121 +++++++++++++++++++++++++ crates/uv/tests/workspace.rs | 34 +------ 10 files changed, 387 insertions(+), 85 deletions(-) create mode 100644 crates/uv/tests/run.rs diff --git a/crates/uv-toolchain/src/discovery.rs b/crates/uv-toolchain/src/discovery.rs index fbf68894787a..92de7ce4a085 100644 --- a/crates/uv-toolchain/src/discovery.rs +++ b/crates/uv-toolchain/src/discovery.rs @@ -1,11 +1,20 @@ +use std::borrow::Cow; +use std::collections::HashSet; +use std::fmt::{self, Formatter}; +use std::num::ParseIntError; +use std::{env, io}; +use std::{path::Path, path::PathBuf, str::FromStr}; + use itertools::Itertools; +use same_file::is_same_file; use thiserror::Error; use tracing::{debug, instrument, trace}; +use which::which; + use uv_cache::Cache; use uv_configuration::PreviewMode; use uv_fs::Simplified; use uv_warnings::warn_user_once; -use which::which; use crate::implementation::{ImplementationName, LenientImplementationName}; use crate::interpreter::Error as InterpreterError; @@ -17,17 +26,10 @@ use crate::virtualenv::{ virtualenv_python_executable, }; use crate::{Interpreter, PythonVersion}; -use std::borrow::Cow; - -use std::collections::HashSet; -use std::fmt::{self, Formatter}; -use std::num::ParseIntError; -use std::{env, io}; -use std::{path::Path, path::PathBuf, str::FromStr}; /// A request to find a Python toolchain. /// -/// See [`InterpreterRequest::from_str`]. +/// See [`ToolchainRequest::from_str`]. #[derive(Debug, Clone, PartialEq, Eq, Default)] pub enum ToolchainRequest { /// Use any discovered Python toolchain @@ -699,7 +701,7 @@ pub fn find_best_toolchain( debug!("Looking for Python toolchain with any version"); let request = ToolchainRequest::Any; Ok(find_toolchain( - // TODO(zanieb): Add a dedicated `Default` variant to `InterpreterRequest` + // TODO(zanieb): Add a dedicated `Default` variant to `ToolchainRequest` &request, system, &sources, cache, )? .map_err(|err| { @@ -860,7 +862,7 @@ fn is_windows_store_shim(_path: &Path) -> bool { impl ToolchainRequest { /// Create a request from a string. /// - /// This cannot fail, which means weird inputs will be parsed as [`InterpreterRequest::File`] or [`InterpreterRequest::ExecutableName`]. + /// This cannot fail, which means weird inputs will be parsed as [`ToolchainRequest::File`] or [`ToolchainRequest::ExecutableName`]. pub fn parse(value: &str) -> Self { // e.g. `3.12.1` if let Ok(version) = VersionRequest::from_str(value) { @@ -934,6 +936,93 @@ impl ToolchainRequest { // e.g. foo.exe Self::ExecutableName(value.to_string()) } + + /// Check if a given interpreter satisfies the interpreter request. + pub fn satisfied(&self, interpreter: &Interpreter, cache: &Cache) -> bool { + /// Returns `true` if the two paths refer to the same interpreter executable. + fn is_same_executable(path1: &Path, path2: &Path) -> bool { + path1 == path2 || is_same_file(path1, path2).unwrap_or(false) + } + + match self { + ToolchainRequest::Any => true, + ToolchainRequest::Version(version_request) => { + version_request.matches_interpreter(interpreter) + } + ToolchainRequest::Directory(directory) => { + // `sys.prefix` points to the venv root. + is_same_executable(directory, interpreter.sys_prefix()) + } + ToolchainRequest::File(file) => { + // The interpreter satisfies the request both if it is the venv... + if is_same_executable(interpreter.sys_executable(), file) { + return true; + } + // ...or if it is the base interpreter the venv was created from. + if interpreter + .sys_base_executable() + .is_some_and(|sys_base_executable| { + is_same_executable(sys_base_executable, file) + }) + { + return true; + } + // ...or, on Windows, if both interpreters have the same base executable. On + // Windows, interpreters are copied rather than symlinked, so a virtual environment + // created from within a virtual environment will _not_ evaluate to the same + // `sys.executable`, but will have the same `sys._base_executable`. + if cfg!(windows) { + if let Ok(file_interpreter) = Interpreter::query(file, cache) { + if let (Some(file_base), Some(interpreter_base)) = ( + file_interpreter.sys_base_executable(), + interpreter.sys_base_executable(), + ) { + if is_same_executable(file_base, interpreter_base) { + return true; + } + } + } + } + false + } + ToolchainRequest::ExecutableName(name) => { + // First, see if we have a match in the venv ... + if interpreter + .sys_executable() + .file_name() + .is_some_and(|filename| filename == name.as_str()) + { + return true; + } + // ... or the venv's base interpreter (without performing IO), if that fails, ... + if interpreter + .sys_base_executable() + .and_then(|executable| executable.file_name()) + .is_some_and(|file_name| file_name == name.as_str()) + { + return true; + } + // ... check in `PATH`. The name we find here does not need to be the + // name we install, so we can find `foopython` here which got installed as `python`. + if which(name) + .ok() + .as_ref() + .and_then(|executable| executable.file_name()) + .is_some_and(|file_name| file_name == name.as_str()) + { + return true; + } + false + } + ToolchainRequest::Implementation(implementation) => { + interpreter.implementation_name() == implementation.as_str() + } + ToolchainRequest::ImplementationVersion(implementation, version) => { + version.matches_interpreter(interpreter) + && interpreter.implementation_name() == implementation.as_str() + } + } + } } impl VersionRequest { @@ -1353,12 +1442,10 @@ impl fmt::Display for ToolchainSources { #[cfg(test)] mod tests { - use std::{path::PathBuf, str::FromStr}; - use test_log::test; - use assert_fs::{prelude::*, TempDir}; + use test_log::test; use crate::{ discovery::{ToolchainRequest, VersionRequest}, diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index 20758f369661..67b468b21fa4 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -14,7 +14,7 @@ use uv_git::GitResolver; use uv_normalize::PackageName; use uv_requirements::upgrade::{read_lockfile, LockedRequirements}; use uv_resolver::{ExcludeNewer, FlatIndex, InMemoryIndex, Lock, OptionsBuilder, RequiresPython}; -use uv_toolchain::{Interpreter, Toolchain}; +use uv_toolchain::{Interpreter, SystemPython, Toolchain, ToolchainRequest}; use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy, InFlight}; use uv_warnings::warn_user; @@ -28,6 +28,7 @@ pub(crate) async fn lock( index_locations: IndexLocations, upgrade: Upgrade, exclude_newer: Option, + python: Option, preview: PreviewMode, cache: &Cache, printer: Printer, @@ -41,9 +42,23 @@ pub(crate) async fn lock( // Find an interpreter for the project let interpreter = match project::find_environment(&workspace, cache) { - Ok(environment) => environment.into_interpreter(), + Ok(environment) => { + let interpreter = environment.into_interpreter(); + if let Some(python) = python.as_deref() { + let request = ToolchainRequest::parse(python); + if request.satisfied(&interpreter, cache) { + interpreter + } else { + Toolchain::find_requested(python, SystemPython::Allowed, preview, cache)? + .into_interpreter() + } + } else { + interpreter + } + } Err(uv_toolchain::Error::NotFound(_)) => { - Toolchain::find_default(PreviewMode::Enabled, cache)?.into_interpreter() + Toolchain::find(python.as_deref(), SystemPython::Allowed, preview, cache)? + .into_interpreter() } Err(err) => return Err(err.into()), }; diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index beca0f76f3f9..38b207847575 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -1,6 +1,6 @@ use std::fmt::Write; -use anyhow::Result; +use anyhow::{Context, Result}; use itertools::Itertools; use owo_colors::OwoColorize; use tracing::debug; @@ -21,8 +21,9 @@ use uv_git::GitResolver; use uv_installer::{SatisfiesResult, SitePackages}; use uv_requirements::{RequirementsSource, RequirementsSpecification}; use uv_resolver::{FlatIndex, InMemoryIndex, Options, RequiresPython}; -use uv_toolchain::{PythonEnvironment, Toolchain}; +use uv_toolchain::{PythonEnvironment, SystemPython, Toolchain, ToolchainRequest}; use uv_types::{BuildIsolation, HashStrategy, InFlight}; +use uv_warnings::warn_user; use crate::commands::pip; use crate::printer::Printer; @@ -81,42 +82,93 @@ pub(crate) fn find_environment( /// Initialize a virtual environment for the current project. pub(crate) fn init_environment( workspace: &Workspace, + python: Option<&str>, preview: PreviewMode, cache: &Cache, printer: Printer, ) -> Result { - // Discover or create the virtual environment. - // TODO(charlie): If the environment isn't compatible with `--python`, recreate it. - match find_environment(workspace, cache) { - Ok(venv) => Ok(venv), - Err(uv_toolchain::Error::NotFound(_)) => { - // TODO(charlie): Respect `--python`; if unset, respect `Requires-Python`. - let interpreter = Toolchain::find_default(preview, cache)?.into_interpreter(); + let venv = workspace.root().join(".venv"); - writeln!( - printer.stderr(), - "Using Python {} interpreter at: {}", - interpreter.python_version(), - interpreter.sys_executable().user_display().cyan() - )?; + let requires_python = workspace + .root_member() + .and_then(|root| root.project().requires_python.as_ref()); + + // Discover or create the virtual environment. + match PythonEnvironment::from_root(venv, cache) { + Ok(venv) => { + // `--python` has highest precedence, after that we check `requires_python` from + // `pyproject.toml`. If `--python` and `requires_python` are mutually incompatible, + // we'll fail at the build or at last the install step when we aren't able to install + // the editable wheel for the current project into the venv. + // TODO(konsti): Do we want to support a workspace python version requirement? + let is_satisfied = if let Some(python) = python { + ToolchainRequest::parse(python).satisfied(venv.interpreter(), cache) + } else if let Some(requires_python) = requires_python { + requires_python.contains(venv.interpreter().python_version()) + } else { + true + }; + + if is_satisfied { + return Ok(venv); + } - let venv = workspace.venv(); writeln!( printer.stderr(), - "Creating virtualenv at: {}", - venv.user_display().cyan() + "Removing virtual environment at: {}", + venv.root().user_display().cyan() )?; + fs_err::remove_dir_all(venv.root()) + .context("Failed to remove existing virtual environment")?; + } + Err(uv_toolchain::Error::NotFound(_)) => {} + Err(e) => return Err(e.into()), + } - Ok(uv_virtualenv::create_venv( - &venv, - interpreter, - uv_virtualenv::Prompt::None, - false, - false, - )?) + // TODO(konsti): Extend `VersionRequest` to support `VersionSpecifiers`. + let requires_python_str = requires_python.map(ToString::to_string); + let interpreter = Toolchain::find( + python.or(requires_python_str.as_deref()), + // Otherwise we'll try to use the venv we just deleted. + SystemPython::Required, + preview, + cache, + )? + .into_interpreter(); + + if let Some(requires_python) = requires_python { + if !requires_python.contains(interpreter.python_version()) { + warn_user!( + "The Python {} you requested with {} is incompatible with the requirement of the \ + project of {}", + interpreter.python_version(), + python.unwrap_or("(default)"), + requires_python + ); } - Err(e) => Err(e.into()), } + + writeln!( + printer.stderr(), + "Using Python {} interpreter at: {}", + interpreter.python_version(), + interpreter.sys_executable().user_display().cyan() + )?; + + let venv = workspace.venv(); + writeln!( + printer.stderr(), + "Creating virtualenv at: {}", + venv.user_display().cyan() + )?; + + Ok(uv_virtualenv::create_venv( + &venv, + interpreter, + uv_virtualenv::Prompt::None, + false, + false, + )?) } /// Update a [`PythonEnvironment`] to satisfy a set of [`RequirementsSource`]s. diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index ec755056d369..f024282d28b0 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -63,7 +63,13 @@ pub(crate) async fn run( } else { ProjectWorkspace::discover(&std::env::current_dir()?, None).await? }; - let venv = project::init_environment(project.workspace(), preview, cache, printer)?; + let venv = project::init_environment( + project.workspace(), + python.as_deref(), + preview, + cache, + printer, + )?; // Lock and sync the environment. let root_project_name = project diff --git a/crates/uv/src/commands/project/sync.rs b/crates/uv/src/commands/project/sync.rs index 9ab29088bbdf..2bb710c5ff29 100644 --- a/crates/uv/src/commands/project/sync.rs +++ b/crates/uv/src/commands/project/sync.rs @@ -28,6 +28,7 @@ pub(crate) async fn sync( index_locations: IndexLocations, extras: ExtrasSpecification, dev: bool, + python: Option, preview: PreviewMode, cache: &Cache, printer: Printer, @@ -40,7 +41,13 @@ pub(crate) async fn sync( let project = ProjectWorkspace::discover(&std::env::current_dir()?, None).await?; // Discover or create the virtual environment. - let venv = project::init_environment(project.workspace(), preview, cache, printer)?; + let venv = project::init_environment( + project.workspace(), + python.as_deref(), + preview, + cache, + printer, + )?; // Read the lockfile. let lock: Lock = { diff --git a/crates/uv/src/main.rs b/crates/uv/src/main.rs index aa7cf0329a75..54d8c1262116 100644 --- a/crates/uv/src/main.rs +++ b/crates/uv/src/main.rs @@ -612,6 +612,7 @@ async fn run() -> Result { args.index_locations, args.extras, args.dev, + args.python, globals.preview, &cache, printer, @@ -629,6 +630,7 @@ async fn run() -> Result { args.index_locations, args.upgrade, args.exclude_newer, + args.python, globals.preview, &cache, printer, diff --git a/crates/uv/tests/common/mod.rs b/crates/uv/tests/common/mod.rs index 5f25ac6a1e18..8f6aa7e9b5be 100644 --- a/crates/uv/tests/common/mod.rs +++ b/crates/uv/tests/common/mod.rs @@ -274,6 +274,38 @@ impl TestContext { command } + /// Create a `uv run` command with options shared across scenarios. + pub fn run(&self) -> std::process::Command { + let mut command = self.run_without_exclude_newer(); + command.arg("--exclude-newer").arg(EXCLUDE_NEWER); + command + } + + /// Create a `uv run` command with no `--exclude-newer` option. + /// + /// One should avoid using this in tests to the extent possible because + /// it can result in tests failing when the index state changes. Therefore, + /// if you use this, there should be some other kind of mitigation in place. + /// For example, pinning package versions. + pub fn run_without_exclude_newer(&self) -> std::process::Command { + let mut command = std::process::Command::new(get_bin()); + command + .arg("run") + .arg("--cache-dir") + .arg(self.cache_dir.path()) + .env("VIRTUAL_ENV", self.venv.as_os_str()) + .env("UV_NO_WRAP", "1") + .current_dir(&self.temp_dir); + + if cfg!(all(windows, debug_assertions)) { + // TODO(konstin): Reduce stack usage in debug mode enough that the tests pass with the + // default windows stack of 1MB + command.env("UV_STACK_SIZE", (4 * 1024 * 1024).to_string()); + } + + command + } + pub fn interpreter(&self) -> PathBuf { venv_to_interpreter(&self.venv) } @@ -397,14 +429,9 @@ pub fn venv_to_interpreter(venv: &Path) -> PathBuf { } } -/// Create a virtual environment named `.venv` in a temporary directory with the given -/// Python version. Expected format for `python` is "". -pub fn create_venv>( - temp_dir: &Parent, - cache_dir: &assert_fs::TempDir, - python: &str, -) -> PathBuf { - let python = InstalledToolchains::from_settings() +/// Get the path to the python interpreter for a specific toolchain version. +pub fn get_toolchain(python: &str) -> PathBuf { + InstalledToolchains::from_settings() .map(|installed_toolchains| { installed_toolchains .find_version( @@ -419,7 +446,17 @@ pub fn create_venv". +pub fn create_venv>( + temp_dir: &Parent, + cache_dir: &assert_fs::TempDir, + python: &str, +) -> PathBuf { + let python = get_toolchain(python); let venv = temp_dir.child(".venv"); Command::new(get_bin()) diff --git a/crates/uv/tests/lock.rs b/crates/uv/tests/lock.rs index bb59300790c0..e4769f5be4d9 100644 --- a/crates/uv/tests/lock.rs +++ b/crates/uv/tests/lock.rs @@ -2078,7 +2078,8 @@ fn lock_requires_python() -> Result<()> { ----- stderr ----- warning: `uv sync` is experimental and may change without warning. - error: The current Python version (3.8.[X]) is not compatible with the locked Python requirement (>=3.12) + Removing virtual environment at: [VENV]/ + error: Requested Python executable `>=3.12` not found in PATH "###); Ok(()) diff --git a/crates/uv/tests/run.rs b/crates/uv/tests/run.rs new file mode 100644 index 000000000000..d28709592183 --- /dev/null +++ b/crates/uv/tests/run.rs @@ -0,0 +1,121 @@ +#![cfg(all(feature = "python", feature = "pypi"))] + +use anyhow::Result; +use assert_fs::prelude::*; +use indoc::indoc; + +use crate::common::get_toolchain; +use common::{uv_snapshot, TestContext}; + +mod common; + +/// Run with different python versions, which also depend on different dependency versions. +#[test] +fn run_with_python_version() -> Result<()> { + let context = TestContext::new("3.12"); + + let pyproject_toml = context.temp_dir.child("pyproject.toml"); + pyproject_toml.write_str(indoc! { r#" + [project] + name = "foo" + version = "1.0.0" + requires-python = ">=3.11, <4" + dependencies = [ + "anyio==3.6.0 ; python_version == '3.11'", + "anyio==3.7.0 ; python_version == '3.12'", + ] + "# + })?; + let test_script = context.temp_dir.child("main.py"); + test_script.write_str(indoc! { r#" + import importlib.metadata + import platform + + print(platform.python_version()) + print(importlib.metadata.version("anyio")) + "# + })?; + + // Our tests change files in <1s, so we must disable CPython bytecode caching with `-B` or we'll + // get stale files, see https://github.com/python/cpython/issues/75953. + let mut command = context.run(); + let command_with_args = command + .arg("--preview") + .arg("python") + .arg("-B") + .arg("main.py"); + uv_snapshot!(context.filters(), command_with_args, @r###" + success: true + exit_code: 0 + ----- stdout ----- + 3.12.[X] + 3.7.0 + + ----- stderr ----- + Resolved 10 packages in [TIME] + Downloaded 4 packages in [TIME] + Installed 4 packages in [TIME] + + anyio==3.7.0 + + foo==1.0.0 (from file://[TEMP_DIR]/) + + idna==3.6 + + sniffio==1.3.1 + "###); + + // This is the same Python, no reinstallation. + let mut command = context.run(); + let command_with_args = command + .arg("--preview") + .arg("-p") + .arg(get_toolchain("3.12")) + .arg("python") + .arg("-B") + .arg("main.py"); + uv_snapshot!(context.filters(), command_with_args, @r###" + success: true + exit_code: 0 + ----- stdout ----- + 3.12.[X] + 3.7.0 + + ----- stderr ----- + Resolved 10 packages in [TIME] + Audited 4 packages in [TIME] + "###); + + // This time, we target Python 3.11 instead. + let mut command = context.run(); + let command_with_args = command + .arg("--preview") + .arg("-p") + .arg(get_toolchain("3.11")) + .arg("python") + .arg("-B") + .arg("main.py"); + let mut filters = context.filters(); + filters.push(( + r"Using Python 3.11.\d+ interpreter at: .*", + "Using Python 3.11.[X] interpreter at: [PYTHON]", + )); + filters.push((r"3.11.\d+", "3.11.[X]")); + uv_snapshot!(filters, command_with_args, @r###" + success: true + exit_code: 0 + ----- stdout ----- + 3.11.[X] + 3.6.0 + + ----- stderr ----- + Removing virtual environment at: [VENV]/ + Using Python 3.11.[X] interpreter at: [PYTHON] + Creating virtualenv at: [VENV]/ + Resolved 10 packages in [TIME] + Downloaded 4 packages in [TIME] + Installed 4 packages in [TIME] + + anyio==3.6.0 + + foo==1.0.0 (from file://[TEMP_DIR]/) + + idna==3.6 + + sniffio==1.3.1 + "###); + + Ok(()) +} diff --git a/crates/uv/tests/workspace.rs b/crates/uv/tests/workspace.rs index d066fdc12d0c..5513246633e4 100644 --- a/crates/uv/tests/workspace.rs +++ b/crates/uv/tests/workspace.rs @@ -402,16 +402,6 @@ fn test_uv_run_with_package_virtual_workspace() -> Result<()> { &work_dir, )?; - // TODO(konsti): `--python` is being ignored atm, so we need to create the correct venv - // ourselves and add the output filters. - let venv = work_dir.join(".venv"); - assert_cmd::Command::new(get_bin()) - .arg("venv") - .arg("-p") - .arg(context.interpreter()) - .arg(&venv) - .assert(); - let mut filters = context.filters(); filters.push(( r"Using Python 3.12.\[X\] interpreter at: .*", @@ -429,6 +419,8 @@ fn test_uv_run_with_package_virtual_workspace() -> Result<()> { Success ----- stderr ----- + Using Python 3.12.[X] interpreter at: [PYTHON] + Creating virtualenv at: [VENV]/ Resolved 10 packages in [TIME] Downloaded 5 packages in [TIME] Installed 5 packages in [TIME] @@ -470,16 +462,6 @@ fn test_uv_run_with_package_root_workspace() -> Result<()> { copy_dir_ignore(workspaces_dir().join("albatross-root-workspace"), &work_dir)?; - // TODO(konsti): `--python` is being ignored atm, so we need to create the correct venv - // ourselves and add the output filters. - let venv = work_dir.join(".venv"); - assert_cmd::Command::new(get_bin()) - .arg("venv") - .arg("-p") - .arg(context.interpreter()) - .arg(&venv) - .assert(); - let mut filters = context.filters(); filters.push(( r"Using Python 3.12.\[X\] interpreter at: .*", @@ -497,6 +479,8 @@ fn test_uv_run_with_package_root_workspace() -> Result<()> { Success ----- stderr ----- + Using Python 3.12.[X] interpreter at: [PYTHON] + Creating virtualenv at: [VENV]/ Resolved 10 packages in [TIME] Downloaded 5 packages in [TIME] Installed 5 packages in [TIME] @@ -540,16 +524,6 @@ fn workspace_lock_idempotence(workspace: &str, subdirectories: &[&str]) -> Resul copy_dir_ignore(workspaces_dir().join(workspace), &work_dir)?; - // TODO(konsti): `--python` is being ignored atm, so we need to create the correct venv - // ourselves and add the output filters. - let venv = work_dir.join(".venv"); - assert_cmd::Command::new(get_bin()) - .arg("venv") - .arg("-p") - .arg(context.interpreter()) - .arg(&venv) - .assert(); - lock_workspace(&context) .current_dir(&work_dir.join(dir)) .assert()