From 197be1def41edb530a3e1d1133a5837f01159e61 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 11 Feb 2025 22:01:33 -0500 Subject: [PATCH] Support --active for PEP 723 script environments --- Cargo.lock | 2 +- crates/uv-cli/src/lib.rs | 8 +- crates/uv-fs/Cargo.toml | 1 + crates/uv-fs/src/lib.rs | 32 +++++++ crates/uv-workspace/Cargo.toml | 1 - crates/uv-workspace/src/workspace.rs | 30 +----- crates/uv/src/commands/project/add.rs | 1 + crates/uv/src/commands/project/export.rs | 1 + crates/uv/src/commands/project/lock.rs | 1 + crates/uv/src/commands/project/mod.rs | 111 ++++++++++++++++++----- crates/uv/src/commands/project/remove.rs | 1 + crates/uv/src/commands/project/run.rs | 3 + crates/uv/src/commands/project/sync.rs | 1 + crates/uv/src/commands/project/tree.rs | 1 + crates/uv/tests/it/run.rs | 111 ++++++++++++++++++++++- crates/uv/tests/it/sync.rs | 106 +++++++++++++++++++++- docs/reference/cli.md | 4 +- 17 files changed, 354 insertions(+), 61 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c1865924a1997..d2f1e8d3f2283 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5111,6 +5111,7 @@ dependencies = [ "path-slash", "percent-encoding", "rustix", + "same-file", "schemars", "serde", "tempfile", @@ -5789,7 +5790,6 @@ dependencies = [ "owo-colors", "regex", "rustc-hash", - "same-file", "schemars", "serde", "tempfile", diff --git a/crates/uv-cli/src/lib.rs b/crates/uv-cli/src/lib.rs index 1887ba9be76cb..95f951e376534 100644 --- a/crates/uv-cli/src/lib.rs +++ b/crates/uv-cli/src/lib.rs @@ -3053,10 +3053,11 @@ pub struct SyncArgs { #[arg(long, overrides_with("inexact"), hide = true)] pub exact: bool, - /// Prefer the active virtual environment over the project's virtual environment. + /// Sync dependencies to the active virtual environment. /// - /// If the project virtual environment is active or no virtual environment is active, this has - /// no effect. + /// Instead of creating or updating the virtual environment for the project or script, the + /// active virtual environment will be preferred, if the `VIRTUAL_ENV` environment variable is + /// set. #[arg(long, overrides_with = "no_active")] pub active: bool, @@ -3150,7 +3151,6 @@ pub struct SyncArgs { /// adherence with PEP 723. #[arg( long, - conflicts_with = "active", conflicts_with = "all_packages", conflicts_with = "package", conflicts_with = "no_install_project", diff --git a/crates/uv-fs/Cargo.toml b/crates/uv-fs/Cargo.toml index f15a1fc6bebaa..2860dc8a89a92 100644 --- a/crates/uv-fs/Cargo.toml +++ b/crates/uv-fs/Cargo.toml @@ -23,6 +23,7 @@ fs-err = { workspace = true } fs2 = { workspace = true } path-slash = { workspace = true } percent-encoding = { workspace = true } +same-file = { workspace = true } schemars = { workspace = true, optional = true } serde = { workspace = true, optional = true } tempfile = { workspace = true } diff --git a/crates/uv-fs/src/lib.rs b/crates/uv-fs/src/lib.rs index 2b7f3f6fd6ff0..ce0bdc6865775 100644 --- a/crates/uv-fs/src/lib.rs +++ b/crates/uv-fs/src/lib.rs @@ -11,6 +11,38 @@ pub mod cachedir; mod path; pub mod which; +/// Attempt to check if the two paths refer to the same file. +/// +/// Returns `Some(true)` if the files are missing, but would be the same if they existed. +pub fn is_same_file_allow_missing(left: &Path, right: &Path) -> Option { + // First, check an exact path comparison. + if left == right { + return Some(true); + } + + // Second, check the files directly. + if let Ok(value) = same_file::is_same_file(left, right) { + return Some(value); + }; + + // Often, one of the directories won't exist yet so perform the comparison up a level. + if let (Some(left_parent), Some(right_parent), Some(left_name), Some(right_name)) = ( + left.parent(), + right.parent(), + left.file_name(), + right.file_name(), + ) { + match same_file::is_same_file(left_parent, right_parent) { + Ok(true) => return Some(left_name == right_name), + Ok(false) => return Some(false), + _ => (), + } + }; + + // We couldn't determine if they're the same. + None +} + /// Reads data from the path and requires that it be valid UTF-8 or UTF-16. /// /// This uses BOM sniffing to determine if the data should be transcoded diff --git a/crates/uv-workspace/Cargo.toml b/crates/uv-workspace/Cargo.toml index 373eacc9e213e..e63033bba8b4c 100644 --- a/crates/uv-workspace/Cargo.toml +++ b/crates/uv-workspace/Cargo.toml @@ -34,7 +34,6 @@ glob = { workspace = true } itertools = { workspace = true } owo-colors = { workspace = true } rustc-hash = { workspace = true } -same-file = { workspace = true } schemars = { workspace = true, optional = true } serde = { workspace = true, features = ["derive"] } thiserror = { workspace = true } diff --git a/crates/uv-workspace/src/workspace.rs b/crates/uv-workspace/src/workspace.rs index f8cf4053a3913..db42408d69c89 100644 --- a/crates/uv-workspace/src/workspace.rs +++ b/crates/uv-workspace/src/workspace.rs @@ -560,7 +560,7 @@ impl Workspace { Some(workspace.install_path.join(path)) } - // Resolve the `VIRTUAL_ENV` variable, if any. + /// Resolve the `VIRTUAL_ENV` variable, if any. fn from_virtual_env_variable() -> Option { let value = std::env::var_os(EnvVars::VIRTUAL_ENV)?; @@ -578,38 +578,14 @@ impl Workspace { Some(CWD.join(path)) } - // Attempt to check if the two paths refer to the same directory. - fn is_same_dir(left: &Path, right: &Path) -> Option { - // First, attempt to check directly - if let Ok(value) = same_file::is_same_file(left, right) { - return Some(value); - }; - - // Often, one of the directories won't exist yet so perform the comparison up a level - if let (Some(left_parent), Some(right_parent), Some(left_name), Some(right_name)) = ( - left.parent(), - right.parent(), - left.file_name(), - right.file_name(), - ) { - match same_file::is_same_file(left_parent, right_parent) { - Ok(true) => return Some(left_name == right_name), - Ok(false) => return Some(false), - _ => (), - } - }; - - // We couldn't determine if they're the same - None - } - // Determine the default value let project_env = from_project_environment_variable(self) .unwrap_or_else(|| self.install_path.join(".venv")); // Warn if it conflicts with `VIRTUAL_ENV` if let Some(from_virtual_env) = from_virtual_env_variable() { - if !is_same_dir(&from_virtual_env, &project_env).unwrap_or(false) { + if !uv_fs::is_same_file_allow_missing(&from_virtual_env, &project_env).unwrap_or(false) + { match active { Some(true) => { debug!( diff --git a/crates/uv/src/commands/project/add.rs b/crates/uv/src/commands/project/add.rs index b217d83bc7341..e8f978aac2b12 100644 --- a/crates/uv/src/commands/project/add.rs +++ b/crates/uv/src/commands/project/add.rs @@ -165,6 +165,7 @@ pub(crate) async fn add( allow_insecure_host, &install_mirrors, no_config, + active, cache, printer, ) diff --git a/crates/uv/src/commands/project/export.rs b/crates/uv/src/commands/project/export.rs index 6ea2a02156de6..6f70322544728 100644 --- a/crates/uv/src/commands/project/export.rs +++ b/crates/uv/src/commands/project/export.rs @@ -147,6 +147,7 @@ pub(crate) async fn export( allow_insecure_host, &install_mirrors, no_config, + Some(false), cache, printer, ) diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs index f613c3cebc692..a457e81ab3f3e 100644 --- a/crates/uv/src/commands/project/lock.rs +++ b/crates/uv/src/commands/project/lock.rs @@ -137,6 +137,7 @@ pub(crate) async fn lock( allow_insecure_host, &install_mirrors, no_config, + Some(false), cache, printer, ) diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index 1de0d82c8ee3f..df62a2530ebb2 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -40,6 +40,7 @@ use uv_resolver::{ }; use uv_scripts::Pep723ItemRef; use uv_settings::PythonInstallMirrors; +use uv_static::EnvVars; use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy}; use uv_warnings::{warn_user, warn_user_once}; use uv_workspace::dependency_groups::DependencyGroupError; @@ -532,30 +533,88 @@ pub(crate) enum ScriptInterpreter { impl ScriptInterpreter { /// Return the expected virtual environment path for the [`Pep723Script`]. - pub(crate) fn root(script: Pep723ItemRef<'_>, cache: &Cache) -> PathBuf { - let entry = match script { - // For local scripts, use a hash of the path to the script. - Pep723ItemRef::Script(script) => { - let digest = cache_digest(&script.path); - if let Some(file_name) = script - .path - .file_stem() - .and_then(|name| name.to_str()) - .and_then(cache_name) - { - format!("{file_name}-{digest}") - } else { - digest + /// + /// If `--active` is set, the active virtual environment will be preferred. + /// + /// See: [`Workspace::venv`]. + pub(crate) fn root(script: Pep723ItemRef<'_>, active: Option, cache: &Cache) -> PathBuf { + /// Resolve the `VIRTUAL_ENV` variable, if any. + fn from_virtual_env_variable() -> Option { + let value = std::env::var_os(EnvVars::VIRTUAL_ENV)?; + + if value.is_empty() { + return None; + }; + + let path = PathBuf::from(value); + if path.is_absolute() { + return Some(path); + }; + + // Resolve the path relative to current directory. + Some(CWD.join(path)) + } + + // Determine the stable path to the script environment in the cache. + let cache_env = { + let entry = match script { + // For local scripts, use a hash of the path to the script. + Pep723ItemRef::Script(script) => { + let digest = cache_digest(&script.path); + if let Some(file_name) = script + .path + .file_stem() + .and_then(|name| name.to_str()) + .and_then(cache_name) + { + format!("{file_name}-{digest}") + } else { + digest + } } - } - // For remote scripts, use a hash of the URL. - Pep723ItemRef::Remote(.., url) => cache_digest(url), - // Otherwise, use a hash of the metadata. - Pep723ItemRef::Stdin(metadata) => cache_digest(&metadata.raw), + // For remote scripts, use a hash of the URL. + Pep723ItemRef::Remote(.., url) => cache_digest(url), + // Otherwise, use a hash of the metadata. + Pep723ItemRef::Stdin(metadata) => cache_digest(&metadata.raw), + }; + + cache + .shard(CacheBucket::Environments, entry) + .into_path_buf() }; - cache - .shard(CacheBucket::Environments, entry) - .into_path_buf() + + // If `--active` is set, prefer the active virtual environment. + if let Some(from_virtual_env) = from_virtual_env_variable() { + if !uv_fs::is_same_file_allow_missing(&from_virtual_env, &cache_env).unwrap_or(false) { + match active { + Some(true) => { + debug!( + "Using active virtual environment `{}` instead of script environment `{}`", + from_virtual_env.user_display(), + cache_env.user_display() + ); + return from_virtual_env; + } + Some(false) => {} + None => { + warn_user_once!( + "`VIRTUAL_ENV={}` does not match the script environment path `{}` and will be ignored; use `--active` to target the active environment instead", + from_virtual_env.user_display(), + cache_env.user_display() + ); + } + } + } + } else { + if active.unwrap_or_default() { + debug!( + "Use of the active virtual environment was requested, but `VIRTUAL_ENV` is not set" + ); + } + } + + // Otherwise, use the cache root. + cache_env } /// Discover the interpreter to use for the current [`Pep723Item`]. @@ -569,6 +628,7 @@ impl ScriptInterpreter { allow_insecure_host: &[TrustedHost], install_mirrors: &PythonInstallMirrors, no_config: bool, + active: Option, cache: &Cache, printer: Printer, ) -> Result { @@ -581,7 +641,8 @@ impl ScriptInterpreter { requires_python, } = ScriptPython::from_request(python_request, workspace, script, no_config).await?; - let root = Self::root(script, cache); + let root = Self::root(script, active, cache); + match PythonEnvironment::from_root(&root, cache) { Ok(venv) => { if python_request.as_ref().map_or(true, |request| { @@ -1279,6 +1340,7 @@ impl ScriptEnvironment { allow_insecure_host: &[TrustedHost], install_mirrors: &PythonInstallMirrors, no_config: bool, + active: Option, cache: &Cache, dry_run: DryRun, printer: Printer, @@ -1296,6 +1358,7 @@ impl ScriptEnvironment { allow_insecure_host, install_mirrors, no_config, + active, cache, printer, ) @@ -1306,7 +1369,7 @@ impl ScriptEnvironment { // Otherwise, create a virtual environment with the discovered interpreter. ScriptInterpreter::Interpreter(interpreter) => { - let root = ScriptInterpreter::root(script, cache); + let root = ScriptInterpreter::root(script, active, cache); // Determine a prompt for the environment, in order of preference: // diff --git a/crates/uv/src/commands/project/remove.rs b/crates/uv/src/commands/project/remove.rs index 70c887fb8d87e..a7f17c7d5c919 100644 --- a/crates/uv/src/commands/project/remove.rs +++ b/crates/uv/src/commands/project/remove.rs @@ -253,6 +253,7 @@ pub(crate) async fn remove( allow_insecure_host, &install_mirrors, no_config, + active, cache, printer, ) diff --git a/crates/uv/src/commands/project/run.rs b/crates/uv/src/commands/project/run.rs index 3d034fc3ed37e..337c758aa9a8c 100644 --- a/crates/uv/src/commands/project/run.rs +++ b/crates/uv/src/commands/project/run.rs @@ -226,6 +226,7 @@ hint: If you are running a script with `{}` in the shebang, you may need to incl allow_insecure_host, &install_mirrors, no_config, + active.map_or(Some(false), Some), cache, DryRun::Disabled, printer, @@ -346,6 +347,7 @@ hint: If you are running a script with `{}` in the shebang, you may need to incl allow_insecure_host, &install_mirrors, no_config, + active.map_or(Some(false), Some), cache, DryRun::Disabled, printer, @@ -402,6 +404,7 @@ hint: If you are running a script with `{}` in the shebang, you may need to incl allow_insecure_host, &install_mirrors, no_config, + active, cache, printer, ) diff --git a/crates/uv/src/commands/project/sync.rs b/crates/uv/src/commands/project/sync.rs index 4b1ec5043e1ce..5dc241b0c398d 100644 --- a/crates/uv/src/commands/project/sync.rs +++ b/crates/uv/src/commands/project/sync.rs @@ -172,6 +172,7 @@ pub(crate) async fn sync( allow_insecure_host, &install_mirrors, no_config, + active, cache, dry_run, printer, diff --git a/crates/uv/src/commands/project/tree.rs b/crates/uv/src/commands/project/tree.rs index 57857ea6f71f0..756e207ade712 100644 --- a/crates/uv/src/commands/project/tree.rs +++ b/crates/uv/src/commands/project/tree.rs @@ -102,6 +102,7 @@ pub(crate) async fn tree( allow_insecure_host, &install_mirrors, no_config, + Some(false), cache, printer, ) diff --git a/crates/uv/tests/it/run.rs b/crates/uv/tests/it/run.rs index 05b6915564e7a..d3be93db82e65 100644 --- a/crates/uv/tests/it/run.rs +++ b/crates/uv/tests/it/run.rs @@ -3514,7 +3514,7 @@ fn run_linked_environment_path() -> Result<()> { } #[test] -fn run_active_environment() -> Result<()> { +fn run_active_project_environment() -> Result<()> { let context = TestContext::new_with_versions(&["3.11", "3.12"]) .with_filtered_virtualenv_bin() .with_filtered_python_names(); @@ -3620,6 +3620,115 @@ fn run_active_environment() -> Result<()> { Ok(()) } +#[test] +fn run_active_script_environment() -> Result<()> { + let context = TestContext::new_with_versions(&["3.11", "3.12"]) + .with_filtered_virtualenv_bin() + .with_filtered_python_names(); + + let test_script = context.temp_dir.child("main.py"); + test_script.write_str(indoc! { r#" + # /// script + # requires-python = ">=3.11" + # dependencies = [ + # "iniconfig", + # ] + # /// + + import iniconfig + + print("Hello, world!") + "# + })?; + + let filters = context + .filters() + .into_iter() + .chain(vec![( + r"environments-v1/main-\w+", + "environments-v1/main-[HASH]", + )]) + .collect::>(); + + // Running `uv run --script` with `VIRTUAL_ENV` should _not_ warn. + uv_snapshot!(&filters, context.run() + .arg("--script") + .arg("main.py") + .env(EnvVars::VIRTUAL_ENV, "foo"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + Hello, world! + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + iniconfig==2.0.0 + "###); + + // Using `--no-active` should also _not_ warn. + uv_snapshot!(&filters, context.run() + .arg("--no-active") + .arg("--script") + .arg("main.py") + .env(EnvVars::VIRTUAL_ENV, "foo"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + Hello, world! + + ----- stderr ----- + "###); + + context + .temp_dir + .child("foo") + .assert(predicate::path::missing()); + + // Using `--active` should create the environment + uv_snapshot!(&filters, context.run() + .arg("--active") + .arg("--script") + .arg("main.py") + .env(EnvVars::VIRTUAL_ENV, "foo"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + Hello, world! + + ----- stderr ----- + Resolved 1 package in [TIME] + Installed 1 package in [TIME] + + iniconfig==2.0.0 + "###); + + context + .temp_dir + .child("foo") + .assert(predicate::path::is_dir()); + + // Requesting a different Python version should invalidate the environment + uv_snapshot!(&filters, context.run() + .arg("--active") + .arg("-p").arg("3.12") + .arg("--script") + .arg("main.py") + .env(EnvVars::VIRTUAL_ENV, "foo"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + Hello, world! + + ----- stderr ----- + Resolved 1 package in [TIME] + Installed 1 package in [TIME] + + iniconfig==2.0.0 + "###); + + Ok(()) +} + #[test] #[cfg(not(windows))] fn run_gui_script_explicit_stdin_unix() -> Result<()> { diff --git a/crates/uv/tests/it/sync.rs b/crates/uv/tests/it/sync.rs index 755ed3eeba8d2..447d566efa677 100644 --- a/crates/uv/tests/it/sync.rs +++ b/crates/uv/tests/it/sync.rs @@ -3400,7 +3400,7 @@ fn sync_custom_environment_path() -> Result<()> { } #[test] -fn sync_active_environment() -> Result<()> { +fn sync_active_project_environment() -> Result<()> { let context = TestContext::new_with_versions(&["3.11", "3.12"]) .with_filtered_virtualenv_bin() .with_filtered_python_names(); @@ -3524,6 +3524,110 @@ fn sync_active_environment() -> Result<()> { Ok(()) } +#[test] +fn sync_active_script_environment() -> Result<()> { + let context = TestContext::new_with_versions(&["3.11", "3.12"]) + .with_filtered_virtualenv_bin() + .with_filtered_python_names(); + + let script = context.temp_dir.child("script.py"); + script.write_str(indoc! { r#" + # /// script + # requires-python = ">=3.11" + # dependencies = [ + # "anyio", + # ] + # /// + + import anyio + "# + })?; + + let filters = context + .filters() + .into_iter() + .chain(vec![( + r"environments-v1/script-\w+", + "environments-v1/script-[HASH]", + )]) + .collect::>(); + + // Running `uv sync --script` with `VIRTUAL_ENV` should warn + uv_snapshot!(&filters, context.sync().arg("--script").arg("script.py").env(EnvVars::VIRTUAL_ENV, "foo"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + warning: `VIRTUAL_ENV=foo` does not match the script environment path `[CACHE_DIR]/environments-v1/script-[HASH]` and will be ignored; use `--active` to target the active environment instead + Creating script environment at: [CACHE_DIR]/environments-v1/script-[HASH] + Resolved 3 packages in [TIME] + Prepared 3 packages in [TIME] + Installed 3 packages in [TIME] + + anyio==4.3.0 + + idna==3.6 + + sniffio==1.3.1 + "###); + + context + .temp_dir + .child("foo") + .assert(predicate::path::missing()); + + // Using `--active` should create the environment + uv_snapshot!(&filters, context.sync().arg("--script").arg("script.py").env(EnvVars::VIRTUAL_ENV, "foo").arg("--active"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Creating script environment at: foo + Resolved 3 packages in [TIME] + Installed 3 packages in [TIME] + + anyio==4.3.0 + + idna==3.6 + + sniffio==1.3.1 + "###); + + context + .temp_dir + .child("foo") + .assert(predicate::path::is_dir()); + + // A subsequent sync will re-use the environment + uv_snapshot!(&filters, context.sync().arg("--script").arg("script.py").env(EnvVars::VIRTUAL_ENV, "foo").arg("--active"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Using script environment at: foo + "###); + + // Requesting another Python version will invalidate the environment + uv_snapshot!(&filters, context.sync() + .arg("--script") + .arg("script.py") + .env(EnvVars::VIRTUAL_ENV, "foo") + .arg("--active") + .arg("-p") + .arg("3.12"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Recreating script environment at: foo + Resolved 3 packages in [TIME] + Installed 3 packages in [TIME] + + anyio==4.3.0 + + idna==3.6 + + sniffio==1.3.1 + "###); + + Ok(()) +} + #[test] #[cfg(feature = "git")] fn sync_workspace_custom_environment_path() -> Result<()> { diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 8d6be84c413b4..4e409ce503108 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -1486,9 +1486,9 @@ uv sync [OPTIONS]

Options

-
--active

Prefer the active virtual environment over the project’s virtual environment.

+
--active

Prefer the active virtual environment over the project or script’s virtual environment.

-

If the project virtual environment is active or no virtual environment is active, this has no effect.

+

If the project or script virtual environment is active (or no virtual environment is active), this has no effect.

--all-extras

Include all optional dependencies.