feat: global update should add new executables

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<Option<(Mapping, PackageName)>> {
-    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)
-}

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<StateChanges> {
         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);
 

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<PackageName, Vec<(String, PathBuf)>>,
+    exposed_mapping_binaries: &IndexSet<Mapping>,
+) -> 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::*;

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<Option<(String, PathBuf)>> {
+    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;

src/global/project/manifest.rs

@@ -468,6 +468,26 @@ impl FromStr for Mapping {
     }
 }
 
+#[derive(Default)]
+pub enum ExposedType {
+    #[default]
+    All,
+    Subset(Vec<Mapping>),
+}
+
+impl ExposedType {
+    pub fn from_mappings(mappings: Vec<Mapping>) -> 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;

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<IndexMap<PackageName, Vec<(String, PathBuf)>>> {
+        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<StateChanges> {
-        // 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