diff --git a/Cargo.lock b/Cargo.lock index f2fb9c87..17248b83 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -650,6 +650,7 @@ dependencies = [ "pet-conda", "pet-core", "pet-python-utils", + "tempfile", ] [[package]] @@ -798,6 +799,7 @@ dependencies = [ "pet-virtualenv", "pet-windows-store", "regex", + "tempfile", "winreg", ] diff --git a/crates/pet-core/src/cache.rs b/crates/pet-core/src/cache.rs index 361a3bbd..6cdcacb2 100644 --- a/crates/pet-core/src/cache.rs +++ b/crates/pet-core/src/cache.rs @@ -29,19 +29,29 @@ impl LocatorCache { /// Returns a cloned value for the given key if it exists in the cache. pub fn get(&self, key: &K) -> Option { - self.cache.read().unwrap().get(key).cloned() + self.cache + .read() + .expect("locator cache lock poisoned") + .get(key) + .cloned() } /// Checks if the cache contains the given key. pub fn contains_key(&self, key: &K) -> bool { - self.cache.read().unwrap().contains_key(key) + self.cache + .read() + .expect("locator cache lock poisoned") + .contains_key(key) } /// Inserts a key-value pair into the cache. /// /// Returns the previous value if the key was already present. pub fn insert(&self, key: K, value: V) -> Option { - self.cache.write().unwrap().insert(key, value) + self.cache + .write() + .expect("locator cache lock poisoned") + .insert(key, value) } /// Inserts multiple key-value pairs into the cache atomically. @@ -49,7 +59,7 @@ impl LocatorCache { /// This method acquires a single write lock for all insertions, which is more /// efficient than calling `insert` multiple times when inserting many entries. pub fn insert_many(&self, entries: impl IntoIterator) { - let mut cache = self.cache.write().unwrap(); + let mut cache = self.cache.write().expect("locator cache lock poisoned"); for (key, value) in entries { cache.insert(key, value); } @@ -68,7 +78,7 @@ impl LocatorCache { { // First check with read lock { - let cache = self.cache.read().unwrap(); + let cache = self.cache.read().expect("locator cache lock poisoned"); if let Some(value) = cache.get(&key) { return Some(value.clone()); } @@ -77,7 +87,7 @@ impl LocatorCache { // Compute the value (outside of any lock) if let Some(value) = f() { // Acquire write lock and insert - let mut cache = self.cache.write().unwrap(); + let mut cache = self.cache.write().expect("locator cache lock poisoned"); // Double-check in case another thread inserted while we were computing if let Some(existing) = cache.get(&key) { return Some(existing.clone()); @@ -91,22 +101,36 @@ impl LocatorCache { /// Clears all entries from the cache. pub fn clear(&self) { - self.cache.write().unwrap().clear(); + self.cache + .write() + .expect("locator cache lock poisoned") + .clear(); } /// Returns all values in the cache as a vector. pub fn values(&self) -> Vec { - self.cache.read().unwrap().values().cloned().collect() + self.cache + .read() + .expect("locator cache lock poisoned") + .values() + .cloned() + .collect() } /// Returns the number of entries in the cache. pub fn len(&self) -> usize { - self.cache.read().unwrap().len() + self.cache + .read() + .expect("locator cache lock poisoned") + .len() } /// Returns true if the cache is empty. pub fn is_empty(&self) -> bool { - self.cache.read().unwrap().is_empty() + self.cache + .read() + .expect("locator cache lock poisoned") + .is_empty() } /// Returns all entries in the cache as a HashMap. @@ -114,7 +138,10 @@ impl LocatorCache { where K: Clone, { - self.cache.read().unwrap().clone() + self.cache + .read() + .expect("locator cache lock poisoned") + .clone() } } diff --git a/crates/pet-pixi/Cargo.toml b/crates/pet-pixi/Cargo.toml index 66f3816a..447d853a 100644 --- a/crates/pet-pixi/Cargo.toml +++ b/crates/pet-pixi/Cargo.toml @@ -12,3 +12,6 @@ pet-conda = { path = "../pet-conda" } pet-core = { path = "../pet-core" } pet-python-utils = { path = "../pet-python-utils" } log = "0.4.21" + +[dev-dependencies] +tempfile = "3.13" diff --git a/crates/pet-pixi/src/lib.rs b/crates/pet-pixi/src/lib.rs index e6841a68..c4557610 100644 --- a/crates/pet-pixi/src/lib.rs +++ b/crates/pet-pixi/src/lib.rs @@ -89,24 +89,11 @@ impl Locator for Pixi { #[cfg(test)] mod tests { use super::*; - use std::{ - fs, - time::{SystemTime, UNIX_EPOCH}, - }; - - fn create_test_dir(name: &str) -> PathBuf { - let unique = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_nanos(); - let directory = - std::env::temp_dir().join(format!("pet-pixi-{name}-{}-{unique}", std::process::id())); - fs::create_dir_all(&directory).unwrap(); - directory - } + use std::fs; + use tempfile::TempDir; - fn create_pixi_prefix() -> PathBuf { - let prefix = create_test_dir("prefix").join("pixi-env"); + fn create_pixi_prefix(temp_dir: &TempDir) -> PathBuf { + let prefix = temp_dir.path().join("pixi-env"); fs::create_dir_all(prefix.join("conda-meta")).unwrap(); fs::write(prefix.join("conda-meta").join("pixi"), b"").unwrap(); fs::create_dir_all(prefix.join(if cfg!(windows) { "Scripts" } else { "bin" })).unwrap(); @@ -126,17 +113,17 @@ mod tests { #[test] fn is_pixi_env_checks_for_pixi_marker_file() { - let prefix = create_pixi_prefix(); + let temp_dir = TempDir::new().unwrap(); + let prefix = create_pixi_prefix(&temp_dir); assert!(is_pixi_env(&prefix)); assert!(!is_pixi_env(&prefix.join("conda-meta"))); - - fs::remove_dir_all(prefix.parent().unwrap()).unwrap(); } #[test] fn try_from_identifies_pixi_env_from_explicit_prefix() { - let prefix = create_pixi_prefix(); + let temp_dir = TempDir::new().unwrap(); + let prefix = create_pixi_prefix(&temp_dir); let executable = prefix .join(if cfg!(windows) { "Scripts" } else { "bin" }) .join(if cfg!(windows) { @@ -163,7 +150,7 @@ mod tests { .map(fs::canonicalize) .transpose() .unwrap(), - Some(fs::canonicalize(prefix.clone()).unwrap()) + Some(fs::canonicalize(prefix).unwrap()) ); assert_eq!( pixi_env @@ -174,13 +161,12 @@ mod tests { .unwrap(), Some(fs::canonicalize(executable).unwrap()) ); - - fs::remove_dir_all(prefix.parent().unwrap()).unwrap(); } #[test] fn try_from_derives_pixi_prefix_from_nested_python_executable() { - let prefix = create_pixi_prefix(); + let temp_dir = TempDir::new().unwrap(); + let prefix = create_pixi_prefix(&temp_dir); let executable = prefix .join(if cfg!(windows) { "Scripts" } else { "bin" }) .join(if cfg!(windows) { @@ -202,22 +188,18 @@ mod tests { .map(fs::canonicalize) .transpose() .unwrap(), - Some(fs::canonicalize(prefix.clone()).unwrap()) + Some(fs::canonicalize(prefix).unwrap()) ); - - fs::remove_dir_all(prefix.parent().unwrap()).unwrap(); } #[test] fn try_from_rejects_non_pixi_environments() { - let prefix = create_test_dir("plain-prefix"); - let executable = prefix.join("python"); + let temp_dir = TempDir::new().unwrap(); + let executable = temp_dir.path().join("python"); fs::write(&executable, b"").unwrap(); let locator = Pixi::new(); - let env = PythonEnv::new(executable, Some(prefix.clone()), None); + let env = PythonEnv::new(executable, Some(temp_dir.path().to_path_buf()), None); assert!(locator.try_from(&env).is_none()); - - fs::remove_dir_all(prefix).unwrap(); } } diff --git a/crates/pet-uv/src/lib.rs b/crates/pet-uv/src/lib.rs index 77d16385..ffd29c9e 100644 --- a/crates/pet-uv/src/lib.rs +++ b/crates/pet-uv/src/lib.rs @@ -344,12 +344,32 @@ fn parse_version_from_uv_dir_name(dir_name: &str) -> Option { let mut parts = dir_name.splitn(3, '-'); let _impl = parts.next()?; let version = parts.next()?; - // Verify at minimum X.Y format (e.g., "3.12" or "3.12.3") - if version.starts_with(|c: char| c.is_ascii_digit()) && version.split('.').count() >= 2 { - Some(version.to_string()) - } else { - None + // Require the platform segment to exist (rejects bare "-"). + let platform = parts.next()?; + if platform.is_empty() { + return None; + } + // Verify at minimum X.Y format (e.g., "3.12" or "3.12.3"). + // Components before the last must be purely numeric. + // The last component may have a pre-release suffix (e.g., "0a4", "0rc1"). + let components: Vec<&str> = version.split('.').collect(); + if components.len() < 2 { + return None; + } + // All components except the last must be purely numeric. + let all_but_last = &components[..components.len() - 1]; + if !all_but_last + .iter() + .all(|c| !c.is_empty() && c.chars().all(|ch| ch.is_ascii_digit())) + { + return None; + } + // The last component must start with a digit (allows pre-release suffix like "0a4"). + let last = components.last()?; + if last.is_empty() || !last.starts_with(|ch: char| ch.is_ascii_digit()) { + return None; } + Some(version.to_string()) } /// Walks up from `project_path` looking for a workspace that this project belongs to. @@ -1107,6 +1127,41 @@ exclude = ["packages/legacy"]"#; assert_eq!(parse_version_from_uv_dir_name("cpython-9-linux"), None); } + #[test] + fn test_parse_version_from_uv_dir_name_rejects_non_numeric_components() { + // Version components must start with a digit — "3.abc.def" should be rejected. + assert_eq!( + parse_version_from_uv_dir_name("cpython-3.abc.def-linux"), + None + ); + // Empty component after dot. + assert_eq!(parse_version_from_uv_dir_name("cpython-3.12.-linux"), None); + // Non-numeric middle component must be rejected even if it starts with a digit. + assert_eq!( + parse_version_from_uv_dir_name("cpython-3.12abc.1-linux"), + None + ); + } + + #[test] + fn test_parse_version_from_uv_dir_name_rejects_missing_platform() { + // Bare "-" without platform segment should be rejected. + assert_eq!(parse_version_from_uv_dir_name("cpython-3.12"), None); + } + + #[test] + fn test_parse_version_from_uv_dir_name_accepts_prerelease() { + // Pre-release versions like "3.14.0a4" are valid uv install dirs. + assert_eq!( + parse_version_from_uv_dir_name("cpython-3.14.0a4-linux-x86_64-gnu"), + Some("3.14.0a4".to_string()) + ); + assert_eq!( + parse_version_from_uv_dir_name("cpython-3.13.0rc1-linux-x86_64-gnu"), + Some("3.13.0rc1".to_string()) + ); + } + #[test] fn test_find_managed_python_installs_discovers_versions() { let temp_dir = TempDir::new().unwrap(); diff --git a/crates/pet-windows-registry/Cargo.toml b/crates/pet-windows-registry/Cargo.toml index a0b44af4..feba65c9 100644 --- a/crates/pet-windows-registry/Cargo.toml +++ b/crates/pet-windows-registry/Cargo.toml @@ -20,3 +20,6 @@ pet-conda = { path = "../pet-conda" } log = "0.4.21" lazy_static = "1.4.0" regex = "1.10.4" + +[dev-dependencies] +tempfile = "3.13" diff --git a/crates/pet-windows-registry/src/lib.rs b/crates/pet-windows-registry/src/lib.rs index ae550c45..9701d26a 100644 --- a/crates/pet-windows-registry/src/lib.rs +++ b/crates/pet-windows-registry/src/lib.rs @@ -149,21 +149,8 @@ mod tests { use std::{ fs, path::{Path, PathBuf}, - time::{SystemTime, UNIX_EPOCH}, }; - - fn create_test_dir(name: &str) -> PathBuf { - let unique = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_nanos(); - let directory = std::env::temp_dir().join(format!( - "pet-windows-registry-{name}-{}-{unique}", - std::process::id() - )); - fs::create_dir_all(&directory).unwrap(); - directory - } + use tempfile::TempDir; fn create_virtualenv(prefix: &Path) -> PathBuf { let scripts_dir = prefix.join(if cfg!(windows) { "Scripts" } else { "bin" }); @@ -318,27 +305,25 @@ mod tests { #[test] fn test_try_from_rejects_virtualenv_before_registry_lookup() { - let prefix = create_test_dir("virtualenv"); + let temp_dir = TempDir::new().unwrap(); + let prefix = temp_dir.path().to_path_buf(); let executable = create_virtualenv(&prefix); - let env = PythonEnv::new(executable, Some(prefix.clone()), None); + let env = PythonEnv::new(executable, Some(prefix), None); let locator = create_locator(); assert!(locator.try_from(&env).is_none()); - - fs::remove_dir_all(prefix).unwrap(); } #[test] fn test_try_from_rejects_conda_prefix_before_registry_lookup() { - let prefix = create_test_dir("conda-env"); + let temp_dir = TempDir::new().unwrap(); + let prefix = temp_dir.path().to_path_buf(); fs::create_dir_all(prefix.join("conda-meta")).unwrap(); let executable = prefix.join("python.exe"); fs::write(&executable, b"").unwrap(); - let env = PythonEnv::new(executable, Some(prefix.clone()), None); + let env = PythonEnv::new(executable, Some(prefix), None); let locator = create_locator(); assert!(locator.try_from(&env).is_none()); - - fs::remove_dir_all(prefix).unwrap(); } } diff --git a/crates/pet/tests/jsonrpc_client.rs b/crates/pet/tests/jsonrpc_client.rs index 23837f82..2f43f845 100644 --- a/crates/pet/tests/jsonrpc_client.rs +++ b/crates/pet/tests/jsonrpc_client.rs @@ -100,7 +100,15 @@ impl PetJsonRpcClient { .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) + // Clear all inherited env vars to prevent host-specific tool + // configuration from leaking into the test environment, then + // restore only the minimum required for the OS to function. + .env_clear() .env("PATH", "") + .env( + "SYSTEMROOT", + std::env::var("SYSTEMROOT").unwrap_or_default(), + ) .spawn() .map_err(|e| format!("Failed to spawn pet server: {e}"))?;