Skip to content

Commit

Permalink
Add warning when VIRTUAL_ENV is set but will not be respected in pr…
Browse files Browse the repository at this point in the history
…oject commands (#6864)

Following #6834
  • Loading branch information
zanieb authored Sep 3, 2024
1 parent 88ba1e9 commit 2a42515
Show file tree
Hide file tree
Showing 7 changed files with 219 additions and 5 deletions.
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-workspace/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ either = { workspace = true }
fs-err = { workspace = true }
glob = { workspace = true }
rustc-hash = { workspace = true }
same-file = { workspace = true }
schemars = { workspace = true, optional = true }
serde = { workspace = true, features = ["derive"] }
thiserror = { workspace = true }
Expand Down
67 changes: 62 additions & 5 deletions crates/uv-workspace/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ use tracing::{debug, trace, warn};

use pep508_rs::{MarkerTree, RequirementOrigin, VerbatimUrl};
use pypi_types::{Requirement, RequirementSource, SupportedEnvironments, VerbatimParsedUrl};
use uv_fs::Simplified;
use uv_fs::{Simplified, CWD};
use uv_normalize::{GroupName, PackageName, DEV_DEPENDENCIES};
use uv_warnings::warn_user;
use uv_warnings::{warn_user, warn_user_once};

use crate::pyproject::{Project, PyProjectToml, Source, ToolUvWorkspace};

Expand Down Expand Up @@ -442,7 +442,7 @@ impl Workspace {
/// it is resolved relative to the install path.
pub fn venv(&self) -> PathBuf {
/// Resolve the `UV_PROJECT_ENVIRONMENT` value, if any.
fn from_environment_variable(workspace: &Workspace) -> Option<PathBuf> {
fn from_project_environment_variable(workspace: &Workspace) -> Option<PathBuf> {
let value = std::env::var_os("UV_PROJECT_ENVIRONMENT")?;

if value.is_empty() {
Expand All @@ -458,8 +458,65 @@ impl Workspace {
Some(workspace.install_path.join(path))
}

// TODO(zanieb): Warn if `VIRTUAL_ENV` is set and does not match
from_environment_variable(self).unwrap_or_else(|| self.install_path.join(".venv"))
// Resolve the `VIRTUAL_ENV` variable, if any.
fn from_virtual_env_variable() -> Option<PathBuf> {
let value = std::env::var_os("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.
// Note this differs from `UV_PROJECT_ENVIRONMENT`
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<bool> {
// 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) {
warn_user_once!(
"`VIRTUAL_ENV={}` does not match the project environment path `{}` and will be ignored",
from_virtual_env.user_display(),
project_env.user_display()
);
}
}

project_env
}

/// The members of the workspace.
Expand Down
3 changes: 3 additions & 0 deletions crates/uv/tests/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ fn init_application() -> Result<()> {
Hello from foo!
----- stderr -----
warning: `VIRTUAL_ENV=[VENV]/` does not match the project environment path `.venv` and will be ignored
Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
Creating virtualenv at: .venv
Resolved 1 package in [TIME]
Expand Down Expand Up @@ -306,6 +307,7 @@ fn init_application_package() -> Result<()> {
Hello from foo!
----- stderr -----
warning: `VIRTUAL_ENV=[VENV]/` does not match the project environment path `.venv` and will be ignored
Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
Creating virtualenv at: .venv
Resolved 1 package in [TIME]
Expand Down Expand Up @@ -377,6 +379,7 @@ fn init_library() -> Result<()> {
Hello from foo!
----- stderr -----
warning: `VIRTUAL_ENV=[VENV]/` does not match the project environment path `.venv` and will be ignored
Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
Creating virtualenv at: .venv
Resolved 1 package in [TIME]
Expand Down
1 change: 1 addition & 0 deletions crates/uv/tests/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1307,6 +1307,7 @@ fn run_from_directory() -> Result<()> {
3.12.[X]
----- stderr -----
warning: `VIRTUAL_ENV=[VENV]/` does not match the project environment path `.venv` and will be ignored
Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
Creating virtualenv at: .venv
Resolved 1 package in [TIME]
Expand Down
144 changes: 144 additions & 0 deletions crates/uv/tests/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1747,3 +1747,147 @@ fn sync_workspace_custom_environment_path() -> Result<()> {

Ok(())
}

// Test for warnings when `VIRTUAL_ENV` is set but will not be respected.
#[test]
fn sync_virtual_env_warning() -> Result<()> {
let context = TestContext::new("3.12");

let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["iniconfig"]
"#,
)?;

// We should not warn if it matches the project environment
uv_snapshot!(context.filters(), context.sync().env("VIRTUAL_ENV", context.temp_dir.join(".venv")), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 2 packages in [TIME]
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ iniconfig==2.0.0
"###);

// Including if it's a relative path that matches
uv_snapshot!(context.filters(), context.sync().env("VIRTUAL_ENV", ".venv"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 2 packages in [TIME]
Audited 1 package in [TIME]
"###);

// Or, if it's a link that resolves to the same path
#[cfg(unix)]
{
use fs_err::os::unix::fs::symlink;

let link = context.temp_dir.join("link");
symlink(context.temp_dir.join(".venv"), &link)?;

uv_snapshot!(context.filters(), context.sync().env("VIRTUAL_ENV", link), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 2 packages in [TIME]
Audited 1 package in [TIME]
"###);
}

// But we should warn if it's a different path
uv_snapshot!(context.filters(), context.sync().env("VIRTUAL_ENV", "foo"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `VIRTUAL_ENV=foo` does not match the project environment path `.venv` and will be ignored
Resolved 2 packages in [TIME]
Audited 1 package in [TIME]
"###);

// Including absolute paths
uv_snapshot!(context.filters(), context.sync().env("VIRTUAL_ENV", context.temp_dir.join("foo")), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `VIRTUAL_ENV=foo` does not match the project environment path `.venv` and will be ignored
Resolved 2 packages in [TIME]
Audited 1 package in [TIME]
"###);

// We should not warn if the project environment has been customized and matches
uv_snapshot!(context.filters(), context.sync().env("VIRTUAL_ENV", "foo").env("UV_PROJECT_ENVIRONMENT", "foo"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
Creating virtualenv at: foo
Resolved 2 packages in [TIME]
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ iniconfig==2.0.0
"###);

// But we should warn if they don't match still
uv_snapshot!(context.filters(), context.sync().env("VIRTUAL_ENV", "foo").env("UV_PROJECT_ENVIRONMENT", "bar"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `VIRTUAL_ENV=foo` does not match the project environment path `bar` and will be ignored
Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
Creating virtualenv at: bar
Resolved 2 packages in [TIME]
Prepared 1 package in [TIME]
Installed 1 package in [TIME]
+ iniconfig==2.0.0
"###);

let child = context.temp_dir.child("child");
child.create_dir_all()?;

// And `VIRTUAL_ENV` is resolved relative to the project root so with relative paths we should
// warn from a child too
uv_snapshot!(context.filters(), context.sync().env("VIRTUAL_ENV", "foo").env("UV_PROJECT_ENVIRONMENT", "foo").current_dir(&child), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: `VIRTUAL_ENV=foo` does not match the project environment path `[TEMP_DIR]/foo` and will be ignored
Resolved 2 packages in [TIME]
Audited 1 package in [TIME]
"###);

// But, a matching absolute path shouldn't warn
uv_snapshot!(context.filters(), context.sync().env("VIRTUAL_ENV", context.temp_dir.join("foo")).env("UV_PROJECT_ENVIRONMENT", "foo").current_dir(&child), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 2 packages in [TIME]
Audited 1 package in [TIME]
"###);

Ok(())
}
7 changes: 7 additions & 0 deletions crates/uv/tests/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ fn test_uv_run_with_package_virtual_workspace() -> Result<()> {
Success
----- stderr -----
warning: `VIRTUAL_ENV=[VENV]/` does not match the project environment path `.venv` and will be ignored
Using Python 3.12.[X] interpreter at: [PYTHON]
Creating virtualenv at: .venv
Resolved 8 packages in [TIME]
Expand All @@ -402,6 +403,7 @@ fn test_uv_run_with_package_virtual_workspace() -> Result<()> {
Success
----- stderr -----
warning: `VIRTUAL_ENV=[VENV]/` does not match the project environment path `.venv` and will be ignored
Resolved 8 packages in [TIME]
Prepared 2 packages in [TIME]
Installed 2 packages in [TIME]
Expand Down Expand Up @@ -435,6 +437,7 @@ fn test_uv_run_virtual_workspace_root() -> Result<()> {
Success
----- stderr -----
warning: `VIRTUAL_ENV=[VENV]/` does not match the project environment path `.venv` and will be ignored
Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
Creating virtualenv at: .venv
Resolved 8 packages in [TIME]
Expand Down Expand Up @@ -479,6 +482,7 @@ fn test_uv_run_with_package_root_workspace() -> Result<()> {
Success
----- stderr -----
warning: `VIRTUAL_ENV=[VENV]/` does not match the project environment path `.venv` and will be ignored
Using Python 3.12.[X] interpreter at: [PYTHON]
Creating virtualenv at: .venv
Resolved 8 packages in [TIME]
Expand All @@ -504,6 +508,7 @@ fn test_uv_run_with_package_root_workspace() -> Result<()> {
Success
----- stderr -----
warning: `VIRTUAL_ENV=[VENV]/` does not match the project environment path `.venv` and will be ignored
Resolved 8 packages in [TIME]
Prepared 2 packages in [TIME]
Installed 2 packages in [TIME]
Expand Down Expand Up @@ -542,6 +547,7 @@ fn test_uv_run_isolate() -> Result<()> {
Success
----- stderr -----
warning: `VIRTUAL_ENV=[VENV]/` does not match the project environment path `.venv` and will be ignored
Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
Creating virtualenv at: .venv
Resolved 8 packages in [TIME]
Expand Down Expand Up @@ -572,6 +578,7 @@ fn test_uv_run_isolate() -> Result<()> {
Success
----- stderr -----
warning: `VIRTUAL_ENV=[VENV]/` does not match the project environment path `.venv` and will be ignored
Resolved 8 packages in [TIME]
Audited 5 packages in [TIME]
"###
Expand Down

0 comments on commit 2a42515

Please sign in to comment.