From efc361223c134840b7650fed1ef9ebd9577b5454 Mon Sep 17 00:00:00 2001 From: Jack O'Connor Date: Thu, 26 Jun 2025 10:20:28 -0700 Subject: [PATCH] move the test buckets dir into the canonicalized temp dir Previously we were using the XDG data dir to avoid symlinks, but there's no particular guarantee that that's not going to be a symlink too. Using the canonicalized temp dir by default is also slightly nicer for a couple reasons: It's sometimes faster (an in-memory tempfs on e.g. Arch), and it makes overriding `$TMPDIR` or `%TMP%` sufficient to control where tests put temp files, without needing to override `UV_INTERNAL__TEST_DIR` too. --- .github/workflows/setup-dev-drive.ps1 | 1 - Cargo.lock | 1 - crates/uv-static/src/env_vars.rs | 4 ---- crates/uv/Cargo.toml | 1 - crates/uv/tests/it/common/mod.rs | 26 ++++++++++---------------- 5 files changed, 10 insertions(+), 23 deletions(-) diff --git a/.github/workflows/setup-dev-drive.ps1 b/.github/workflows/setup-dev-drive.ps1 index e003cc359..474b082dc 100644 --- a/.github/workflows/setup-dev-drive.ps1 +++ b/.github/workflows/setup-dev-drive.ps1 @@ -85,7 +85,6 @@ Write-Output ` "DEV_DRIVE=$($Drive)" ` "TMP=$($Tmp)" ` "TEMP=$($Tmp)" ` - "UV_INTERNAL__TEST_DIR=$($Tmp)" ` "RUSTUP_HOME=$($Drive)/.rustup" ` "CARGO_HOME=$($Drive)/.cargo" ` "UV_WORKSPACE=$($Drive)/uv" ` diff --git a/Cargo.lock b/Cargo.lock index 009b3060f..d25d7d252 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4584,7 +4584,6 @@ dependencies = [ "ctrlc", "dotenvy", "dunce", - "etcetera", "filetime", "flate2", "fs-err 3.1.1", diff --git a/crates/uv-static/src/env_vars.rs b/crates/uv-static/src/env_vars.rs index 4a44579d7..4ac2976d9 100644 --- a/crates/uv-static/src/env_vars.rs +++ b/crates/uv-static/src/env_vars.rs @@ -359,10 +359,6 @@ impl EnvVars { #[attr_hidden] pub const UV_INTERNAL__SHOW_DERIVATION_TREE: &'static str = "UV_INTERNAL__SHOW_DERIVATION_TREE"; - /// Used to set a temporary directory for some tests. - #[attr_hidden] - pub const UV_INTERNAL__TEST_DIR: &'static str = "UV_INTERNAL__TEST_DIR"; - /// Path to system-level configuration directory on Unix systems. pub const XDG_CONFIG_DIRS: &'static str = "XDG_CONFIG_DIRS"; diff --git a/crates/uv/Cargo.toml b/crates/uv/Cargo.toml index f4afb87fa..9b73a9ebd 100644 --- a/crates/uv/Cargo.toml +++ b/crates/uv/Cargo.toml @@ -114,7 +114,6 @@ assert_cmd = { version = "2.0.16" } assert_fs = { version = "1.1.2" } base64 = { workspace = true } byteorder = { version = "1.5.0" } -etcetera = { workspace = true } filetime = { version = "0.2.25" } flate2 = { workspace = true, default-features = false } ignore = { version = "0.4.23" } diff --git a/crates/uv/tests/it/common/mod.rs b/crates/uv/tests/it/common/mod.rs index 4d65aa4a3..9ead0a7c2 100644 --- a/crates/uv/tests/it/common/mod.rs +++ b/crates/uv/tests/it/common/mod.rs @@ -13,7 +13,6 @@ use assert_cmd::assert::{Assert, OutputAssertExt}; use assert_fs::assert::PathAssert; use assert_fs::fixture::{ChildPath, PathChild, PathCopy, PathCreateDir, SymlinkToFile}; use base64::{Engine, prelude::BASE64_STANDARD as base64}; -use etcetera::BaseStrategy; use futures::StreamExt; use indoc::formatdoc; use itertools::Itertools; @@ -407,25 +406,20 @@ impl TestContext { self } - /// Discover the path to the XDG state directory. We use this, rather than the OS-specific - /// temporary directory, because on macOS (and Windows on GitHub Actions), they involve - /// symlinks. (On macOS, the temporary directory is, like `/var/...`, which resolves to - /// `/private/var/...`.) + /// Default to the canonicalized path to the temp directory. We need to do this because on + /// macOS (and Windows on GitHub Actions) the standard temp dir is a symlink. (On macOS, the + /// temporary directory is, like `/var/...`, which resolves to `/private/var/...`.) /// /// It turns out that, at least on macOS, if we pass a symlink as `current_dir`, it gets /// _immediately_ resolved (such that if you call `current_dir` in the running `Command`, it - /// returns resolved symlink). This is problematic, as we _don't_ want to resolve symlinks - /// for user-provided paths. + /// returns resolved symlink). This breaks some snapshot tests, since we _don't_ want to + /// resolve symlinks for user-provided paths. pub fn test_bucket_dir() -> PathBuf { - env::var(EnvVars::UV_INTERNAL__TEST_DIR) - .map(PathBuf::from) - .unwrap_or_else(|_| { - etcetera::base_strategy::choose_base_strategy() - .expect("Failed to find base strategy") - .data_dir() - .join("uv") - .join("tests") - }) + std::env::temp_dir() + .simple_canonicalize() + .expect("failed to canonicalize temp dir") + .join("uv") + .join("tests") } /// Create a new test context with multiple Python versions.