Skip to content

Commit ccd9c30

Browse files
charliermarshLoïc LESCOAT
charliermarsh
authored and
Loïc LESCOAT
committed
Support --active for PEP 723 script environments (astral-sh#11433)
## Summary See: astral-sh#11361 (comment)
1 parent a22145f commit ccd9c30

File tree

17 files changed

+354
-61
lines changed

17 files changed

+354
-61
lines changed

‎Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/uv-cli/src/lib.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3063,10 +3063,11 @@ pub struct SyncArgs {
30633063
#[arg(long, overrides_with("inexact"), hide = true)]
30643064
pub exact: bool,
30653065

3066-
/// Prefer the active virtual environment over the project's virtual environment.
3066+
/// Sync dependencies to the active virtual environment.
30673067
///
3068-
/// If the project virtual environment is active or no virtual environment is active, this has
3069-
/// no effect.
3068+
/// Instead of creating or updating the virtual environment for the project or script, the
3069+
/// active virtual environment will be preferred, if the `VIRTUAL_ENV` environment variable is
3070+
/// set.
30703071
#[arg(long, overrides_with = "no_active")]
30713072
pub active: bool,
30723073

@@ -3160,7 +3161,6 @@ pub struct SyncArgs {
31603161
/// adherence with PEP 723.
31613162
#[arg(
31623163
long,
3163-
conflicts_with = "active",
31643164
conflicts_with = "all_packages",
31653165
conflicts_with = "package",
31663166
conflicts_with = "no_install_project",

‎crates/uv-fs/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ fs-err = { workspace = true }
2323
fs2 = { workspace = true }
2424
path-slash = { workspace = true }
2525
percent-encoding = { workspace = true }
26+
same-file = { workspace = true }
2627
schemars = { workspace = true, optional = true }
2728
serde = { workspace = true, optional = true }
2829
tempfile = { workspace = true }

‎crates/uv-fs/src/lib.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,38 @@ pub mod cachedir;
1111
mod path;
1212
pub mod which;
1313

14+
/// Attempt to check if the two paths refer to the same file.
15+
///
16+
/// Returns `Some(true)` if the files are missing, but would be the same if they existed.
17+
pub fn is_same_file_allow_missing(left: &Path, right: &Path) -> Option<bool> {
18+
// First, check an exact path comparison.
19+
if left == right {
20+
return Some(true);
21+
}
22+
23+
// Second, check the files directly.
24+
if let Ok(value) = same_file::is_same_file(left, right) {
25+
return Some(value);
26+
};
27+
28+
// Often, one of the directories won't exist yet so perform the comparison up a level.
29+
if let (Some(left_parent), Some(right_parent), Some(left_name), Some(right_name)) = (
30+
left.parent(),
31+
right.parent(),
32+
left.file_name(),
33+
right.file_name(),
34+
) {
35+
match same_file::is_same_file(left_parent, right_parent) {
36+
Ok(true) => return Some(left_name == right_name),
37+
Ok(false) => return Some(false),
38+
_ => (),
39+
}
40+
};
41+
42+
// We couldn't determine if they're the same.
43+
None
44+
}
45+
1446
/// Reads data from the path and requires that it be valid UTF-8 or UTF-16.
1547
///
1648
/// This uses BOM sniffing to determine if the data should be transcoded

‎crates/uv-workspace/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ glob = { workspace = true }
3434
itertools = { workspace = true }
3535
owo-colors = { workspace = true }
3636
rustc-hash = { workspace = true }
37-
same-file = { workspace = true }
3837
schemars = { workspace = true, optional = true }
3938
serde = { workspace = true, features = ["derive"] }
4039
thiserror = { workspace = true }

‎crates/uv-workspace/src/workspace.rs

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ impl Workspace {
560560
Some(workspace.install_path.join(path))
561561
}
562562

563-
// Resolve the `VIRTUAL_ENV` variable, if any.
563+
/// Resolve the `VIRTUAL_ENV` variable, if any.
564564
fn from_virtual_env_variable() -> Option<PathBuf> {
565565
let value = std::env::var_os(EnvVars::VIRTUAL_ENV)?;
566566

@@ -578,38 +578,14 @@ impl Workspace {
578578
Some(CWD.join(path))
579579
}
580580

581-
// Attempt to check if the two paths refer to the same directory.
582-
fn is_same_dir(left: &Path, right: &Path) -> Option<bool> {
583-
// First, attempt to check directly
584-
if let Ok(value) = same_file::is_same_file(left, right) {
585-
return Some(value);
586-
};
587-
588-
// Often, one of the directories won't exist yet so perform the comparison up a level
589-
if let (Some(left_parent), Some(right_parent), Some(left_name), Some(right_name)) = (
590-
left.parent(),
591-
right.parent(),
592-
left.file_name(),
593-
right.file_name(),
594-
) {
595-
match same_file::is_same_file(left_parent, right_parent) {
596-
Ok(true) => return Some(left_name == right_name),
597-
Ok(false) => return Some(false),
598-
_ => (),
599-
}
600-
};
601-
602-
// We couldn't determine if they're the same
603-
None
604-
}
605-
606581
// Determine the default value
607582
let project_env = from_project_environment_variable(self)
608583
.unwrap_or_else(|| self.install_path.join(".venv"));
609584

610585
// Warn if it conflicts with `VIRTUAL_ENV`
611586
if let Some(from_virtual_env) = from_virtual_env_variable() {
612-
if !is_same_dir(&from_virtual_env, &project_env).unwrap_or(false) {
587+
if !uv_fs::is_same_file_allow_missing(&from_virtual_env, &project_env).unwrap_or(false)
588+
{
613589
match active {
614590
Some(true) => {
615591
debug!(

‎crates/uv/src/commands/project/add.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ pub(crate) async fn add(
165165
allow_insecure_host,
166166
&install_mirrors,
167167
no_config,
168+
active,
168169
cache,
169170
printer,
170171
)

‎crates/uv/src/commands/project/export.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ pub(crate) async fn export(
147147
allow_insecure_host,
148148
&install_mirrors,
149149
no_config,
150+
Some(false),
150151
cache,
151152
printer,
152153
)

‎crates/uv/src/commands/project/lock.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ pub(crate) async fn lock(
137137
allow_insecure_host,
138138
&install_mirrors,
139139
no_config,
140+
Some(false),
140141
cache,
141142
printer,
142143
)

‎crates/uv/src/commands/project/mod.rs

Lines changed: 87 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ use uv_resolver::{
4040
};
4141
use uv_scripts::Pep723ItemRef;
4242
use uv_settings::PythonInstallMirrors;
43+
use uv_static::EnvVars;
4344
use uv_types::{BuildIsolation, EmptyInstalledPackages, HashStrategy};
4445
use uv_warnings::{warn_user, warn_user_once};
4546
use uv_workspace::dependency_groups::DependencyGroupError;
@@ -532,30 +533,88 @@ pub(crate) enum ScriptInterpreter {
532533

533534
impl ScriptInterpreter {
534535
/// Return the expected virtual environment path for the [`Pep723Script`].
535-
pub(crate) fn root(script: Pep723ItemRef<'_>, cache: &Cache) -> PathBuf {
536-
let entry = match script {
537-
// For local scripts, use a hash of the path to the script.
538-
Pep723ItemRef::Script(script) => {
539-
let digest = cache_digest(&script.path);
540-
if let Some(file_name) = script
541-
.path
542-
.file_stem()
543-
.and_then(|name| name.to_str())
544-
.and_then(cache_name)
545-
{
546-
format!("{file_name}-{digest}")
547-
} else {
548-
digest
536+
///
537+
/// If `--active` is set, the active virtual environment will be preferred.
538+
///
539+
/// See: [`Workspace::venv`].
540+
pub(crate) fn root(script: Pep723ItemRef<'_>, active: Option<bool>, cache: &Cache) -> PathBuf {
541+
/// Resolve the `VIRTUAL_ENV` variable, if any.
542+
fn from_virtual_env_variable() -> Option<PathBuf> {
543+
let value = std::env::var_os(EnvVars::VIRTUAL_ENV)?;
544+
545+
if value.is_empty() {
546+
return None;
547+
};
548+
549+
let path = PathBuf::from(value);
550+
if path.is_absolute() {
551+
return Some(path);
552+
};
553+
554+
// Resolve the path relative to current directory.
555+
Some(CWD.join(path))
556+
}
557+
558+
// Determine the stable path to the script environment in the cache.
559+
let cache_env = {
560+
let entry = match script {
561+
// For local scripts, use a hash of the path to the script.
562+
Pep723ItemRef::Script(script) => {
563+
let digest = cache_digest(&script.path);
564+
if let Some(file_name) = script
565+
.path
566+
.file_stem()
567+
.and_then(|name| name.to_str())
568+
.and_then(cache_name)
569+
{
570+
format!("{file_name}-{digest}")
571+
} else {
572+
digest
573+
}
549574
}
550-
}
551-
// For remote scripts, use a hash of the URL.
552-
Pep723ItemRef::Remote(.., url) => cache_digest(url),
553-
// Otherwise, use a hash of the metadata.
554-
Pep723ItemRef::Stdin(metadata) => cache_digest(&metadata.raw),
575+
// For remote scripts, use a hash of the URL.
576+
Pep723ItemRef::Remote(.., url) => cache_digest(url),
577+
// Otherwise, use a hash of the metadata.
578+
Pep723ItemRef::Stdin(metadata) => cache_digest(&metadata.raw),
579+
};
580+
581+
cache
582+
.shard(CacheBucket::Environments, entry)
583+
.into_path_buf()
555584
};
556-
cache
557-
.shard(CacheBucket::Environments, entry)
558-
.into_path_buf()
585+
586+
// If `--active` is set, prefer the active virtual environment.
587+
if let Some(from_virtual_env) = from_virtual_env_variable() {
588+
if !uv_fs::is_same_file_allow_missing(&from_virtual_env, &cache_env).unwrap_or(false) {
589+
match active {
590+
Some(true) => {
591+
debug!(
592+
"Using active virtual environment `{}` instead of script environment `{}`",
593+
from_virtual_env.user_display(),
594+
cache_env.user_display()
595+
);
596+
return from_virtual_env;
597+
}
598+
Some(false) => {}
599+
None => {
600+
warn_user_once!(
601+
"`VIRTUAL_ENV={}` does not match the script environment path `{}` and will be ignored; use `--active` to target the active environment instead",
602+
from_virtual_env.user_display(),
603+
cache_env.user_display()
604+
);
605+
}
606+
}
607+
}
608+
} else {
609+
if active.unwrap_or_default() {
610+
debug!(
611+
"Use of the active virtual environment was requested, but `VIRTUAL_ENV` is not set"
612+
);
613+
}
614+
}
615+
616+
// Otherwise, use the cache root.
617+
cache_env
559618
}
560619

561620
/// Discover the interpreter to use for the current [`Pep723Item`].
@@ -569,6 +628,7 @@ impl ScriptInterpreter {
569628
allow_insecure_host: &[TrustedHost],
570629
install_mirrors: &PythonInstallMirrors,
571630
no_config: bool,
631+
active: Option<bool>,
572632
cache: &Cache,
573633
printer: Printer,
574634
) -> Result<Self, ProjectError> {
@@ -581,7 +641,8 @@ impl ScriptInterpreter {
581641
requires_python,
582642
} = ScriptPython::from_request(python_request, workspace, script, no_config).await?;
583643

584-
let root = Self::root(script, cache);
644+
let root = Self::root(script, active, cache);
645+
585646
match PythonEnvironment::from_root(&root, cache) {
586647
Ok(venv) => {
587648
if python_request.as_ref().map_or(true, |request| {
@@ -1279,6 +1340,7 @@ impl ScriptEnvironment {
12791340
allow_insecure_host: &[TrustedHost],
12801341
install_mirrors: &PythonInstallMirrors,
12811342
no_config: bool,
1343+
active: Option<bool>,
12821344
cache: &Cache,
12831345
dry_run: DryRun,
12841346
printer: Printer,
@@ -1296,6 +1358,7 @@ impl ScriptEnvironment {
12961358
allow_insecure_host,
12971359
install_mirrors,
12981360
no_config,
1361+
active,
12991362
cache,
13001363
printer,
13011364
)
@@ -1306,7 +1369,7 @@ impl ScriptEnvironment {
13061369

13071370
// Otherwise, create a virtual environment with the discovered interpreter.
13081371
ScriptInterpreter::Interpreter(interpreter) => {
1309-
let root = ScriptInterpreter::root(script, cache);
1372+
let root = ScriptInterpreter::root(script, active, cache);
13101373

13111374
// Determine a prompt for the environment, in order of preference:
13121375
//

0 commit comments

Comments
 (0)