diff --git a/Cargo.lock b/Cargo.lock index ae950ace3c..c22e71fe09 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6891,6 +6891,7 @@ dependencies = [ "rattler_virtual_packages", "regex", "rstest", + "same-file", "serde", "serde_derive", "serde_json", diff --git a/crates/pixi_cli/src/global/install.rs b/crates/pixi_cli/src/global/install.rs index a77ca9a5a2..a41d5f01e7 100644 --- a/crates/pixi_cli/src/global/install.rs +++ b/crates/pixi_cli/src/global/install.rs @@ -248,9 +248,19 @@ async fn setup_environment( // Add shortcuts if !args.no_shortcuts { let prefix = project.environment_prefix(env_name).await?; + let prefix_records = project.environment_prefix_records(env_name).await?; for spec in specs.iter() { - let prefix_record = prefix.find_designated_package(spec.name()).await?; - if contains_menuinst_document(&prefix_record, prefix.root()) { + let prefix_record = prefix_records + .iter() + .find(|record| record.name() == spec.name()) + .ok_or_else(|| { + miette::miette!( + "could not find package {} in environment {}", + spec.name().as_source(), + env_name.fancy_display() + ) + })?; + if contains_menuinst_document(prefix_record, prefix.root()) { project.manifest.add_shortcut(env_name, spec.name())?; } } diff --git a/crates/pixi_cli/src/global/remove.rs b/crates/pixi_cli/src/global/remove.rs index 566fe3dba1..a90e1f07f0 100644 --- a/crates/pixi_cli/src/global/remove.rs +++ b/crates/pixi_cli/src/global/remove.rs @@ -71,14 +71,15 @@ pub async fn execute(args: Args) -> miette::Result<()> { // Figure out which package the exposed binaries belong to let prefix = project.environment_prefix(env_name).await?; + let prefix_records = project.environment_prefix_records(env_name).await?; for spec in specs { { let name = spec.name.as_exact().expect("package name must be exact"); // If the package is not existent, don't try to remove executables - if let Ok(record) = prefix.find_designated_package(name).await { + if let Some(record) = prefix_records.iter().find(|record| record.name() == name) { prefix - .find_executables(&[record]) + .find_executables(std::slice::from_ref(record)) .into_iter() .filter_map(|executable| { ExposedName::from_str(executable.name.as_str()).ok() diff --git a/crates/pixi_global/Cargo.toml b/crates/pixi_global/Cargo.toml index 74df9ed142..ff34c1b05e 100644 --- a/crates/pixi_global/Cargo.toml +++ b/crates/pixi_global/Cargo.toml @@ -45,6 +45,7 @@ rattler_shell = { workspace = true } rattler_virtual_packages = { workspace = true } regex = { workspace = true } rstest = { workspace = true } +same-file = { workspace = true } serde = { workspace = true } serde_derive = { workspace = true } serde_json = { workspace = true } diff --git a/crates/pixi_global/src/common.rs b/crates/pixi_global/src/common.rs index a6fb0858c4..d722399d52 100644 --- a/crates/pixi_global/src/common.rs +++ b/crates/pixi_global/src/common.rs @@ -183,36 +183,31 @@ pub(crate) fn is_binary(file_path: impl AsRef) -> miette::Result { Ok(buffer[..bytes_read].contains(&0)) } -/// Finds the package record from the `conda-meta` directory. +/// Finds the package records from the `conda-meta` directory. +/// +/// The records are parsed in parallel on the blocking thread pool since the +/// record files of a single environment can add up to tens of MBs of JSON. pub async fn find_package_records(conda_meta: &Path) -> miette::Result> { - let read_dir = tokio_fs::read_dir(conda_meta).await; - let mut records = Vec::new(); - - let mut read_dir = match read_dir { - Ok(dir) => dir, - Err(e) => match e.kind() { - std::io::ErrorKind::NotFound => return Ok(records), - _ => miette::bail!( - "Failed to read conda-meta directory {}: {}", - conda_meta.display(), - e - ), - }, + let Some(prefix_root) = conda_meta + .file_name() + .is_some_and(|name| name == OsStr::new(pixi_consts::consts::CONDA_META_DIR)) + .then(|| conda_meta.parent()) + .flatten() + .map(Path::to_path_buf) + else { + miette::bail!( + "Expected a path to a `{}` directory, got {}", + pixi_consts::consts::CONDA_META_DIR, + conda_meta.display() + ); }; - while let Some(entry) = read_dir.next_entry().await.into_diagnostic()? { - let path = entry.path(); - // Check if the entry is a file and has a .json extension - if path.is_file() && path.extension().and_then(OsStr::to_str) == Some("json") { - let prefix_record = PrefixRecord::from_path(&path) - .into_diagnostic() - .wrap_err_with(|| format!("Couldn't parse json from {}", path.display()))?; - - records.push(prefix_record); - } - } - - Ok(records) + let prefix = pixi_utils::prefix::Prefix::new(prefix_root); + tokio::task::spawn_blocking(move || prefix.find_installed_packages()) + .await + .into_diagnostic()? + .into_diagnostic() + .wrap_err_with(|| format!("Couldn't parse records from {}", conda_meta.display())) } #[derive(Debug, Clone, PartialEq, Eq)] @@ -440,12 +435,15 @@ impl StateChanges { | InstallChange::Reinstalled(_, _) ) { // Get the package record from the environment prefix - let prefix = project.environment_prefix(env_name).await?; - if let Ok(prefix_record) = prefix.find_designated_package(&package_name).await { + let prefix_records = project.environment_prefix_records(env_name).await?; + if let Some(prefix_record) = prefix_records + .iter() + .find(|record| record.name() == &package_name) + { self.insert_change( env_name, StateChange::AddedPackage(Box::new( - prefix_record.repodata_record.package_record, + prefix_record.repodata_record.package_record.clone(), )), ); } @@ -884,17 +882,17 @@ pub(crate) fn channel_url_to_prioritized_channel( /// contain menuinst JSON files. It then compares these records with the /// requested `shortcuts` to determine which records need to be installed and /// which need to be uninstalled. -pub(crate) fn shortcuts_sync_status( +pub(crate) fn shortcuts_sync_status<'a>( shortcuts: IndexSet, - prefix_records: Vec, + prefix_records: &'a [PrefixRecord], prefix_root: &Path, -) -> miette::Result<(Vec, Vec)> { +) -> miette::Result<(Vec<&'a PrefixRecord>, Vec<&'a PrefixRecord>)> { let mut remaining_shortcuts = shortcuts; let mut records_to_install = Vec::new(); let mut records_to_uninstall = Vec::new(); let records_with_menuinst = prefix_records - .into_iter() + .iter() .filter(|record| contains_menuinst_document(record, prefix_root)); for record in records_with_menuinst { diff --git a/crates/pixi_global/src/install.rs b/crates/pixi_global/src/install.rs index b0a629653f..d823999b70 100644 --- a/crates/pixi_global/src/install.rs +++ b/crates/pixi_global/src/install.rs @@ -11,7 +11,7 @@ use pixi_utils::{ prefix::{Executable, Prefix}, }; use rattler_conda_types::{ - MatchSpec, Matches, PackageName, PackageRecord, ParseMatchSpecOptions, Platform, + MatchSpec, Matches, PackageName, PackageRecord, ParseMatchSpecOptions, Platform, PrefixRecord, RepodataRevision, }; use rattler_shell::activation::prefix_path_entries; @@ -344,12 +344,12 @@ pub(crate) fn local_environment_matches_spec( /// Finds the package name in the prefix and automatically exposes it if an executable is found. /// This is useful for packages like `ansible` and `jupyter` which don't ship executables their own executables. /// This function will return the mapping and the package name of the package in which the binary was found. -pub async fn find_binary_by_name( +pub fn find_binary_by_name( prefix: &Prefix, package_name: &PackageName, -) -> miette::Result> { - let installed_packages = prefix.find_installed_packages()?; - for package in &installed_packages { + installed_packages: &[PrefixRecord], +) -> Option { + for package in installed_packages { let executables = prefix.find_executables(std::slice::from_ref(package)); // Check if any of the executables match the package name @@ -357,10 +357,10 @@ pub async fn find_binary_by_name( .iter() .find(|executable| executable.name.as_str() == package_name.as_normalized()) { - return Ok(Some(executable.clone())); + return Some(executable.clone()); } } - Ok(None) + None } #[cfg(test)] diff --git a/crates/pixi_global/src/list.rs b/crates/pixi_global/src/list.rs index 3829a9cf2d..df5c7ca213 100644 --- a/crates/pixi_global/src/list.rs +++ b/crates/pixi_global/src/list.rs @@ -4,14 +4,12 @@ use fancy_display::FancyDisplay; use indexmap::{IndexMap, IndexSet}; use itertools::Itertools; use miette::{IntoDiagnostic, miette}; -use pixi_consts::consts; use pixi_core::environment::list::{PackageToOutput, print_package_table}; use pixi_spec::PixiSpec; use rattler_conda_types::{PackageName, PrefixRecord, Version}; use serde::Serialize; use super::{EnvChanges, EnvState, EnvironmentName, Mapping, Project, project::ParsedEnvironment}; -use crate::common::find_package_records; /// JSON-serializable representation of an exposed mapping. #[derive(Serialize)] @@ -65,9 +63,7 @@ pub async fn list_global_environments_json( let mut environments = Vec::new(); for (env_name, env) in project_envs.iter() { - let env_dir = project.env_root.path().join(env_name.as_str()); - let conda_meta = env_dir.join(consts::CONDA_META_DIR); - let records = find_package_records(&conda_meta).await?; + let records = project.environment_prefix_records(env_name).await?; let dependencies = env .dependencies @@ -230,14 +226,7 @@ pub async fn list_specific_global_environment( .get(environment_name) .ok_or_else(|| miette!("Environment {} not found", environment_name.fancy_display()))?; - let records = find_package_records( - &project - .env_root - .path() - .join(environment_name.as_str()) - .join(consts::CONDA_META_DIR), - ) - .await?; + let records = project.environment_prefix_records(environment_name).await?; let mut packages_to_output = records .iter() @@ -327,9 +316,7 @@ pub async fn list_all_global_environments( let len = project_envs.len(); for (idx, (env_name, env)) in project_envs.iter().enumerate() { - let env_dir = project.env_root.path().join(env_name.as_str()); - let conda_meta = env_dir.join(consts::CONDA_META_DIR); - let records = find_package_records(&conda_meta).await?; + let records = project.environment_prefix_records(env_name).await?; let last = (idx + 1) == len; diff --git a/crates/pixi_global/src/project/mod.rs b/crates/pixi_global/src/project/mod.rs index 50ded265c2..2c7b1626c4 100644 --- a/crates/pixi_global/src/project/mod.rs +++ b/crates/pixi_global/src/project/mod.rs @@ -1,10 +1,10 @@ use std::{ - collections::{BTreeMap, HashSet}, + collections::{BTreeMap, HashMap, HashSet}, ffi::OsStr, fmt::{Debug, Formatter}, path::{Path, PathBuf}, str::FromStr, - sync::Arc, + sync::{Arc, Mutex}, }; pub use environment::EnvironmentName; @@ -149,6 +149,15 @@ pub struct Project { top_level_progress: OnceCell>, /// Optional backend override for testing purposes backend_override: Option, + /// Cache of the parsed `conda-meta` prefix records per environment. + /// Parsing these records is expensive (the JSON files of a large + /// environment add up to tens of MBs) and a single command needs them + /// several times: the sync check, exposing executables, shortcuts and + /// completions all inspect the same records. The cache is shared between + /// clones of the project so an entry invalidated after an install is + /// re-read everywhere. Entries must be invalidated whenever the + /// environment's prefix is modified. + prefix_records_cache: Arc>>>>, } impl Debug for Project { @@ -333,6 +342,7 @@ impl Project { command_dispatcher: OnceCell::new(), top_level_progress: OnceCell::new(), backend_override: None, + prefix_records_cache: Arc::new(Mutex::new(HashMap::new())), } } @@ -543,6 +553,55 @@ impl Project { Ok(Prefix::new(self.environment_dir(env_name).await?.path())) } + /// Returns the prefix records of the environment with the given name. + /// + /// The records are parsed from the environment's `conda-meta` directory + /// once and then served from a cache shared between clones of this + /// project. Any code that modifies the environment's prefix must call + /// [`Self::invalidate_prefix_records`] afterwards. + pub async fn environment_prefix_records( + &self, + env_name: &EnvironmentName, + ) -> miette::Result>> { + if let Some(records) = self + .prefix_records_cache + .lock() + .expect("prefix records cache lock is poisoned") + .get(env_name) + { + return Ok(records.clone()); + } + + // Read the prefix directly from the environment root without going + // through `environment_prefix`, which would create the environment + // directory as a side effect. A missing directory simply yields an + // empty record list. + let prefix = Prefix::new(self.env_root.path().join(env_name.as_str())); + // Parsing happens on the blocking pool: it's rayon-parallel CPU work + // that would otherwise stall the async executor. + let records = tokio::task::spawn_blocking(move || prefix.find_installed_packages()) + .await + .into_diagnostic()? + .into_diagnostic()?; + + let records = Arc::new(records); + self.prefix_records_cache + .lock() + .expect("prefix records cache lock is poisoned") + .insert(env_name.clone(), records.clone()); + Ok(records) + } + + /// Drops the cached prefix records of an environment. Must be called + /// after the environment's prefix has been modified (packages installed + /// or removed, environment deleted). + pub fn invalidate_prefix_records(&self, env_name: &EnvironmentName) { + self.prefix_records_cache + .lock() + .expect("prefix records cache lock is poisoned") + .remove(env_name); + } + /// Create an authenticated reqwest client for this project /// use authentication from `rattler_networking` pub fn authenticated_client(&self) -> miette::Result<&LazyClient> { @@ -715,6 +774,9 @@ impl Project { }) .await?; + // The prefix changed, so any cached records are stale now. + self.invalidate_prefix_records(env_name); + self.write_environment_file( env_name, &prefix, @@ -788,6 +850,7 @@ impl Project { tokio_fs::remove_dir_all(env_dir.path()) .await .into_diagnostic()?; + self.invalidate_prefix_records(env_name); // Get all removable binaries related to the environment let (to_remove, _to_add) = @@ -844,12 +907,10 @@ impl Project { &self, env_name: &EnvironmentName, ) -> miette::Result> { - let env_dir = EnvDir::from_env_root(self.env_root.clone(), env_name).await?; - let prefix = Prefix::new(env_dir.path()); - - let prefix_records = &prefix.find_installed_packages()?; + let prefix = self.environment_prefix(env_name).await?; + let prefix_records = self.environment_prefix_records(env_name).await?; - let all_executables = find_executables_for_many_records(&prefix, prefix_records); + let all_executables = find_executables_for_many_records(&prefix, &prefix_records); Ok(all_executables) } @@ -866,19 +927,31 @@ impl Project { let package_names: Vec<_> = parsed_env.dependencies.specs.keys().cloned().collect(); + let prefix = self.environment_prefix(env_name).await?; + let prefix_records = self.environment_prefix_records(env_name).await?; + let mut executables_for_package = IndexMap::new(); for package_name in &package_names { - let prefix = self.environment_prefix(env_name).await?; - let prefix_package = prefix.find_designated_package(package_name).await?; - let mut package_executables = prefix.find_executables(&[prefix_package]); + let prefix_package = prefix_records + .iter() + .find(|record| record.name() == package_name) + .ok_or_else(|| { + miette::miette!( + "could not find package {} in environment {}", + package_name.as_source(), + env_name.fancy_display() + ) + })?; + let mut package_executables = + prefix.find_executables(std::slice::from_ref(prefix_package)); // Sometimes the package don't ship executables on their own. // We need to search for it in different packages. if !package_executables .iter() .any(|executable| executable.name.as_str() == package_name.as_normalized()) - && let Some(exec) = find_binary_by_name(&prefix, package_name).await? + && let Some(exec) = find_binary_by_name(&prefix, package_name, &prefix_records) { package_executables.push(exec); } @@ -1046,7 +1119,7 @@ impl Project { EnvDir::from_path(self.env_root.clone().path().join(env_name.clone().as_str())); let prefix = self.environment_prefix(env_name).await?; - let prefix_records = prefix.find_installed_packages()?; + let prefix_records = self.environment_prefix_records(env_name).await?; let specs_in_sync = environment_specs_in_sync( &prefix_records, &specs, @@ -1076,7 +1149,7 @@ impl Project { tracing::debug!("Verify that the shortcuts are in sync with the environment"); let shortcuts = environment.shortcuts.clone().unwrap_or_default(); let (shortcuts_to_remove, shortcuts_to_add) = - shortcuts_sync_status(shortcuts, prefix_records, prefix.root())?; + shortcuts_sync_status(shortcuts, &prefix_records, prefix.root())?; if !shortcuts_to_remove.is_empty() || !shortcuts_to_add.is_empty() { tracing::debug!( "Environment {} shortcuts are not in sync: to_remove: {}, to_add: {}", @@ -1307,6 +1380,7 @@ impl Project { tokio_fs::remove_dir_all(&env_path) .await .into_diagnostic()?; + self.invalidate_prefix_records(&env_name); // Get all removable binaries related to the environment let (to_remove, _to_add) = expose_scripts_sync_status( &self.bin_dir, @@ -1334,11 +1408,11 @@ impl Project { .ok_or_else(|| miette::miette!("Environment {} not found", env_name.fancy_display()))?; let prefix = self.environment_prefix(env_name).await?; - let prefix_records = prefix.find_installed_packages()?; + let prefix_records = self.environment_prefix_records(env_name).await?; let shortcuts = environment.shortcuts.clone().unwrap_or_default(); let (records_to_install, records_to_uninstall) = - shortcuts_sync_status(shortcuts, prefix_records, prefix.root())?; + shortcuts_sync_status(shortcuts, &prefix_records, prefix.root())?; for record in records_to_install { tracing::debug!( @@ -1347,7 +1421,7 @@ impl Project { ); rattler_menuinst::install_menuitems_for_record( prefix.root(), - &record, + record, environment.platform.unwrap_or(Platform::current()), MenuMode::User, ) @@ -1387,6 +1461,12 @@ impl Project { ); } + if state_changes.has_changed() { + // menuinst tracks the installed menu items by rewriting the + // prefix records on disk, so the cached records are stale now. + self.invalidate_prefix_records(env_name); + } + Ok(state_changes) } @@ -1398,11 +1478,10 @@ impl Project { let mut state_changes = StateChanges::default(); // Find menu items in the prefix - let prefix = self.environment_prefix(env_name).await?; - let prefix_records = prefix.find_installed_packages()?; + let prefix_records = self.environment_prefix_records(env_name).await?; // Remove menu items - for record in prefix_records { + for record in prefix_records.iter() { rattler_menuinst::remove_menu_items(&record.installed_system_menus) .into_diagnostic()?; tracing::info!("Uninstalled menu items for: '{}'", record.file_name()); diff --git a/crates/pixi_global/src/trampoline.rs b/crates/pixi_global/src/trampoline.rs index 7cd76ff53e..812f05dfc5 100644 --- a/crates/pixi_global/src/trampoline.rs +++ b/crates/pixi_global/src/trampoline.rs @@ -17,10 +17,11 @@ /// use std::{ collections::HashMap, + ffi::OsStr, io::ErrorKind, path::{Path, PathBuf}, str::FromStr, - sync::LazyLock, + sync::{LazyLock, Mutex}, }; use miette::IntoDiagnostic; @@ -368,6 +369,7 @@ impl Trampoline { tokio_fs::write(&trampoline_path, Trampoline::decompressed_trampoline()) .await .into_diagnostic()?; + mark_shared_trampoline_verified(&trampoline_path, true); } else if !Trampoline::is_trampoline(&self.trampoline_path()).await? { tokio_fs::remove_file(&trampoline_path) .await @@ -375,6 +377,7 @@ impl Trampoline { tokio_fs::write(&trampoline_path, Trampoline::decompressed_trampoline()) .await .into_diagnostic()?; + mark_shared_trampoline_verified(&trampoline_path, true); } // If the path doesn't exist yet, create a hard link to the shared trampoline binary @@ -420,9 +423,46 @@ impl Trampoline { Ok(()) } - /// Checks if executable is a saved trampoline - /// by comparing the file size and then by reading the first 1048 bytes of the file + /// Checks if executable is a saved trampoline of the current pixi + /// version. + /// + /// Exposed binaries are hard links to the shared trampoline binary, so + /// the common case is answered from file metadata alone: the shared + /// binary is compared against the embedded trampoline once per process, + /// after which any file that is the same file (hard link) as the shared + /// binary is a trampoline without reading its contents. Everything else + /// falls back to a full content comparison. pub async fn is_trampoline(path: &Path) -> miette::Result { + if let Some(shared_trampoline) = shared_trampoline_path_for(path) + && shared_trampoline.is_file() + && same_file::is_same_file(path, &shared_trampoline).unwrap_or(false) + { + return Self::shared_trampoline_is_current(&shared_trampoline).await; + } + + Self::matches_embedded_trampoline(path).await + } + + /// Checks whether the shared trampoline binary matches the trampoline + /// embedded in this pixi version, caching the answer per process. + async fn shared_trampoline_is_current(shared_trampoline: &Path) -> miette::Result { + if let Some(verified) = VERIFIED_SHARED_TRAMPOLINES + .lock() + .expect("verified trampolines lock is poisoned") + .get(shared_trampoline) + { + return Ok(*verified); + } + + let verified = Self::matches_embedded_trampoline(shared_trampoline).await?; + mark_shared_trampoline_verified(shared_trampoline, verified); + Ok(verified) + } + + /// Compares the file contents at `path` with the embedded trampoline + /// binary, comparing the file size first to avoid reading the file if + /// it can't match. + async fn matches_embedded_trampoline(path: &Path) -> miette::Result { let mut bin_file = tokio_fs::File::open(path).await.into_diagnostic()?; let metadata = bin_file.metadata().await.into_diagnostic()?; let file_size = metadata.len(); @@ -445,6 +485,42 @@ impl Trampoline { } } +/// Shared trampoline binaries whose contents have been compared against the +/// embedded trampoline in this process. The value records whether they +/// matched. +static VERIFIED_SHARED_TRAMPOLINES: LazyLock>> = + LazyLock::new(Mutex::default); + +/// Records the comparison result of a shared trampoline binary with the +/// embedded one, so subsequent [`Trampoline::is_trampoline`] calls can skip +/// re-reading it. +fn mark_shared_trampoline_verified(shared_trampoline: &Path, verified: bool) { + VERIFIED_SHARED_TRAMPOLINES + .lock() + .expect("verified trampolines lock is poisoned") + .insert(shared_trampoline.to_path_buf(), verified); +} + +/// Returns the path of the shared trampoline binary that would back the +/// given exposed executable. For the shared binary itself this returns the +/// path unchanged. +fn shared_trampoline_path_for(path: &Path) -> Option { + let is_shared = path.file_name() == Some(OsStr::new(TRAMPOLINE_BIN_NAME)) + && path + .parent() + .and_then(Path::file_name) + .is_some_and(|parent| parent == OsStr::new(TRAMPOLINE_CONFIGURATION)); + if is_shared { + Some(path.to_path_buf()) + } else { + path.parent().map(|parent| { + parent + .join(TRAMPOLINE_CONFIGURATION) + .join(TRAMPOLINE_BIN_NAME) + }) + } +} + mod tests { // Test is_trampoline when it is a trampoline #[tokio::test]