fix: Audit 11/11 (#653)

moonrepo · Nov 12, 2024 · b1cb091 · b1cb091
1 parent 2d1dc78
commit b1cb091
Show file tree

Hide file tree

Showing 8 changed files with 99 additions and 67 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,17 @@
 - [Rust](https://github.com/moonrepo/tools/blob/master/tools/rust/CHANGELOG.md)
 - [Schema (TOML, JSON, YAML)](https://github.com/moonrepo/tools/blob/master/tools/internal-schema/CHANGELOG.md)
 
+## Unreleased
+
+#### 🚀 Updates
+
+- When installing many tools with `proto install|use`, a failed install for a single tool will no longer abort the install of the other tools.
+- Added more logging to debug the "File exists (os error 17)" issue.
+
+#### 🐞 Fixes
+
+- Fixed the wrong `proto_version` being passed to WASM function calls.
+
 ## 0.42.0
 
 #### 💥 Breaking

diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -29,8 +29,8 @@ serde = { version = "1.0.214", features = ["derive"] }
 serde_json = "1.0.132"
 sha2 = "0.10.8"
 shell-words = "1.1.0"
-starbase = { version = "0.9.2" }
-starbase_archive = { version = "0.8.7", features = [
+starbase = { version = "0.9.4" }
+starbase_archive = { version = "0.8.8", features = [
     "gz",
     "miette",
     "tar-bz2",
@@ -41,10 +41,10 @@ starbase_archive = { version = "0.8.7", features = [
     "zip-deflate",
 ] }
 starbase_events = { version = "0.6.3" }
-starbase_sandbox = { version = "0.7.4" }
-starbase_shell = { version = "0.5.8", features = ["miette"] }
-starbase_styles = { version = "0.4.4" }
-starbase_utils = { version = "0.8.10", default-features = false, features = [
+starbase_sandbox = { version = "0.7.5" }
+starbase_shell = { version = "0.5.9", features = ["miette"] }
+starbase_styles = { version = "0.4.5" }
+starbase_utils = { version = "0.8.11", default-features = false, features = [
     "json",
     "miette",
     "net",

diff --git a/crates/cli/src/commands/install.rs b/crates/cli/src/commands/install.rs
@@ -5,7 +5,6 @@ use crate::shell::{self, Export};
 use crate::telemetry::{track_usage, Metric};
 use clap::Args;
 use indicatif::ProgressBar;
-use miette::IntoDiagnostic;
 use proto_core::flow::install::{InstallOptions, InstallPhase};
 use proto_core::{Id, PinType, Tool, UnresolvedVersionSpec, VersionSpec, PROTO_PLUGIN_KEY};
 use proto_pdk_api::{InstallHook, SyncShellProfileInput, SyncShellProfileOutput};
@@ -16,7 +15,7 @@ use std::env;
 use std::time::Duration;
 use tokio::task::JoinSet;
 use tokio::time::sleep;
-use tracing::{debug, instrument};
+use tracing::{debug, instrument, trace};
 
 #[derive(Args, Clone, Debug, Default)]
 pub struct InstallArgs {
@@ -198,7 +197,7 @@ async fn update_shell(tool: &Tool, passthrough_args: Vec<String>) -> miette::Res
 pub async fn do_install(
     tool: &mut Tool,
     args: InstallArgs,
-    pb: ProgressBar,
+    pb: &ProgressBar,
 ) -> miette::Result<bool> {
     let version = args.get_unresolved_spec();
     let pin_type = args.get_pin_type();
@@ -375,7 +374,7 @@ async fn install_one(session: &ProtoSession, id: &Id, args: InstallArgs) -> miet
     let mut tool = session.load_tool(id).await?;
     let pb = create_progress_bar(format!("Installing {}", tool.get_name()));
 
-    if do_install(&mut tool, args, pb).await? {
+    if do_install(&mut tool, args, &pb).await? {
         println!(
             "{} {} has been installed to {}!",
             tool.get_name(),
@@ -435,12 +434,14 @@ pub async fn install_all(session: &ProtoSession) -> AppResult {
 
     for mut tool in tools {
         if let Some(version) = versions.remove(&tool.id) {
+            let tool_id = tool.id.clone();
+
             let pb = mpb.add(ProgressBar::new(0));
             pb.set_style(pbs.clone());
 
             // Defer writing content till the thread starts,
             // otherwise the progress bars fail to render correctly
-            set.spawn(async move {
+            let handle = set.spawn(async move {
                 sleep(Duration::from_millis(25)).await;
 
                 pb.set_prefix(color::id(format!(
@@ -450,39 +451,60 @@ pub async fn install_all(session: &ProtoSession) -> AppResult {
                 )));
 
                 pb.set_message(format!("Installing {} {}", tool.get_name(), version));
-
                 print_progress_state(&pb);
 
-                do_install(
+                if let Err(error) = do_install(
                     &mut tool,
                     InstallArgs {
                         spec: Some(version),
                         ..Default::default()
                     },
-                    pb,
+                    &pb,
                 )
                 .await
+                {
+                    pb.set_message(format!("Failed to install: {}", error));
+                    print_progress_state(&pb);
+                }
             });
+
+            trace!(
+                task_id = handle.id().to_string(),
+                "Spawning {} in background task",
+                color::id(tool_id)
+            );
         }
     }
 
-    let total = set.len();
-
-    while let Some(result) = set.join_next().await {
-        match result.into_diagnostic() {
-            Err(error) | Ok(Err(error)) => {
-                mpb.clear().into_diagnostic()?;
-                drop(mpb);
-
-                return Err(error);
+    let mut installed_count = 0;
+    let mut failed_count = 0;
+
+    while let Some(result) = set.join_next_with_id().await {
+        match result {
+            Err(error) => {
+                trace!(
+                    task_id = error.id().to_string(),
+                    "Spawned task failed: {}",
+                    error
+                );
+                failed_count += 1;
+            }
+            Ok((task_id, _)) => {
+                trace!(task_id = task_id.to_string(), "Spawned task successful");
+                installed_count += 1;
             }
-            _ => {}
         };
     }
 
     // When no TTY, we should display something to the user!
     if mpb.is_hidden() {
-        println!("Successfully installed {} tools!", total);
+        if installed_count > 0 {
+            println!("Successfully installed {} tools!", installed_count);
+        }
+
+        if failed_count > 0 {
+            println!("Failed to install {} tools!", failed_count);
+        }
     }
 
     Ok(None)

diff --git a/crates/cli/src/commands/run.rs b/crates/cli/src/commands/run.rs
@@ -191,12 +191,9 @@ pub async fn run(session: ProtoSession, args: RunArgs) -> AppResult {
             ..Default::default()
         };
 
-        do_install(
-            &mut tool,
-            install_args,
-            create_progress_bar(format!("Installing {resolved_version}")),
-        )
-        .await?;
+        let pb = create_progress_bar(format!("Installing {resolved_version}"));
+
+        do_install(&mut tool, install_args, &pb).await?;
 
         println!(
             "{} {} has been installed, continuing execution...",

diff --git a/crates/cli/src/commands/upgrade.rs b/crates/cli/src/commands/upgrade.rs
@@ -118,6 +118,12 @@ pub async fn upgrade(session: ProtoSession, args: UpgradeArgs) -> AppResult {
     // Load the tool and install the new version
     let mut tool = session.load_proto_tool().await?;
 
+    let pb = create_progress_bar(if target_version > current_version {
+        format!("Upgrading to {}", target_version)
+    } else {
+        format!("Downgrading to {}", target_version)
+    });
+
     do_install(
         &mut tool,
         InstallArgs {
@@ -127,11 +133,7 @@ pub async fn upgrade(session: ProtoSession, args: UpgradeArgs) -> AppResult {
             ))),
             ..Default::default()
         },
-        create_progress_bar(if target_version > current_version {
-            format!("Upgrading to {}", target_version)
-        } else {
-            format!("Downgrading to {}", target_version)
-        }),
+        &pb,
     )
     .await?;
 

diff --git a/crates/core/src/helpers.rs b/crates/core/src/helpers.rs
@@ -22,7 +22,15 @@ pub static ENV_VAR_SUB: LazyLock<Regex> =
 pub fn get_proto_version() -> &'static Version {
     static VERSION_CACHE: OnceLock<Version> = OnceLock::new();
 
-    VERSION_CACHE.get_or_init(|| Version::parse(env!("CARGO_PKG_VERSION")).unwrap())
+    VERSION_CACHE.get_or_init(|| {
+        Version::parse(
+            env::var("PROTO_VERSION")
+                .ok()
+                .as_deref()
+                .unwrap_or(env!("CARGO_PKG_VERSION")),
+        )
+        .unwrap()
+    })
 }
 
 pub fn is_offline() -> bool {

diff --git a/crates/core/src/tool.rs b/crates/core/src/tool.rs
@@ -211,17 +211,9 @@ impl Tool {
 
         #[cfg(not(debug_assertions))]
         if let Some(expected_version) = &metadata.minimum_proto_version {
-            use miette::IntoDiagnostic;
+            let actual_version = get_proto_version();
 
-            let actual_version = Version::parse(
-                std::env::var("PROTO_VERSION")
-                    .ok()
-                    .as_deref()
-                    .unwrap_or_else(|| env!("CARGO_PKG_VERSION")),
-            )
-            .into_diagnostic()?;
-
-            if &actual_version < expected_version {
+            if actual_version < expected_version {
                 return Err(ProtoError::InvalidMinimumVersion {
                     tool: metadata.name,
                     id: self.id.clone(),