From 3f11e552b4b1d4d24dfd771963147ae18d8def6a Mon Sep 17 00:00:00 2001 From: Karthik Nadig Date: Sat, 4 Apr 2026 00:35:18 -0700 Subject: [PATCH] fix: formalize refresh locator state (Fixes #387) --- crates/pet-conda/src/lib.rs | 38 ++++- crates/pet-core/src/lib.rs | 44 +++++- crates/pet-linux-global-python/src/lib.rs | 5 +- crates/pet-pipenv/src/lib.rs | 13 +- crates/pet-poetry/src/lib.rs | 151 +++++++++++++++++++- crates/pet-pyenv/src/lib.rs | 5 +- crates/pet-pyenv/tests/pyenv_test.rs | 42 ++++++ crates/pet-uv/src/lib.rs | 16 ++- crates/pet-windows-registry/src/lib.rs | 107 ++++++++++++++- crates/pet-windows-store/src/lib.rs | 97 ++++++++++++- crates/pet/src/jsonrpc.rs | 160 ++++++++++++++++++---- 11 files changed, 622 insertions(+), 56 deletions(-) diff --git a/crates/pet-conda/src/lib.rs b/crates/pet-conda/src/lib.rs index 61a311d9..a444d388 100644 --- a/crates/pet-conda/src/lib.rs +++ b/crates/pet-conda/src/lib.rs @@ -16,7 +16,7 @@ use pet_core::{ os_environment::Environment, python_environment::{PythonEnvironment, PythonEnvironmentKind}, reporter::Reporter, - Locator, LocatorKind, + Locator, LocatorKind, RefreshStatePersistence, RefreshStateSyncScope, }; use pet_fs::path::norm_case; use rayon::prelude::*; @@ -216,11 +216,39 @@ impl Locator for Conda { fn get_kind(&self) -> LocatorKind { LocatorKind::Conda } - fn configure(&self, config: &pet_core::Configuration) { - if let Some(ref conda_exe) = config.conda_executable { - let mut conda_executable = self.conda_executable.write().unwrap(); - conda_executable.replace(conda_exe.clone()); + fn refresh_state(&self) -> RefreshStatePersistence { + RefreshStatePersistence::SyncedDiscoveryState + } + fn sync_refresh_state_from(&self, source: &dyn Locator, scope: &RefreshStateSyncScope) { + let source = source.as_any().downcast_ref::().unwrap_or_else(|| { + panic!("attempted to sync Conda state from {:?}", source.get_kind()) + }); + + match scope { + RefreshStateSyncScope::Full => {} + RefreshStateSyncScope::GlobalFiltered(kind) + if self.supported_categories().contains(kind) => {} + RefreshStateSyncScope::GlobalFiltered(_) | RefreshStateSyncScope::Workspace => { + return; + } } + + self.environments.clear(); + self.environments + .insert_many(source.environments.clone_map()); + + self.managers.clear(); + self.managers.insert_many(source.managers.clone_map()); + + self.mamba_managers.clear(); + self.mamba_managers + .insert_many(source.mamba_managers.clone_map()); + } + fn configure(&self, config: &pet_core::Configuration) { + self.conda_executable + .write() + .unwrap() + .clone_from(&config.conda_executable); } fn supported_categories(&self) -> Vec { vec![PythonEnvironmentKind::Conda] diff --git a/crates/pet-core/src/lib.rs b/crates/pet-core/src/lib.rs index c3cffdf2..9829131f 100644 --- a/crates/pet-core/src/lib.rs +++ b/crates/pet-core/src/lib.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use std::path::PathBuf; +use std::{any::Any, path::PathBuf}; use env::PythonEnv; use manager::EnvManager; @@ -61,7 +61,26 @@ pub enum LocatorKind { WindowsStore, } -pub trait Locator: Send + Sync { +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RefreshStatePersistence { + /// The locator keeps no mutable state across requests. + Stateless, + /// The locator keeps configured inputs only; refresh must not copy them back. + ConfiguredOnly, + /// The locator keeps cache-like state, but later requests can repopulate it on demand. + SelfHydratingCache, + /// The locator keeps refresh-discovered state that later requests depend on for correctness. + SyncedDiscoveryState, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum RefreshStateSyncScope { + Full, + GlobalFiltered(PythonEnvironmentKind), + Workspace, +} + +pub trait Locator: Any + Send + Sync { /// Returns the name of the locator. fn get_kind(&self) -> LocatorKind; /// Configures the locator with the given configuration. @@ -100,6 +119,21 @@ pub trait Locator: Send + Sync { fn configure(&self, _config: &Configuration) { // } + /// Describes what mutable state, if any, must survive a refresh boundary. + /// + /// Refresh runs execute against transient locator graphs and then invoke + /// `sync_refresh_state_from()` on the long-lived shared locators. + fn refresh_state(&self) -> RefreshStatePersistence { + RefreshStatePersistence::Stateless + } + /// Copies correctness-critical post-refresh state from a transient locator into this + /// long-lived shared locator. + /// + /// Override this only when `refresh_state()` returns + /// `RefreshStatePersistence::SyncedDiscoveryState`. + fn sync_refresh_state_from(&self, _source: &dyn Locator, _scope: &RefreshStateSyncScope) { + // + } /// Returns a list of supported categories for this locator. fn supported_categories(&self) -> Vec; /// Given a Python executable, and some optional data like prefix, @@ -112,3 +146,9 @@ pub trait Locator: Send + Sync { /// Finds all environments specific to this locator. fn find(&self, reporter: &dyn Reporter); } + +impl dyn Locator { + pub fn as_any(&self) -> &dyn Any { + self + } +} diff --git a/crates/pet-linux-global-python/src/lib.rs b/crates/pet-linux-global-python/src/lib.rs index 1b0297e2..67f23759 100644 --- a/crates/pet-linux-global-python/src/lib.rs +++ b/crates/pet-linux-global-python/src/lib.rs @@ -15,7 +15,7 @@ use pet_core::{ env::PythonEnv, python_environment::{PythonEnvironment, PythonEnvironmentBuilder, PythonEnvironmentKind}, reporter::Reporter, - Locator, LocatorKind, + Locator, LocatorKind, RefreshStatePersistence, }; use pet_fs::path::resolve_symlink; use pet_python_utils::{env::ResolvedPythonEnv, executable::find_executables}; @@ -62,6 +62,9 @@ impl Locator for LinuxGlobalPython { fn get_kind(&self) -> LocatorKind { LocatorKind::LinuxGlobal } + fn refresh_state(&self) -> RefreshStatePersistence { + RefreshStatePersistence::SelfHydratingCache + } fn supported_categories(&self) -> Vec { vec![PythonEnvironmentKind::LinuxGlobal] } diff --git a/crates/pet-pipenv/src/lib.rs b/crates/pet-pipenv/src/lib.rs index 4bc91fc4..12ceb192 100644 --- a/crates/pet-pipenv/src/lib.rs +++ b/crates/pet-pipenv/src/lib.rs @@ -11,7 +11,7 @@ use pet_core::LocatorKind; use pet_core::{ python_environment::{PythonEnvironment, PythonEnvironmentBuilder, PythonEnvironmentKind}, reporter::Reporter, - Configuration, Locator, + Configuration, Locator, RefreshStatePersistence, }; use pet_fs::path::norm_case; use pet_python_utils::executable::find_executables; @@ -418,10 +418,15 @@ impl Locator for PipEnv { LocatorKind::PipEnv } + fn refresh_state(&self) -> RefreshStatePersistence { + RefreshStatePersistence::ConfiguredOnly + } + fn configure(&self, config: &Configuration) { - if let Some(exe) = &config.pipenv_executable { - self.pipenv_executable.write().unwrap().replace(exe.clone()); - } + self.pipenv_executable + .write() + .unwrap() + .clone_from(&config.pipenv_executable); } fn supported_categories(&self) -> Vec { diff --git a/crates/pet-poetry/src/lib.rs b/crates/pet-poetry/src/lib.rs index 1324f8a8..54c37081 100644 --- a/crates/pet-poetry/src/lib.rs +++ b/crates/pet-poetry/src/lib.rs @@ -11,7 +11,8 @@ use pet_core::{ os_environment::Environment, python_environment::{PythonEnvironment, PythonEnvironmentKind}, reporter::Reporter, - Configuration, Locator, LocatorKind, LocatorResult, + Configuration, Locator, LocatorKind, LocatorResult, RefreshStatePersistence, + RefreshStateSyncScope, }; use pet_virtualenv::is_virtualenv; use regex::Regex; @@ -137,7 +138,6 @@ impl Poetry { } } fn clear(&self) { - self.poetry_executable.write().unwrap().take(); self.search_result.write().unwrap().take(); } pub fn from(environment: &dyn Environment) -> Poetry { @@ -152,6 +152,34 @@ impl Poetry { .clone_from(&search_result); } + pub fn merge_search_result_from(&self, source: &Poetry) { + let source_search_result = source.search_result.read().unwrap().clone(); + let Some(source_search_result) = source_search_result else { + return; + }; + + let mut merged = self + .search_result + .read() + .unwrap() + .clone() + .unwrap_or(LocatorResult { + managers: vec![], + environments: vec![], + }); + merged.managers.extend(source_search_result.managers); + merged.managers.sort(); + merged.managers.dedup(); + + merged + .environments + .extend(source_search_result.environments); + merged.environments.sort(); + merged.environments.dedup(); + + self.search_result.write().unwrap().replace(merged); + } + fn find_with_cache(&self) -> Option { // First check if we have cached results { @@ -226,17 +254,39 @@ impl Locator for Poetry { fn get_kind(&self) -> LocatorKind { LocatorKind::Poetry } + fn refresh_state(&self) -> RefreshStatePersistence { + RefreshStatePersistence::SyncedDiscoveryState + } + fn sync_refresh_state_from(&self, source: &dyn Locator, scope: &RefreshStateSyncScope) { + let source = source.as_any().downcast_ref::().unwrap_or_else(|| { + panic!( + "attempted to sync Poetry state from {:?}", + source.get_kind() + ) + }); + match scope { + RefreshStateSyncScope::Full => self.sync_search_result_from(source), + RefreshStateSyncScope::GlobalFiltered(kind) + if self.supported_categories().contains(kind) => + { + self.sync_search_result_from(source) + } + RefreshStateSyncScope::Workspace => self.merge_search_result_from(source), + RefreshStateSyncScope::GlobalFiltered(_) => {} + } + } fn configure(&self, config: &Configuration) { + let mut ws_dirs = self.workspace_directories.write().unwrap(); + ws_dirs.clear(); if let Some(workspace_directories) = &config.workspace_directories { - let mut ws_dirs = self.workspace_directories.write().unwrap(); - ws_dirs.clear(); if !workspace_directories.is_empty() { ws_dirs.extend(workspace_directories.clone()); } } - if let Some(exe) = &config.poetry_executable { - self.poetry_executable.write().unwrap().replace(exe.clone()); - } + self.poetry_executable + .write() + .unwrap() + .clone_from(&config.poetry_executable); } fn supported_categories(&self) -> Vec { @@ -349,4 +399,91 @@ mod tests { Some("fresh") ); } + + #[test] + fn test_workspace_scope_merges_search_results() { + let environment = EnvironmentApi::new(); + let target = Poetry::from(&environment); + let source = Poetry::from(&environment); + + target + .search_result + .write() + .unwrap() + .replace(LocatorResult { + managers: vec![], + environments: vec![PythonEnvironment { + name: Some("existing".to_string()), + kind: Some(PythonEnvironmentKind::Poetry), + ..Default::default() + }], + }); + + source + .search_result + .write() + .unwrap() + .replace(LocatorResult { + managers: vec![], + environments: vec![PythonEnvironment { + name: Some("workspace".to_string()), + kind: Some(PythonEnvironmentKind::Poetry), + ..Default::default() + }], + }); + + target.sync_refresh_state_from(&source, &RefreshStateSyncScope::Workspace); + + let result = target.search_result.read().unwrap().clone().unwrap(); + let mut names = result + .environments + .iter() + .map(|environment| environment.name.clone().unwrap()) + .collect::>(); + names.sort(); + + assert_eq!(names, vec!["existing".to_string(), "workspace".to_string()]); + } + + #[test] + fn test_clear_preserves_configured_poetry_executable() { + let environment = EnvironmentApi::new(); + let poetry = Poetry::from(&environment); + let configured = PathBuf::from("/configured/poetry"); + + poetry.configure(&Configuration { + poetry_executable: Some(configured.clone()), + ..Default::default() + }); + poetry + .search_result + .write() + .unwrap() + .replace(LocatorResult { + managers: vec![], + environments: vec![], + }); + + poetry.clear(); + + assert_eq!( + poetry.poetry_executable.read().unwrap().clone(), + Some(configured) + ); + assert!(poetry.search_result.read().unwrap().is_none()); + } + + #[test] + fn test_configure_clears_poetry_executable_when_unset() { + let environment = EnvironmentApi::new(); + let poetry = Poetry::from(&environment); + + poetry.configure(&Configuration { + poetry_executable: Some(PathBuf::from("/configured/poetry")), + ..Default::default() + }); + poetry.configure(&Configuration::default()); + + assert!(poetry.poetry_executable.read().unwrap().is_none()); + } } diff --git a/crates/pet-pyenv/src/lib.rs b/crates/pet-pyenv/src/lib.rs index da4549ff..90900306 100644 --- a/crates/pet-pyenv/src/lib.rs +++ b/crates/pet-pyenv/src/lib.rs @@ -19,7 +19,7 @@ use pet_core::{ os_environment::Environment, python_environment::{PythonEnvironment, PythonEnvironmentKind}, reporter::Reporter, - Locator, LocatorKind, + Locator, LocatorKind, RefreshStatePersistence, }; use pet_python_utils::executable::find_executable; @@ -84,6 +84,9 @@ impl Locator for PyEnv { fn get_kind(&self) -> LocatorKind { LocatorKind::PyEnv } + fn refresh_state(&self) -> RefreshStatePersistence { + RefreshStatePersistence::SelfHydratingCache + } fn supported_categories(&self) -> Vec { vec![ PythonEnvironmentKind::Pyenv, diff --git a/crates/pet-pyenv/tests/pyenv_test.rs b/crates/pet-pyenv/tests/pyenv_test.rs index 69706f31..bf7fbe94 100644 --- a/crates/pet-pyenv/tests/pyenv_test.rs +++ b/crates/pet-pyenv/tests/pyenv_test.rs @@ -546,3 +546,45 @@ fn resolve_pyenv_environment() { assert!(result.is_some()); assert_eq!(result.unwrap().kind, Some(PythonEnvironmentKind::Conda)); } + +#[test] +#[cfg(unix)] +fn pyenv_refresh_state_self_hydrates_without_sync() { + use crate::common::create_test_environment; + use common::resolve_test_path; + use pet_conda::Conda; + use pet_core::{ + env::PythonEnv, python_environment::PythonEnvironmentKind, Locator, RefreshStatePersistence, + }; + use pet_pyenv::PyEnv; + use pet_reporter::{cache::CacheReporter, collect}; + use std::{collections::HashMap, sync::Arc}; + + let home = resolve_test_path(&["unix", "pyenv", "user_home"]); + let homebrew_bin = resolve_test_path(&["unix", "pyenv", "home", "opt", "homebrew", "bin"]); + let environment = + create_test_environment(HashMap::new(), Some(home.clone()), vec![homebrew_bin], None); + + let shared_conda = Arc::new(Conda::from(&environment)); + let shared = PyEnv::from(&environment, shared_conda.clone()); + let refreshed = PyEnv::from(&environment, shared_conda); + + assert_eq!( + shared.refresh_state(), + RefreshStatePersistence::SelfHydratingCache + ); + + let reporter = Arc::new(collect::create_reporter()); + refreshed.find(&CacheReporter::new(reporter)); + + let executable = + resolve_test_path(&[home.to_str().unwrap(), ".pyenv/versions/3.9.9/bin/python"]); + let prefix = resolve_test_path(&[home.to_str().unwrap(), ".pyenv/versions/3.9.9"]); + + let resolved = shared.try_from(&PythonEnv::new(executable, Some(prefix.clone()), None)); + + assert!(resolved.is_some()); + let resolved = resolved.unwrap(); + assert_eq!(resolved.kind, Some(PythonEnvironmentKind::Pyenv)); + assert_eq!(resolved.prefix, Some(prefix)); +} diff --git a/crates/pet-uv/src/lib.rs b/crates/pet-uv/src/lib.rs index 66e7612d..77d16385 100644 --- a/crates/pet-uv/src/lib.rs +++ b/crates/pet-uv/src/lib.rs @@ -12,7 +12,7 @@ use pet_core::{ python_environment::{PythonEnvironment, PythonEnvironmentBuilder, PythonEnvironmentKind}, pyvenv_cfg::PyVenvCfg, reporter::Reporter, - Configuration, Locator, LocatorKind, + Configuration, Locator, LocatorKind, RefreshStatePersistence, }; use pet_fs::path::norm_case; use pet_python_utils::executable::{find_executable, find_executables}; @@ -85,6 +85,10 @@ impl Locator for Uv { LocatorKind::Uv } + fn refresh_state(&self) -> RefreshStatePersistence { + RefreshStatePersistence::ConfiguredOnly + } + fn supported_categories(&self) -> Vec { vec![ PythonEnvironmentKind::Uv, @@ -93,12 +97,12 @@ impl Locator for Uv { } fn configure(&self, config: &Configuration) { + let mut ws = self + .workspace_directories + .lock() + .expect("workspace_directories mutex poisoned"); + ws.clear(); if let Some(workspace_directories) = config.workspace_directories.as_ref() { - let mut ws = self - .workspace_directories - .lock() - .expect("workspace_directories mutex poisoned"); - ws.clear(); ws.extend(workspace_directories.iter().cloned()); } } diff --git a/crates/pet-windows-registry/src/lib.rs b/crates/pet-windows-registry/src/lib.rs index 86cab9b8..aab44348 100644 --- a/crates/pet-windows-registry/src/lib.rs +++ b/crates/pet-windows-registry/src/lib.rs @@ -8,7 +8,7 @@ use pet_core::{ env::PythonEnv, python_environment::{PythonEnvironment, PythonEnvironmentKind}, reporter::Reporter, - Locator, LocatorKind, LocatorResult, + Locator, LocatorKind, LocatorResult, RefreshStatePersistence, RefreshStateSyncScope, }; use pet_virtualenv::is_virtualenv; use std::sync::{Arc, Mutex}; @@ -52,12 +52,48 @@ impl WindowsRegistry { .expect("search_result mutex poisoned"); search_result.take(); } + + fn sync_search_result_from(&self, source: &WindowsRegistry) { + let search_result = source + .search_result + .lock() + .expect("search_result mutex poisoned") + .clone(); + self.search_result + .lock() + .expect("search_result mutex poisoned") + .clone_from(&search_result); + } } impl Locator for WindowsRegistry { fn get_kind(&self) -> LocatorKind { LocatorKind::WindowsRegistry } + fn refresh_state(&self) -> RefreshStatePersistence { + RefreshStatePersistence::SyncedDiscoveryState + } + fn sync_refresh_state_from(&self, source: &dyn Locator, scope: &RefreshStateSyncScope) { + let source = source + .as_any() + .downcast_ref::() + .unwrap_or_else(|| { + panic!( + "attempted to sync WindowsRegistry state from {:?}", + source.get_kind() + ) + }); + + match scope { + RefreshStateSyncScope::Full => self.sync_search_result_from(source), + RefreshStateSyncScope::GlobalFiltered(kind) + if self.supported_categories().contains(kind) => + { + self.sync_search_result_from(source) + } + RefreshStateSyncScope::GlobalFiltered(_) | RefreshStateSyncScope::Workspace => {} + } + } fn supported_categories(&self) -> Vec { vec![ PythonEnvironmentKind::WindowsRegistry, @@ -104,3 +140,72 @@ impl Locator for WindowsRegistry { // } } + +#[cfg(test)] +mod tests { + use super::*; + use pet_conda::Conda; + use pet_core::os_environment::EnvironmentApi; + + #[test] + fn test_full_refresh_sync_replaces_registry_cache() { + let environment = EnvironmentApi::new(); + let shared = WindowsRegistry::from(Arc::new(Conda::from(&environment))); + let refreshed = WindowsRegistry::from(Arc::new(Conda::from(&environment))); + + shared.search_result.lock().unwrap().replace(LocatorResult { + managers: vec![], + environments: vec![PythonEnvironment { + name: Some("stale".to_string()), + ..Default::default() + }], + }); + refreshed + .search_result + .lock() + .unwrap() + .replace(LocatorResult { + managers: vec![], + environments: vec![PythonEnvironment { + name: Some("fresh".to_string()), + ..Default::default() + }], + }); + + shared.sync_refresh_state_from(&refreshed, &RefreshStateSyncScope::Full); + + let result = shared.search_result.lock().unwrap().clone().unwrap(); + assert_eq!(result.environments[0].name.as_deref(), Some("fresh")); + } + + #[test] + fn test_workspace_scope_does_not_replace_registry_cache() { + let environment = EnvironmentApi::new(); + let shared = WindowsRegistry::from(Arc::new(Conda::from(&environment))); + let refreshed = WindowsRegistry::from(Arc::new(Conda::from(&environment))); + + shared.search_result.lock().unwrap().replace(LocatorResult { + managers: vec![], + environments: vec![PythonEnvironment { + name: Some("stale".to_string()), + ..Default::default() + }], + }); + refreshed + .search_result + .lock() + .unwrap() + .replace(LocatorResult { + managers: vec![], + environments: vec![PythonEnvironment { + name: Some("fresh".to_string()), + ..Default::default() + }], + }); + + shared.sync_refresh_state_from(&refreshed, &RefreshStateSyncScope::Workspace); + + let result = shared.search_result.lock().unwrap().clone().unwrap(); + assert_eq!(result.environments[0].name.as_deref(), Some("stale")); + } +} diff --git a/crates/pet-windows-store/src/lib.rs b/crates/pet-windows-store/src/lib.rs index 77ccafc6..e37ed401 100644 --- a/crates/pet-windows-store/src/lib.rs +++ b/crates/pet-windows-store/src/lib.rs @@ -12,7 +12,9 @@ use pet_core::env::PythonEnv; use pet_core::python_environment::{PythonEnvironment, PythonEnvironmentKind}; use pet_core::reporter::Reporter; use pet_core::LocatorKind; -use pet_core::{os_environment::Environment, Locator}; +use pet_core::{ + os_environment::Environment, Locator, RefreshStatePersistence, RefreshStateSyncScope, +}; use std::path::Path; use std::sync::{Arc, RwLock}; @@ -52,12 +54,41 @@ impl WindowsStore { fn clear(&self) { self.environments.write().unwrap().take(); } + + fn sync_environments_from(&self, source: &WindowsStore) { + let environments = source.environments.read().unwrap().clone(); + self.environments.write().unwrap().clone_from(&environments); + } } impl Locator for WindowsStore { fn get_kind(&self) -> LocatorKind { LocatorKind::WindowsStore } + fn refresh_state(&self) -> RefreshStatePersistence { + RefreshStatePersistence::SyncedDiscoveryState + } + fn sync_refresh_state_from(&self, source: &dyn Locator, scope: &RefreshStateSyncScope) { + let source = source + .as_any() + .downcast_ref::() + .unwrap_or_else(|| { + panic!( + "attempted to sync WindowsStore state from {:?}", + source.get_kind() + ) + }); + + match scope { + RefreshStateSyncScope::Full => self.sync_environments_from(source), + RefreshStateSyncScope::GlobalFiltered(kind) + if self.supported_categories().contains(kind) => + { + self.sync_environments_from(source) + } + RefreshStateSyncScope::GlobalFiltered(_) | RefreshStateSyncScope::Workspace => {} + } + } fn supported_categories(&self) -> Vec { vec![PythonEnvironmentKind::WindowsStore] } @@ -141,3 +172,67 @@ impl Locator for WindowsStore { // } } + +#[cfg(test)] +mod tests { + use super::*; + use pet_core::os_environment::EnvironmentApi; + + #[test] + fn test_full_refresh_sync_replaces_store_cache() { + let environment = EnvironmentApi::new(); + let shared = WindowsStore::from(&environment); + let refreshed = WindowsStore::from(&environment); + + shared + .environments + .write() + .unwrap() + .replace(vec![PythonEnvironment { + name: Some("stale".to_string()), + ..Default::default() + }]); + refreshed + .environments + .write() + .unwrap() + .replace(vec![PythonEnvironment { + name: Some("fresh".to_string()), + ..Default::default() + }]); + + shared.sync_refresh_state_from(&refreshed, &RefreshStateSyncScope::Full); + + let result = shared.environments.read().unwrap().clone().unwrap(); + assert_eq!(result[0].name.as_deref(), Some("fresh")); + } + + #[test] + fn test_workspace_scope_does_not_replace_store_cache() { + let environment = EnvironmentApi::new(); + let shared = WindowsStore::from(&environment); + let refreshed = WindowsStore::from(&environment); + + shared + .environments + .write() + .unwrap() + .replace(vec![PythonEnvironment { + name: Some("stale".to_string()), + ..Default::default() + }]); + refreshed + .environments + .write() + .unwrap() + .replace(vec![PythonEnvironment { + name: Some("fresh".to_string()), + ..Default::default() + }]); + + shared.sync_refresh_state_from(&refreshed, &RefreshStateSyncScope::Workspace); + + let result = shared.environments.read().unwrap().clone().unwrap(); + assert_eq!(result[0].name.as_deref(), Some("stale")); + } +} diff --git a/crates/pet/src/jsonrpc.rs b/crates/pet/src/jsonrpc.rs index 012ef3d2..8e4703ac 100644 --- a/crates/pet/src/jsonrpc.rs +++ b/crates/pet/src/jsonrpc.rs @@ -18,7 +18,7 @@ use pet_core::telemetry::TelemetryEvent; use pet_core::{ os_environment::{Environment, EnvironmentApi}, reporter::Reporter, - Configuration, Locator, + Configuration, Locator, RefreshStatePersistence, RefreshStateSyncScope, }; use pet_env_var_path::get_search_paths_from_env_variables; use pet_fs::glob::expand_glob_patterns; @@ -316,7 +316,6 @@ pub struct Context { configuration: RwLock, locators: Arc>>, conda_locator: Arc, - poetry_locator: Arc, os_environment: Arc, refresh_coordinator: RefreshCoordinator, } @@ -336,7 +335,6 @@ pub fn start_jsonrpc_server() { let context = Context { locators: create_locators(conda_locator.clone(), poetry_locator.clone(), &environment), conda_locator, - poetry_locator, configuration: RwLock::new(ConfigurationState::default()), os_environment: Arc::new(environment), refresh_coordinator: RefreshCoordinator::default(), @@ -522,23 +520,44 @@ fn create_refresh_locators(environment: &dyn Environment) -> RefreshLocators { } } -fn sync_conda_locator_state(target: &Conda, source: &Conda) { - target.environments.clear(); - target - .environments - .insert_many(source.environments.clone_map()); +fn sync_refresh_locator_state( + target_locators: &[Arc], + source_locators: &[Arc], + search_scope: Option<&SearchScope>, +) { + let sync_scope = refresh_state_sync_scope(search_scope); + + assert_eq!( + target_locators.len(), + source_locators.len(), + "refresh locator graphs drifted" + ); + + for (target, source) in target_locators.iter().zip(source_locators.iter()) { + assert_eq!( + target.get_kind(), + source.get_kind(), + "refresh locator order drifted" + ); - target.managers.clear(); - target.managers.insert_many(source.managers.clone_map()); + if !matches!(target.refresh_state(), RefreshStatePersistence::Stateless) { + trace!( + "Applying refresh state contract for locator {:?}: {:?}", + target.get_kind(), + target.refresh_state() + ); + } - target.mamba_managers.clear(); - target - .mamba_managers - .insert_many(source.mamba_managers.clone_map()); + target.sync_refresh_state_from(source.as_ref(), &sync_scope); + } } -fn sync_poetry_locator_state(target: &Poetry, source: &Poetry) { - target.sync_search_result_from(source); +fn refresh_state_sync_scope(search_scope: Option<&SearchScope>) -> RefreshStateSyncScope { + match search_scope { + Some(SearchScope::Workspace) => RefreshStateSyncScope::Workspace, + Some(SearchScope::Global(kind)) => RefreshStateSyncScope::GlobalFiltered(*kind), + None => RefreshStateSyncScope::Full, + } } fn execute_refresh( @@ -577,7 +596,7 @@ fn execute_refresh( config, &refresh_locators.locators, context.os_environment.deref(), - search_scope, + search_scope.clone(), ); let summary = summary.lock().expect("summary mutex poisoned"); for locator in summary.locators.iter() { @@ -588,15 +607,12 @@ fn execute_refresh( } trace!("Finished refreshing environments in {:?}", summary.total); - // Refresh runs on a transient locator graph, so copy back any discovery state - // that later JSONRPC requests rely on from the long-lived shared locators. - sync_conda_locator_state( - context.conda_locator.as_ref(), - refresh_locators.conda_locator.as_ref(), - ); - sync_poetry_locator_state( - context.poetry_locator.as_ref(), - refresh_locators.poetry_locator.as_ref(), + // Refresh runs on a transient locator graph, so apply each locator's refresh-state + // contract back into the long-lived shared locator graph. + sync_refresh_locator_state( + context.locators.as_ref(), + refresh_locators.locators.as_ref(), + search_scope.as_ref(), ); let perf = RefreshPerformance { @@ -946,6 +962,7 @@ mod tests { use super::*; use pet_conda::manager::CondaManager; use pet_core::manager::EnvManagerType; + use pet_core::RefreshStatePersistence; use std::path::PathBuf; use std::sync::mpsc; use std::thread; @@ -1057,7 +1074,7 @@ mod tests { } #[test] - fn test_sync_conda_locator_state_replaces_shared_caches() { + fn test_conda_refresh_state_sync_replaces_shared_caches() { let environment = EnvironmentApi::new(); let shared = Conda::from(&environment); let refreshed = Conda::from(&environment); @@ -1127,7 +1144,12 @@ mod tests { }, ); - sync_conda_locator_state(&shared, &refreshed); + assert_eq!( + shared.refresh_state(), + RefreshStatePersistence::SyncedDiscoveryState + ); + + shared.sync_refresh_state_from(&refreshed, &RefreshStateSyncScope::Full); assert_eq!(shared.environments.len(), 1); assert!(!shared.environments.contains_key(&stale_env_path)); @@ -1142,6 +1164,88 @@ mod tests { assert!(shared.mamba_managers.contains_key(&fresh_mamba_path)); } + #[test] + fn test_workspace_refresh_does_not_replace_shared_conda_state() { + let environment = EnvironmentApi::new(); + let shared = Arc::new(Conda::from(&environment)); + let refreshed = Arc::new(Conda::from(&environment)); + + let stale_env_path = PathBuf::from("/stale/env"); + let fresh_env_path = PathBuf::from("/fresh/env"); + + shared.environments.insert( + stale_env_path.clone(), + PythonEnvironment::new( + Some(stale_env_path.join("python")), + Some(PythonEnvironmentKind::Conda), + Some(stale_env_path.clone()), + None, + Some("3.10.0".to_string()), + ), + ); + refreshed.environments.insert( + fresh_env_path.clone(), + PythonEnvironment::new( + Some(fresh_env_path.join("python")), + Some(PythonEnvironmentKind::Conda), + Some(fresh_env_path.clone()), + None, + Some("3.11.0".to_string()), + ), + ); + + sync_refresh_locator_state( + &[shared.clone() as Arc], + &[refreshed as Arc], + Some(&SearchScope::Workspace), + ); + + assert_eq!(shared.environments.len(), 1); + assert!(shared.environments.contains_key(&stale_env_path)); + assert!(!shared.environments.contains_key(&fresh_env_path)); + } + + #[test] + fn test_kind_filtered_refresh_skips_unrelated_conda_state_sync() { + let environment = EnvironmentApi::new(); + let shared = Arc::new(Conda::from(&environment)); + let refreshed = Arc::new(Conda::from(&environment)); + + let stale_env_path = PathBuf::from("/stale/env"); + let fresh_env_path = PathBuf::from("/fresh/env"); + + shared.environments.insert( + stale_env_path.clone(), + PythonEnvironment::new( + Some(stale_env_path.join("python")), + Some(PythonEnvironmentKind::Conda), + Some(stale_env_path.clone()), + None, + Some("3.10.0".to_string()), + ), + ); + refreshed.environments.insert( + fresh_env_path.clone(), + PythonEnvironment::new( + Some(fresh_env_path.join("python")), + Some(PythonEnvironmentKind::Conda), + Some(fresh_env_path.clone()), + None, + Some("3.11.0".to_string()), + ), + ); + + sync_refresh_locator_state( + &[shared.clone() as Arc], + &[refreshed as Arc], + Some(&SearchScope::Global(PythonEnvironmentKind::Venv)), + ); + + assert_eq!(shared.environments.len(), 1); + assert!(shared.environments.contains_key(&stale_env_path)); + assert!(!shared.environments.contains_key(&fresh_env_path)); + } + #[test] fn test_refresh_coordinator_does_not_join_different_generations() { let coordinator = RefreshCoordinator::default();