diff --git a/src/cli/global/install.rs b/src/cli/global/install.rs index e52a0a53c..6f5e59f22 100644 --- a/src/cli/global/install.rs +++ b/src/cli/global/install.rs @@ -1,5 +1,3 @@ -use std::str::FromStr; - use clap::Parser; use fancy_display::FancyDisplay; use indexmap::IndexMap; @@ -10,10 +8,9 @@ use rattler_conda_types::{MatchSpec, NamedChannelOrUrl, PackageName, Platform}; use crate::{ cli::{global::revert_environment_after_error, has_specs::HasSpecs}, global::{ - self, common::NotChangedReason, list::list_global_environments, EnvChanges, EnvState, - EnvironmentName, ExposedName, Mapping, Project, StateChange, StateChanges, + self, common::NotChangedReason, list::list_global_environments, project::ExposedType, + EnvChanges, EnvState, EnvironmentName, Mapping, Project, StateChange, StateChanges, }, - prefix::Prefix, }; use pixi_config::{self, Config, ConfigCli}; @@ -190,41 +187,10 @@ async fn setup_environment( // Installing the environment to be able to find the bin paths later project.install_environment(env_name).await?; - // Cleanup removed executables - state_changes |= project.remove_broken_expose_names(env_name).await?; + // Sync exposed binaries + let expose_type = ExposedType::from_mappings(args.expose.clone()); - if args.expose.is_empty() { - // Add the expose binaries for all the packages that were requested to the manifest - for (package_name, _spec) in &specs { - let prefix = project.environment_prefix(env_name).await?; - let prefix_package = prefix.find_designated_package(package_name).await?; - let package_executables = prefix.find_executables(&[prefix_package]); - for (executable_name, _) in &package_executables { - let mapping = Mapping::new( - ExposedName::from_str(executable_name)?, - executable_name.clone(), - ); - project.manifest.add_exposed_mapping(env_name, &mapping)?; - } - // If no executables were found, automatically expose the package name itself from the other packages. - // This is useful for packages like `ansible` and `jupyter` which don't ship executables their own executables. - if !package_executables - .iter() - .any(|(name, _)| name.as_str() == package_name.as_normalized()) - { - if let Some((mapping, source_package_name)) = - find_binary_by_name(&prefix, package_name).await? - { - project.manifest.add_exposed_mapping(env_name, &mapping)?; - tracing::warn!( - "Automatically exposed `{}` from `{}`", - console::style(mapping.exposed_name()).yellow(), - console::style(source_package_name.as_normalized()).green() - ); - } - } - } - } + project.sync_exposed_names(env_name, expose_type).await?; // Figure out added packages and their corresponding versions let specs = specs.values().cloned().collect_vec(); @@ -238,28 +204,3 @@ async fn setup_environment( project.manifest.save().await?; Ok(state_changes) } - -/// 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. -async fn find_binary_by_name( - prefix: &Prefix, - package_name: &PackageName, -) -> miette::Result> { - let installed_packages = prefix.find_installed_packages(None).await?; - for package in &installed_packages { - let executables = prefix.find_executables(&[package.clone()]); - - // Check if any of the executables match the package name - if let Some(executable) = executables - .iter() - .find(|(name, _)| name.as_str() == package_name.as_normalized()) - { - return Ok(Some(( - Mapping::new(ExposedName::from_str(&executable.0)?, executable.0.clone()), - package.repodata_record.package_record.name.clone(), - ))); - } - } - Ok(None) -} diff --git a/src/cli/global/update.rs b/src/cli/global/update.rs index 8cbdb5a7b..12eecafd4 100644 --- a/src/cli/global/update.rs +++ b/src/cli/global/update.rs @@ -1,7 +1,10 @@ use crate::cli::global::revert_environment_after_error; +use crate::global::common::check_all_exposed; +use crate::global::project::ExposedType; use crate::global::{self, StateChanges}; use crate::global::{EnvironmentName, Project}; use clap::Parser; +use fancy_display::FancyDisplay; use pixi_config::{Config, ConfigCli}; /// Updates environments in the global environment. @@ -26,11 +29,32 @@ pub async fn execute(args: Args) -> miette::Result<()> { ) -> miette::Result { let mut state_changes = StateChanges::default(); + // See what executables were installed prior to update + let env_binaries = project.executables(env_name).await?; + + // Get the exposed binaries from mapping + let exposed_mapping_binaries = project + .environment(env_name) + .ok_or_else(|| miette::miette!("Environment {} not found", env_name.fancy_display()))? + .exposed(); + + // Check if they were all auto-exposed, or if the user manually exposed a subset of them + let expose_type = if check_all_exposed(&env_binaries, exposed_mapping_binaries) { + ExposedType::default() + } else { + ExposedType::subset() + }; + // Reinstall the environment project.install_environment(env_name).await?; - // Remove broken executables - state_changes |= project.remove_broken_expose_names(env_name).await?; + // Sync executables exposed names with the manifest + project.sync_exposed_names(env_name, expose_type).await?; + + // Expose or prune executables of the new environment + state_changes |= project + .expose_executables_from_environment(env_name) + .await?; state_changes.insert_change(env_name, global::StateChange::UpdatedEnvironment); diff --git a/src/global/common.rs b/src/global/common.rs index 4472f4131..87efb9c54 100644 --- a/src/global/common.rs +++ b/src/global/common.rs @@ -1,16 +1,19 @@ use super::{extract_executable_from_script, EnvironmentName, ExposedName, Mapping}; +use ahash::HashSet; use console::StyledObject; use fancy_display::FancyDisplay; use fs_err as fs; use fs_err::tokio as tokio_fs; -use indexmap::IndexSet; +use indexmap::{IndexMap, IndexSet}; use is_executable::IsExecutable; use itertools::Itertools; use miette::{Context, IntoDiagnostic}; use pixi_config::home_path; use pixi_manifest::PrioritizedChannel; use pixi_utils::executable_from_path; -use rattler_conda_types::{Channel, ChannelConfig, NamedChannelOrUrl, PackageRecord, PrefixRecord}; +use rattler_conda_types::{ + Channel, ChannelConfig, NamedChannelOrUrl, PackageName, PackageRecord, PrefixRecord, +}; use std::collections::HashMap; use std::ffi::OsStr; use std::str::FromStr; @@ -534,6 +537,24 @@ pub(crate) async fn get_expose_scripts_sync_status( Ok((to_remove, to_add)) } +/// Check if all binaries were exposed, or if the user selected a subset of them. +pub fn check_all_exposed( + env_binaries: &IndexMap>, + exposed_mapping_binaries: &IndexSet, +) -> bool { + let mut env_binaries_names_iter = env_binaries.values().flatten().map(|(name, _)| name); + + let exposed_binaries_names: HashSet<&str> = exposed_mapping_binaries + .iter() + .map(|mapping| mapping.executable_name()) + .collect(); + + let auto_exposed = + env_binaries_names_iter.all(|name| exposed_binaries_names.contains(&name.as_str())); + + auto_exposed +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/global/install.rs b/src/global/install.rs index e9156f697..f209a7c23 100644 --- a/src/global/install.rs +++ b/src/global/install.rs @@ -346,6 +346,28 @@ 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( + prefix: &Prefix, + package_name: &PackageName, +) -> miette::Result> { + let installed_packages = prefix.find_installed_packages(None).await?; + for package in &installed_packages { + let executables = prefix.find_executables(&[package.clone()]); + + // Check if any of the executables match the package name + if let Some(executable) = executables + .iter() + .find(|(name, _)| name.as_str() == package_name.as_normalized()) + { + return Ok(Some(executable.clone())); + } + } + Ok(None) +} + #[cfg(test)] mod tests { use fs_err as fs; diff --git a/src/global/project/manifest.rs b/src/global/project/manifest.rs index 396e5be6d..15964723e 100644 --- a/src/global/project/manifest.rs +++ b/src/global/project/manifest.rs @@ -468,6 +468,26 @@ impl FromStr for Mapping { } } +#[derive(Default)] +pub enum ExposedType { + #[default] + All, + Subset(Vec), +} + +impl ExposedType { + pub fn from_mappings(mappings: Vec) -> Self { + match mappings.is_empty() { + true => Self::All, + false => Self::Subset(mappings), + } + } + + pub fn subset() -> Self { + Self::Subset(Default::default()) + } +} + #[cfg(test)] mod tests { use std::str::FromStr; diff --git a/src/global/project/mod.rs b/src/global/project/mod.rs index b198364a0..585358a4f 100644 --- a/src/global/project/mod.rs +++ b/src/global/project/mod.rs @@ -1,3 +1,4 @@ +use super::install::find_binary_by_name; use super::{extract_executable_from_script, BinDir, EnvRoot, StateChange, StateChanges}; use crate::global::common::{ channel_url_to_prioritized_channel, find_package_records, get_expose_scripts_sync_status, @@ -20,7 +21,7 @@ use fs_err as fs; use futures::stream::StreamExt; use indexmap::{IndexMap, IndexSet}; use itertools::Itertools; -pub(crate) use manifest::{Manifest, Mapping}; +pub(crate) use manifest::{ExposedType, Manifest, Mapping}; use miette::{miette, Context, IntoDiagnostic}; pub(crate) use parsed_manifest::ExposedName; pub(crate) use parsed_manifest::ParsedEnvironment; @@ -605,18 +606,55 @@ impl Project { Ok(state_changes) } - /// Find the exposed names that are no longer installed in the environment - /// and remove them. - /// This needs to happen after a different version of a package was installed - /// which doesn't have the same executables anymore. - pub async fn remove_broken_expose_names( + /// Get all installed executables for specific environment. + pub async fn executables( + &self, + env_name: &EnvironmentName, + ) -> miette::Result>> { + let parsed_env = self + .environment(env_name) + .ok_or_else(|| miette::miette!("Environment {} not found", env_name.fancy_display()))?; + + let package_names: Vec<_> = parsed_env.dependencies().keys().cloned().collect(); + + 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]); + + // Sometimes the package don't ship executables on their own. + // We need to search for it in different packages. + if !package_executables + .iter() + .any(|(exec_name, _)| exec_name.as_str() == package_name.as_normalized()) + { + if let Some(exec) = find_binary_by_name(&prefix, package_name).await? { + package_executables.push(exec); + } + } + + executables_for_package.insert(package_name.clone(), package_executables); + } + Ok(executables_for_package) + } + + /// Sync the `exposed` field in manifest based on the executables in the environment and the expose type. + /// Expose type can be either: + /// * If the user initially chooses to auto-exposed everything, + /// we will add new binaries that are not exposed in the `exposed` field. + /// + /// * If the use chose to expose only a subset of binaries, + /// we will remove the binaries that are not anymore present in the environment + /// and will not expose the new ones + pub async fn sync_exposed_names( &mut self, env_name: &EnvironmentName, - ) -> miette::Result { - // Figure out which package the exposed binaries belong to - let prefix = self.environment_prefix(env_name).await?; - let prefix_records = &prefix.find_installed_packages(None).await?; - let all_executables = &prefix.find_executables(prefix_records.as_slice()); + expose_type: ExposedType, + ) -> miette::Result<()> { + // Get env executables + let env_executables = self.executables(env_name).await?; // Get the parsed environment let environment = self @@ -629,8 +667,9 @@ impl Project { .iter() .filter_map(|mapping| { // If the executable is still requested, do not remove the mapping - if all_executables - .iter() + if env_executables + .values() + .flatten() .any(|(_, path)| executable_from_path(path) == mapping.executable_name()) { tracing::debug!("Not removing mapping to: {}", mapping.executable_name()); @@ -646,8 +685,27 @@ impl Project { self.manifest.remove_exposed_name(env_name, exposed_name)?; } - // Remove outdated binaries - self.prune_exposed(env_name).await + // auto-expose the executables if necessary + match expose_type { + ExposedType::All => { + // Add new binaries that are not exposed + for (executable_name, _) in env_executables.values().flatten() { + let mapping = Mapping::new( + ExposedName::from_str(executable_name)?, + executable_name.to_string(), + ); + self.manifest.add_exposed_mapping(env_name, &mapping)?; + } + } + ExposedType::Subset(mapping) => { + // Expose only the requested binaries + for mapping in mapping { + self.manifest.add_exposed_mapping(env_name, &mapping)?; + } + } + } + + Ok(()) } /// Check if the environment is in sync with the manifest diff --git a/tests/integration/test_global.py b/tests/integration/test_global.py index 22319e7b1..6189ab29f 100644 --- a/tests/integration/test_global.py +++ b/tests/integration/test_global.py @@ -1215,8 +1215,8 @@ def test_global_update_single_package( # After update be left with only the binary that was in both versions. assert package.is_file() assert not package0_1_0.is_file() - # pixi global update doesn't add new exposed mappings - assert not package0_2_0.is_file() + # pixi global update should add new exposed mappings, as all of them were exposed before + assert package0_2_0.is_file() def test_global_update_all_packages( @@ -1257,13 +1257,13 @@ def test_global_update_all_packages( assert package2.is_file() assert package.is_file() assert not package0_1_0.is_file() - # After update be left with only the binary that was in both versions. - assert not package0_2_0.is_file() + # After update be left we auto expose new binary, as all of them were exposed before + assert package0_2_0.is_file() # Check the manifest for removed binaries manifest_content = manifest.read_text() assert "package0.1.0" not in manifest_content - assert "package0.2.0" not in manifest_content + assert "package0.2.0" in manifest_content assert "package2" in manifest_content assert "package" in manifest_content @@ -1299,7 +1299,45 @@ def test_pixi_update_cleanup(pixi: Path, tmp_path: Path, global_update_channel_1 # Update the environment # The package should now have the version `0.2.0` and expose a different executable # The old executable should be removed - # The new executable will also not be there, since `pixi global update` doesn't add new exposed mappings to the manifest. + # The new executable should be there, since user initially auto-exposed all binaries and `pixi global update` should add new binary to the manifest. + verify_cli_command( + [pixi, "global", "update", "package"], + env=env, + ) + assert not package0_1_0.is_file() + assert package0_2_0.is_file() + + +def test_pixi_update_subset_expose( + pixi: Path, tmp_path: Path, global_update_channel_1: str +) -> None: + env = {"PIXI_HOME": str(tmp_path)} + + package0_1_0 = tmp_path / "bin" / exec_extension("package0.1.0") + package0_2_0 = tmp_path / "bin" / exec_extension("package0.2.0") + + verify_cli_command( + [pixi, "global", "install", "--channel", global_update_channel_1, "package==0.1.0"], + env=env, + ) + assert package0_1_0.is_file() + assert not package0_2_0.is_file() + + manifest = tmp_path.joinpath("manifests", "pixi-global.toml") + + # We change the matchspec to '*' + # So we expect to new binary to not be exposed, + # since we exposed only a small subset + parsed_toml = tomllib.loads(manifest.read_text()) + parsed_toml["envs"]["package"]["dependencies"]["package"] = "*" + parsed_toml["envs"]["package"]["exposed"] = {"package": "package0.1.0"} + + manifest.write_text(tomli_w.dumps(parsed_toml)) + + # Update the environment + # The package should now have the version `0.2.0` and expose a different executable + # The old executable should be removed + # The new executable should be there, since user initially auto-exposed all binaries and `pixi global update` should add new binary to the manifest. verify_cli_command( [pixi, "global", "update", "package"], env=env, @@ -1307,6 +1345,11 @@ def test_pixi_update_cleanup(pixi: Path, tmp_path: Path, global_update_channel_1 assert not package0_1_0.is_file() assert not package0_2_0.is_file() + # parse the manifest again + # and check that we don't have any new binary exposed + parsed_toml = tomllib.loads(manifest.read_text()) + assert "exposed" not in parsed_toml["envs"]["package"] + def test_auto_self_expose(pixi: Path, tmp_path: Path, non_self_expose_channel: str) -> None: env = {"PIXI_HOME": str(tmp_path)}