Skip to content

Commit

Permalink
Support --active for PEP 723 script environments
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Feb 12, 2025
1 parent c60b315 commit cafb711
Show file tree
Hide file tree
Showing 14 changed files with 229 additions and 53 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: 0 additions & 1 deletion crates/uv-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3142,7 +3142,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",
Expand Down
1 change: 1 addition & 0 deletions crates/uv-fs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
25 changes: 25 additions & 0 deletions crates/uv-fs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,31 @@ pub mod cachedir;
mod path;
pub mod which;

/// Attempt to check if the two paths refer to the same directory.
pub 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
}

/// 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
Expand Down
29 changes: 2 additions & 27 deletions crates/uv-workspace/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PathBuf> {
let value = std::env::var_os(EnvVars::VIRTUAL_ENV)?;

Expand All @@ -578,38 +578,13 @@ 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<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) {
if !uv_fs::is_same_dir(&from_virtual_env, &project_env).unwrap_or(false) {
match active {
Some(true) => {
debug!(
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/commands/project/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ pub(crate) async fn add(
allow_insecure_host,
&install_mirrors,
no_config,
active,
cache,
printer,
)
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/commands/project/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ pub(crate) async fn export(
allow_insecure_host,
&install_mirrors,
no_config,
Some(false),
cache,
printer,
)
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/commands/project/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ pub(crate) async fn lock(
allow_insecure_host,
&install_mirrors,
no_config,
Some(false),
cache,
printer,
)
Expand Down
110 changes: 86 additions & 24 deletions crates/uv/src/commands/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -532,30 +533,87 @@ 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<bool>, cache: &Cache) -> PathBuf {
/// Resolve the `VIRTUAL_ENV` variable, if any.
fn from_virtual_env_variable() -> Option<PathBuf> {
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))
}

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_dir(&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`].
Expand All @@ -569,6 +627,7 @@ impl ScriptInterpreter {
allow_insecure_host: &[TrustedHost],
install_mirrors: &PythonInstallMirrors,
no_config: bool,
active: Option<bool>,
cache: &Cache,
printer: Printer,
) -> Result<Self, ProjectError> {
Expand All @@ -581,7 +640,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| {
Expand Down Expand Up @@ -1279,6 +1339,7 @@ impl ScriptEnvironment {
allow_insecure_host: &[TrustedHost],
install_mirrors: &PythonInstallMirrors,
no_config: bool,
active: Option<bool>,
cache: &Cache,
dry_run: DryRun,
printer: Printer,
Expand All @@ -1296,6 +1357,7 @@ impl ScriptEnvironment {
allow_insecure_host,
install_mirrors,
no_config,
active,
cache,
printer,
)
Expand All @@ -1306,7 +1368,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:
//
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/commands/project/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ pub(crate) async fn remove(
allow_insecure_host,
&install_mirrors,
no_config,
active,
cache,
printer,
)
Expand Down
3 changes: 3 additions & 0 deletions crates/uv/src/commands/project/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ pub(crate) async fn run(
allow_insecure_host,
&install_mirrors,
no_config,
active,
cache,
DryRun::Disabled,
printer,
Expand Down Expand Up @@ -329,6 +330,7 @@ pub(crate) async fn run(
allow_insecure_host,
&install_mirrors,
no_config,
active,
cache,
DryRun::Disabled,
printer,
Expand Down Expand Up @@ -385,6 +387,7 @@ pub(crate) async fn run(
allow_insecure_host,
&install_mirrors,
no_config,
active,
cache,
printer,
)
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/commands/project/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ pub(crate) async fn sync(
allow_insecure_host,
&install_mirrors,
no_config,
active,
cache,
dry_run,
printer,
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/commands/project/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ pub(crate) async fn tree(
allow_insecure_host,
&install_mirrors,
no_config,
Some(false),
cache,
printer,
)
Expand Down
Loading

0 comments on commit cafb711

Please sign in to comment.