fix: trampoline without corresponding json breaking

src/global/common.rs

@@ -1,4 +1,4 @@
-use super::trampoline::{GlobalBin, Trampoline};
+use super::trampoline::{GlobalExecutable, Trampoline};
 use super::{EnvironmentName, ExposedName, Mapping};
 use crate::prefix::Executable;
 
@@ -22,6 +22,7 @@ use rattler_conda_types::{
 use std::collections::HashMap;
 use std::ffi::OsStr;
 use std::iter::Peekable;
+use std::ops::Not;
 use std::str::FromStr;
 use std::{
     io::Read,
@@ -57,19 +58,18 @@ impl BinDir {
     ///
     /// This function reads the directory specified by `self.0` and try to collect all
     /// file paths into a vector. It returns a `miette::Result` containing the
-    /// vector of `GlobalBin`or an error if the directory can't be read.
-    pub(crate) async fn bins(&self) -> miette::Result<Vec<GlobalBin>> {
+    /// vector of `GlobalExecutable`or an error if the directory can't be read.
+    pub(crate) async fn executables(&self) -> miette::Result<Vec<GlobalExecutable>> {
         let mut files = Vec::new();
         let mut entries = tokio_fs::read_dir(&self.0).await.into_diagnostic()?;
 
         while let Some(entry) = entries.next_entry().await.into_diagnostic()? {
             let path = entry.path();
-            if path.is_file() && path.is_executable() && Trampoline::is_trampoline(&path).await? {
-                let trampoline = Trampoline::try_from(path).await?;
-                files.push(GlobalBin::Trampoline(trampoline));
-            } else if path.is_file() && path.is_executable() && !is_binary(&path)? {
+            if let Ok(trampoline) = Trampoline::try_from(&path).await {
+                files.push(GlobalExecutable::Trampoline(trampoline));
+            } else if path.is_file() && path.is_executable() && is_binary(&path)?.not() {
                 // If the file is not a binary, it's a script
-                files.push(GlobalBin::Script(path));
+                files.push(GlobalExecutable::Script(path));
             }
         }
 
@@ -704,9 +704,9 @@ pub(crate) async fn get_expose_scripts_sync_status(
     bin_dir: &BinDir,
     env_dir: &EnvDir,
     mappings: &IndexSet<Mapping>,
-) -> miette::Result<(Vec<GlobalBin>, IndexSet<ExposedName>)> {
+) -> miette::Result<(Vec<GlobalExecutable>, IndexSet<ExposedName>)> {
     // Get all paths to the binaries from trampolines or scripts in the bin directory.
-    let locally_exposed = bin_dir.bins().await?;
+    let locally_exposed = bin_dir.executables().await?;
     let executable_paths = futures::future::join_all(locally_exposed.iter().map(|global_bin| {
         let global_bin = global_bin.clone();
         let path = global_bin.path().clone();

src/global/install.rs

@@ -107,6 +107,7 @@ pub(crate) async fn create_executable_trampolines(
         original_executable,
     } in mapped_executables
     {
+        tracing::debug!("Create trampoline {}", global_script_path.display());
         let exe = prefix.root().join(original_executable);
         let path = prefix
             .root()
@@ -118,7 +119,14 @@ pub(crate) async fn create_executable_trampolines(
             })?);
         let metadata = Configuration::new(exe, path, Some(activation_variables.clone()));
 
-        let json_path = Configuration::trampoline_configuration(global_script_path);
+        let parent_dir = global_script_path.parent().ok_or_else(|| {
+            miette::miette!(
+                "{} needs to have a parent directory",
+                global_script_path.display()
+            )
+        })?;
+        let exposed_name = Trampoline::name(global_script_path)?;
+        let json_path = Configuration::path_from_trampoline(parent_dir, &exposed_name);
 
         // Check if an old bash script is present and remove it
         let mut changed = if global_script_path.exists()
@@ -463,7 +471,7 @@ mod tests {
     #[cfg(windows)]
     #[tokio::test]
     async fn test_extract_executable_from_script_windows() {
-        use crate::global::trampoline::GlobalBin;
+        use crate::global::trampoline::GlobalExecutable;
         use std::fs;
         use std::path::Path;
         let script_without_quote = r#"
@@ -475,7 +483,7 @@ mod tests {
         let tempdir = tempfile::tempdir().unwrap();
         let script_path = tempdir.path().join(script_path);
         fs::write(&script_path, script_without_quote).unwrap();
-        let script_global_bin = GlobalBin::Script(script_path);
+        let script_global_bin = GlobalExecutable::Script(script_path);
         let executable_path = script_global_bin.executable().await.unwrap();
         assert_eq!(
             executable_path,
@@ -502,7 +510,7 @@ mod tests {
     async fn test_extract_executable_from_script_unix() {
         use std::{fs, path::Path};
 
-        use crate::global::trampoline::GlobalBin;
+        use crate::global::trampoline::GlobalExecutable;
 
         let script = r#"#!/bin/sh
 export PATH="/home/user/.pixi/envs/nushell/bin:${PATH}"
@@ -513,7 +521,7 @@ export CONDA_PREFIX="/home/user/.pixi/envs/nushell"
         let tempdir = tempfile::tempdir().unwrap();
         let script_path = tempdir.path().join(script_path);
         fs::write(&script_path, script).unwrap();
-        let script_global_bin = GlobalBin::Script(script_path);
+        let script_global_bin = GlobalExecutable::Script(script_path);
         let executable_path = script_global_bin.executable().await.unwrap();
         assert_eq!(
             executable_path,

src/global/project/mod.rs

@@ -1,6 +1,8 @@
+use self::trampoline::{Configuration, ConfigurationParseError, Trampoline};
+
 use super::common::{get_install_changes, EnvironmentUpdate};
 use super::install::find_binary_by_name;
-use super::trampoline::GlobalBin;
+use super::trampoline::{self, GlobalExecutable};
 use super::{BinDir, EnvRoot, StateChange, StateChanges};
 use crate::global::common::{
     channel_url_to_prioritized_channel, find_package_records, get_expose_scripts_sync_status,
@@ -22,6 +24,7 @@ use fs::tokio as tokio_fs;
 use fs_err as fs;
 use futures::stream::StreamExt;
 use indexmap::{IndexMap, IndexSet};
+use is_executable::IsExecutable;
 use itertools::Itertools;
 pub(crate) use manifest::{ExposedType, Manifest, Mapping};
 use miette::{miette, Context, IntoDiagnostic};
@@ -113,7 +116,7 @@ impl ExposedData {
     /// reading the associated `conda-meta` directory.
     /// or it looks into the trampoline manifest to extract the metadata.
     pub async fn from_exposed_path(
-        bin: &GlobalBin,
+        bin: &GlobalExecutable,
         env_root: &EnvRoot,
         channel_config: &ChannelConfig,
     ) -> miette::Result<Self> {
@@ -286,17 +289,25 @@ impl Project {
         let env_root = EnvRoot::from_env().await?;
 
         if !manifest_path.exists() {
+            tracing::debug!(
+                "Global manifest {} doesn't exist yet. Creating a new one.",
+                manifest_path.display()
+            );
             tokio_fs::create_dir_all(&manifest_dir)
                 .await
                 .into_diagnostic()?;
 
             if !env_root.directories().await?.is_empty() {
+                tracing::debug!(
+                    "Existing installation found. Creating global manifest from that information."
+                );
                 return Self::try_from_existing_installation(&manifest_path, env_root, bin_dir)
                     .await
                     .wrap_err_with(|| {
                         "Failed to create global manifest from existing installation"
                     });
             } else {
+                tracing::debug!("Create an empty global manifest.");
                 tokio_fs::File::create(&manifest_path)
                     .await
                     .into_diagnostic()?;
@@ -314,7 +325,7 @@ impl Project {
         let config = Config::load(env_root.path());
 
         let exposed_binaries: Vec<ExposedData> = bin_dir
-            .bins()
+            .executables()
             .await?
             .into_iter()
             .map(|bin| {
@@ -793,7 +804,7 @@ impl Project {
             return Ok(false);
         }
 
-        // Verify the binaries to be in sync with the environment
+        tracing::debug!("Verify that the binaries are in sync with the environment");
         let (to_remove, to_add) =
             get_expose_scripts_sync_status(&self.bin_dir, &env_dir, &environment.exposed).await?;
         if !to_remove.is_empty() || !to_add.is_empty() {
@@ -896,9 +907,9 @@ impl Project {
         // Prune environments that are not listed
         state_changes |= self.prune_old_environments().await?;
 
-        // Remove broken scripts
-        if let Err(err) = self.remove_broken_bins().await {
-            tracing::warn!("Couldn't remove broken exposed executables: {err}")
+        // Remove broken files
+        if let Err(err) = self.remove_broken_files().await {
+            tracing::warn!("Couldn't remove broken files: {err}")
         }
 
         for env_name in self.environments().keys() {
@@ -916,9 +927,14 @@ impl Project {
         removed_packages: Option<Vec<PackageName>>,
     ) -> miette::Result<StateChanges> {
         let mut state_changes = StateChanges::new_with_env(env_name.clone());
-        if !self.environment_in_sync(env_name).await? {
+        if self.environment_in_sync(env_name).await? {
+            tracing::debug!(
+                "Environment {} specs already up to date with global manifest",
+                env_name.fancy_display()
+            );
+        } else {
             tracing::debug!(
-                "Environment {} specs not up to date with manifest",
+                "Environment {} specs not up to date with global manifest",
                 env_name.fancy_display()
             );
             let mut environment_update = self.install_environment(env_name).await?;
@@ -940,37 +956,36 @@ impl Project {
     }
 
     /// Delete scripts in the bin folder that are broken
-    pub(crate) async fn remove_broken_bins(&self) -> miette::Result<()> {
-        for exposed_bin in self.bin_dir.bins().await? {
-            let executable = exposed_bin.executable().await;
-
-            if executable
-                .and_then(|path| {
-                    if path.is_file() {
-                        Ok(path)
-                    } else {
-                        miette::bail!("Path is not a file")
+    pub(crate) async fn remove_broken_files(&self) -> miette::Result<()> {
+        // Get all the files in the global binary directory
+        // If there's a trampoline that couldn't be correctly parsed, remove it
+        let root_path = self.bin_dir.path();
+        let mut entries = tokio_fs::read_dir(&root_path).await.into_diagnostic()?;
+
+        while let Some(entry) = entries.next_entry().await.into_diagnostic()? {
+            let path = entry.path();
+            if path.is_file() && path.is_executable() && Trampoline::is_trampoline(&path).await? {
+                let exposed_name = Trampoline::name(&path)?;
+                match Configuration::from_root_path(root_path, &exposed_name).await {
+                    Ok(_) => (),
+                    Err(ConfigurationParseError::ReadError(config_path, err)) => {
+                        tracing::warn!("Couldn't read {}: {err}", config_path.display());
+                        tracing::warn!("Removing the trampoline at {}", path.display());
+                        tokio_fs::remove_file(path).await.into_diagnostic()?;
+                    }
+                    Err(ConfigurationParseError::ParseError(config_path, err)) => {
+                        tracing::warn!("Couldn't parse {}: {err}", config_path.display());
+                        tracing::warn!(
+                            "Removing the trampoline at {} and configuration at {}",
+                            path.display(),
+                            config_path.display()
+                        );
+                        tokio_fs::remove_file(path).await.into_diagnostic()?;
+                        tokio_fs::remove_file(config_path).await.into_diagnostic()?;
                     }
-                })
-                .is_err()
-            {
-                tokio_fs::remove_file(exposed_bin.path())
-                    .await
-                    .into_diagnostic()?;
-
-                if exposed_bin.is_trampoline() {
-                    tokio_fs::remove_file(
-                        exposed_bin
-                            .trampoline()
-                            .expect("we checked it")
-                            .manifest_path(),
-                    )
-                    .await
-                    .into_diagnostic()?;
                 }
             }
         }
-
         Ok(())
     }