Skip to content

Commit 74c93c9

Browse files
karthiknadigCopilot
andcommitted
fix: publish coherent request snapshots (Fixes #536)
Build configured locator graphs off-lock and atomically publish request state. Preserve compatible caches and rebind nested Conda dependencies. Keep callback panics from poisoning publication gates, and prove snapshot, cache, and generation boundaries with deterministic tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 8320c0f commit 74c93c9

6 files changed

Lines changed: 1928 additions & 350 deletions

File tree

‎crates/pet-poetry/src/lib.rs‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,8 @@ impl Locator for Poetry {
375375
#[cfg(test)]
376376
mod tests {
377377
use super::*;
378+
use pet_core::env::PythonEnv;
379+
use pet_core::manager::{EnvManager, EnvManagerType};
378380
use pet_core::os_environment::EnvironmentApi;
379381

380382
fn path_from_components(components: &[&str]) -> PathBuf {
@@ -598,4 +600,64 @@ mod tests {
598600

599601
assert!(poetry.poetry_executable.read().unwrap().is_none());
600602
}
603+
604+
#[test]
605+
fn identical_configure_preserves_cached_identification_fidelity() {
606+
let temp_dir = tempfile::tempdir().unwrap();
607+
let prefix = temp_dir.path().join("project-env");
608+
let executable = prefix.join(if cfg!(windows) {
609+
"Scripts/python.exe"
610+
} else {
611+
"bin/python"
612+
});
613+
std::fs::create_dir_all(executable.parent().unwrap()).unwrap();
614+
std::fs::write(prefix.join("pyvenv.cfg"), "version = 3.12.0").unwrap();
615+
std::fs::write(&executable, "").unwrap();
616+
std::fs::write(executable.parent().unwrap().join("activate"), "").unwrap();
617+
618+
let environment = EnvironmentApi::new();
619+
let poetry = Poetry::from(&environment);
620+
let workspace = temp_dir.path().join("removed-project");
621+
let manager = EnvManager::new(
622+
temp_dir.path().join("removed-poetry"),
623+
EnvManagerType::Poetry,
624+
Some("1.8.0".to_string()),
625+
);
626+
let config = Configuration {
627+
workspace_directories: Some(vec![workspace.clone()]),
628+
poetry_executable: Some(manager.executable.clone()),
629+
..Default::default()
630+
};
631+
poetry.configure(&config);
632+
poetry
633+
.search_result
634+
.write()
635+
.unwrap()
636+
.replace(LocatorResult {
637+
managers: vec![manager.clone()],
638+
environments: vec![PythonEnvironment {
639+
executable: Some(executable.clone()),
640+
prefix: Some(prefix.clone()),
641+
kind: Some(PythonEnvironmentKind::Poetry),
642+
manager: Some(manager.clone()),
643+
project: Some(workspace.clone()),
644+
symlinks: Some(vec![executable.clone()]),
645+
..Default::default()
646+
}],
647+
});
648+
649+
assert!(!workspace.exists());
650+
assert!(!manager.executable.exists());
651+
poetry.configure(&config);
652+
653+
let identified = poetry
654+
.try_from(&PythonEnv::new(
655+
executable,
656+
Some(prefix),
657+
Some("3.12.0".to_string()),
658+
))
659+
.expect("cached Poetry environment must remain identifiable");
660+
assert_eq!(identified.manager, Some(manager));
661+
assert_eq!(identified.project, Some(workspace));
662+
}
601663
}

‎crates/pet-pyenv/src/lib.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,14 @@ impl PyEnv {
4747
versions_dir: Arc::new(Mutex::new(None)),
4848
}
4949
}
50+
pub fn with_conda_locator(&self, conda_locator: Arc<dyn CondaLocator>) -> PyEnv {
51+
PyEnv {
52+
env_vars: self.env_vars.clone(),
53+
conda_locator,
54+
manager: self.manager.clone(),
55+
versions_dir: self.versions_dir.clone(),
56+
}
57+
}
5058
fn clear(&self) {
5159
self.manager.lock().expect("manager mutex poisoned").take();
5260
self.versions_dir

‎crates/pet-python-utils/src/cache.rs‎

Lines changed: 87 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ pub fn get_cache_directory() -> Option<PathBuf> {
4646
}
4747

4848
pub fn set_cache_directory(cache_dir: PathBuf) {
49+
CACHE.set_cache_directory(cache_dir);
50+
}
51+
52+
pub fn set_cache_directory_and_get_effective(cache_dir: PathBuf) -> PathBuf {
4953
CACHE.set_cache_directory(cache_dir)
5054
}
5155

@@ -75,24 +79,18 @@ impl CacheImpl {
7579

7680
/// Once a cache directory has been set, you cannot change it.
7781
/// No point supporting such a scenario.
78-
fn set_cache_directory(&self, cache_dir: PathBuf) {
79-
if let Some(cache_dir) = self
80-
.cache_dir
81-
.lock()
82-
.expect("cache_dir mutex poisoned")
83-
.clone()
84-
{
82+
fn set_cache_directory(&self, cache_dir: PathBuf) -> PathBuf {
83+
let mut effective = self.cache_dir.lock().expect("cache_dir mutex poisoned");
84+
if let Some(existing) = effective.as_ref() {
8585
warn!(
8686
"Cache directory has already been set to {:?}. Cannot change it now.",
87-
cache_dir
87+
existing
8888
);
89-
return;
89+
return existing.clone();
9090
}
9191
trace!("Setting cache directory to {:?}", cache_dir);
92-
self.cache_dir
93-
.lock()
94-
.expect("cache_dir mutex poisoned")
95-
.replace(cache_dir);
92+
effective.replace(cache_dir.clone());
93+
cache_dir
9694
}
9795
fn clear(&self) -> io::Result<()> {
9896
trace!("Clearing cache");
@@ -389,7 +387,11 @@ impl CacheEntry for CacheEntryImpl {
389387
#[cfg(test)]
390388
mod tests {
391389
use super::*;
392-
use std::sync::atomic::{AtomicUsize, Ordering};
390+
use std::sync::{
391+
atomic::{AtomicUsize, Ordering},
392+
mpsc, Barrier,
393+
};
394+
use std::time::Duration;
393395
use tempfile::tempdir_in;
394396

395397
fn environment(executable: PathBuf, aliases: Vec<PathBuf>) -> ResolvedPythonEnv {
@@ -523,4 +525,75 @@ mod tests {
523525

524526
assert!(entry.get().is_none());
525527
}
528+
529+
#[test]
530+
fn concurrent_first_cache_directory_set_has_one_effective_path() {
531+
let temp_dir = tempfile::tempdir().unwrap();
532+
let cache = Arc::new(CacheImpl::new(None));
533+
let barrier = Arc::new(Barrier::new(3));
534+
let (result_tx, result_rx) = mpsc::channel();
535+
let mut workers = Vec::new();
536+
537+
for name in ["cache-a", "cache-b"] {
538+
let cache = cache.clone();
539+
let barrier = barrier.clone();
540+
let result_tx = result_tx.clone();
541+
let requested = temp_dir.path().join(name);
542+
workers.push(std::thread::spawn(move || {
543+
barrier.wait();
544+
result_tx
545+
.send(cache.set_cache_directory(requested))
546+
.unwrap();
547+
}));
548+
}
549+
550+
barrier.wait();
551+
let first = result_rx.recv_timeout(Duration::from_secs(5)).unwrap();
552+
let second = result_rx.recv_timeout(Duration::from_secs(5)).unwrap();
553+
for worker in workers {
554+
worker.join().unwrap();
555+
}
556+
557+
assert_eq!(first, second);
558+
assert_eq!(cache.get_cache_directory(), Some(first));
559+
}
560+
561+
#[test]
562+
fn in_flight_entry_can_repopulate_persistent_cache_after_clear() {
563+
let temp_dir = tempfile::tempdir().unwrap();
564+
let cache_directory = temp_dir.path().join("cache");
565+
std::fs::create_dir_all(&cache_directory).unwrap();
566+
let executable = temp_dir.path().join("python");
567+
std::fs::write(&executable, "python").unwrap();
568+
let cache = Arc::new(CacheImpl::new(Some(cache_directory)));
569+
let entry = cache.create_cache(executable.clone());
570+
let entry_guard = entry.lock().unwrap();
571+
let (clear_done_tx, clear_done_rx) = mpsc::channel();
572+
let clear_cache = cache.clone();
573+
574+
let clear_worker = std::thread::spawn(move || {
575+
clear_done_tx.send(clear_cache.clear()).unwrap();
576+
});
577+
clear_done_rx
578+
.recv_timeout(Duration::from_secs(5))
579+
.expect("clear must complete while the old entry is held")
580+
.unwrap();
581+
582+
let expected = environment(executable.clone(), vec![executable.clone()]);
583+
entry_guard.store(expected.clone());
584+
drop(entry_guard);
585+
clear_worker.join().unwrap();
586+
587+
let next_entry = cache.create_cache(executable);
588+
assert!(!Arc::ptr_eq(&entry, &next_entry));
589+
let repopulated = next_entry
590+
.lock()
591+
.unwrap()
592+
.get()
593+
.expect("post-clear write must be visible to the next entry");
594+
assert_eq!(repopulated.executable, expected.executable);
595+
assert_eq!(repopulated.prefix, expected.prefix);
596+
assert_eq!(repopulated.version, expected.version);
597+
assert_eq!(repopulated.symlinks, expected.symlinks);
598+
}
526599
}

‎crates/pet-windows-registry/src/lib.rs‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,12 @@ impl WindowsRegistry {
4444
search_result: Arc::new(Mutex::new(None)),
4545
}
4646
}
47+
pub fn with_conda_locator(&self, conda_locator: Arc<dyn CondaLocator>) -> WindowsRegistry {
48+
WindowsRegistry {
49+
conda_locator,
50+
search_result: self.search_result.clone(),
51+
}
52+
}
4753
#[cfg(windows)]
4854
fn find_with_cache(
4955
&self,
@@ -521,6 +527,67 @@ mod tests {
521527
);
522528
}
523529

530+
#[cfg(windows)]
531+
#[test]
532+
fn test_rebound_registry_replays_cached_conda_dirs_to_new_locator() {
533+
use pet_core::manager::EnvManager;
534+
use pet_core::python_environment::PythonEnvironment;
535+
use pet_core::telemetry::TelemetryEvent;
536+
537+
struct NoopReporter;
538+
impl Reporter for NoopReporter {
539+
fn report_manager(&self, _manager: &EnvManager) {}
540+
fn report_environment(&self, _env: &PythonEnvironment) {}
541+
fn report_telemetry(&self, _event: &TelemetryEvent) {}
542+
}
543+
544+
let temp = TempDir::new().unwrap();
545+
let conda_prefix = temp.path().join("registry-conda");
546+
fs::create_dir_all(conda_prefix.join("conda-meta")).unwrap();
547+
fs::create_dir_all(conda_prefix.join("envs")).unwrap();
548+
fs::create_dir_all(conda_prefix.join("Scripts")).unwrap();
549+
fs::write(conda_prefix.join("conda-meta").join("history"), "").unwrap();
550+
fs::write(
551+
conda_prefix
552+
.join("conda-meta")
553+
.join("conda-24.1.0-py312_0.json"),
554+
r#"{"version":"24.1.0"}"#,
555+
)
556+
.unwrap();
557+
fs::write(
558+
conda_prefix
559+
.join("conda-meta")
560+
.join("python-3.12.0-h123_0.json"),
561+
r#"{"version":"3.12.0"}"#,
562+
)
563+
.unwrap();
564+
fs::write(conda_prefix.join("Scripts").join("conda.bat"), "").unwrap();
565+
fs::write(conda_prefix.join("python.exe"), "").unwrap();
566+
567+
let environment = EnvironmentApi::new();
568+
let retired_conda = Arc::new(Conda::from(&environment));
569+
let replacement_conda = Arc::new(Conda::from(&environment));
570+
let registry = WindowsRegistry::from(retired_conda.clone());
571+
registry
572+
.search_result
573+
.lock()
574+
.unwrap()
575+
.replace(Arc::new(CachedRegistryWalk {
576+
result: LocatorResult {
577+
managers: Vec::new(),
578+
environments: Vec::new(),
579+
},
580+
conda_install_dirs: vec![conda_prefix.clone()],
581+
}));
582+
let rebound = registry.with_conda_locator(replacement_conda.clone());
583+
584+
rebound.find(&NoopReporter);
585+
586+
assert!(Arc::ptr_eq(&registry.search_result, &rebound.search_result));
587+
assert!(replacement_conda.environments.contains_key(&conda_prefix));
588+
assert!(!retired_conda.environments.contains_key(&conda_prefix));
589+
}
590+
524591
/// Smoke test: on a fresh locator (empty cache), `find()` runs the new
525592
/// parallel walk through HKLM and HKCU and never panics or deadlocks.
526593
/// The discovered environment list may legitimately be empty on a CI

0 commit comments

Comments
 (0)