Clean up run.

CHANGELOG.md

@@ -15,15 +15,15 @@
 #### 🚀 Updates
 
 - Refactored and standardized how executables (bins and shims) are managed.
-  - Binaries (`~/.proto/bin`) and shims (`~/.proto/shims`) share the same internal data structures.
+  - Binaries (`~/.proto/bin`) and shims (`~/.proto/shims`) now share the same internal data structures.
   - For the most part, is a 1:1 relation. There will be a shim for every binary, and vice versa.
   - Reduced the amount of WASM calls to locate executables to 1 call.
-  - Removed the concept of local shims (was an implementation detail).
+  - Removed the concept of local shims (was a hidden implementation detail).
 - Reworked the `proto bin` command.
   - By default returns an absolute path to the real executable (`~/.proto/tools/<tool>/<version>/bin`).
   - Pass `--bin` to return the `~/.proto/bin` path.
   - Pass `--shim` to return the `~/.proto/shims` path.
-- Updated `proto clean --purge` to accurately delete all executables.
+- Updated `proto clean --purge` and `proto uninstall` to accurately delete all executables.
 - WASM API
   - Added `locate_executables` function.
   - Added `LocateExecutablesInput`, `LocateExecutablesOutput`, `ExecutableConfig` structs.

Cargo.toml

@@ -24,7 +24,7 @@ schematic = { version = "0.12.7", default-features = false, features = [
 ] }
 semver = "1.0.20"
 serde = { version = "1.0.190", features = ["derive"] }
-serde_json = "1.0.107"
+serde_json = "1.0.108"
 sha2 = "0.10.8"
 starbase = "0.2.9"
 starbase_archive = { version = "0.2.4", features = [
@@ -37,7 +37,7 @@ starbase_archive = { version = "0.2.4", features = [
 starbase_events = "0.2.2"
 starbase_sandbox = { version = "0.1.12" }
 starbase_styles = "0.1.16"
-starbase_utils = { version = "0.3.6", default-features = false, features = [
+starbase_utils = { version = "0.3.7", default-features = false, features = [
 	"json",
 	"toml",
 ] }

crates/cli/Cargo.toml

@@ -43,6 +43,7 @@ miette = { workspace = true }
 reqwest = { workspace = true, features = ["rustls-tls-native-roots", "stream"] }
 semver = { workspace = true }
 serde = { workspace = true }
+shell-words = "1.1.0"
 starbase = { workspace = true }
 starbase_archive = { workspace = true }
 starbase_styles = { workspace = true }

crates/cli/src/app.rs

@@ -191,7 +191,7 @@ pub enum Commands {
         alias = "r",
         name = "run",
         about = "Run a tool after detecting a version from the environment.",
-        long_about = "Run a tool after detecting a version from the environment. In order of priority,\na version will be resolved from a provided CLI argument, a PROTO_VERSION environment variable,\na local version file (.prototools), and lastly a global version file (~/.proto/tools/version).\n\nIf no version can be found, the program will exit with an error."
+        long_about = "Run a tool after detecting a version from the environment. In order of priority,\na version will be resolved from a provided CLI argument, a PROTO_VERSION environment variable,\na local version file (.prototools), and lastly a global version file (~/.proto/tools).\n\nIf no version can be found, the program will exit with an error."
     )]
     Run(RunArgs),
 

crates/cli/src/commands/run.rs

@@ -6,6 +6,7 @@ use proto_core::{detect_version, load_tool, Id, ProtoError, Tool, UnresolvedVers
 use proto_pdk_api::RunHook;
 use starbase::system;
 use std::env;
+use std::path::Path;
 use std::process::exit;
 use system_env::is_command_on_path;
 use tokio::process::Command;
@@ -19,7 +20,7 @@ pub struct RunArgs {
     #[arg(help = "Version or alias of tool")]
     spec: Option<UnresolvedVersionSpec>,
 
-    #[arg(long, help = "Path to an alternate binary to run")]
+    #[arg(long, help = "Path to a relative alternate binary to run")]
     bin: Option<String>,
 
     // Passthrough args (after --)
@@ -48,6 +49,30 @@ fn is_trying_to_self_upgrade(tool: &Tool, args: &[String]) -> bool {
     false
 }
 
+fn create_command(exe_path: &Path, args: &[String]) -> Command {
+    match exe_path.extension().map(|e| e.to_str().unwrap()) {
+        Some("ps1" | "cmd" | "bat") => {
+            let mut cmd = Command::new(if is_command_on_path("pwsh") {
+                "pwsh"
+            } else {
+                "powershell"
+            });
+            cmd.arg("-Command");
+            cmd.arg(format!(
+                "{} {}",
+                exe_path.display(),
+                shell_words::join(args)
+            ));
+            cmd
+        }
+        _ => {
+            let mut cmd = Command::new(exe_path);
+            cmd.args(args);
+            cmd
+        }
+    }
+}
+
 #[system]
 pub async fn run(args: ArgsRef<RunArgs>) -> SystemResult {
     let mut tool = load_tool(&args.id).await?;
@@ -91,29 +116,27 @@ pub async fn run(args: ArgsRef<RunArgs>) -> SystemResult {
         .await?;
     }
 
-    // TODO rework this handling
-
     // Determine the binary path to execute
-    let tool_dir = tool.get_tool_dir();
-    let mut bin_path = tool.get_exe_path()?.to_path_buf();
-
-    if let Some(alt_bin) = &args.bin {
-        let alt_bin_path = tool_dir.join(alt_bin);
+    let exe_path = match &args.bin {
+        Some(alt_bin) => {
+            let alt_path = tool.get_tool_dir().join(alt_bin);
 
-        debug!(bin = alt_bin, path = ?alt_bin_path, "Received an alternate binary to run with");
+            debug!(bin = alt_bin, path = ?alt_path, "Received an alternate binary to run with");
 
-        if alt_bin_path.exists() {
-            bin_path = alt_bin_path;
-        } else {
-            return Err(ProtoCliError::MissingRunAltBin {
-                bin: alt_bin.to_owned(),
-                path: alt_bin_path,
+            if alt_path.exists() {
+                alt_path
+            } else {
+                return Err(ProtoCliError::MissingRunAltBin {
+                    bin: alt_bin.to_owned(),
+                    path: alt_path,
+                }
+                .into());
             }
-            .into());
         }
-    }
+        None => tool.get_exe_path()?.to_path_buf(),
+    };
 
-    debug!(bin = ?bin_path, args = ?args.passthrough, "Running {}", tool.get_name());
+    debug!(bin = ?exe_path, args = ?args.passthrough, "Running {}", tool.get_name());
 
     // Run before hook
     tool.run_hook("pre_run", || RunHook {
@@ -122,60 +145,28 @@ pub async fn run(args: ArgsRef<RunArgs>) -> SystemResult {
     })?;
 
     // Run the command
-    let mut command = match bin_path.extension().map(|e| e.to_str().unwrap()) {
-        Some("ps1") => {
-            let mut cmd = Command::new(if is_command_on_path("pwsh") {
-                "pwsh"
-            } else {
-                "powershell"
-            });
-            cmd.arg("-Command").arg(format!(
-                "{} {}",
-                bin_path.display(),
-                args.passthrough.join(" ")
-            ));
-            cmd
-        }
-        Some("cmd" | "bat") => {
-            let mut cmd = Command::new("cmd");
-            cmd.arg("/q").arg("/c").arg(format!(
-                "{} {}",
-                bin_path.display(),
-                args.passthrough.join(" ")
-            ));
-            cmd
-        }
-        _ => {
-            let mut cmd = Command::new(bin_path);
-            cmd.args(&args.passthrough);
-            cmd
-        }
-    };
-
-    let status = command
+    let status = create_command(&exe_path, &args.passthrough)
         .env(
             format!("{}_VERSION", tool.get_env_var_prefix()),
             tool.get_resolved_version().to_string(),
         )
         .env(
             format!("{}_BIN", tool.get_env_var_prefix()),
-            tool.get_exe_path()?.to_string_lossy().to_string(),
+            exe_path.to_string_lossy().to_string(),
         )
         .spawn()
         .into_diagnostic()?
         .wait()
         .await
         .into_diagnostic()?;
 
-    if !status.success() {
-        exit(status.code().unwrap_or(1));
-    }
-
     // Run after hook
-    tool.run_hook("post_run", || RunHook {
-        context: tool.create_context(),
-        passthrough_args: args.passthrough.clone(),
-    })?;
+    if status.success() {
+        tool.run_hook("post_run", || RunHook {
+            context: tool.create_context(),
+            passthrough_args: args.passthrough.clone(),
+        })?;
+    }
 
     // Update the last used timestamp in a separate task,
     // as to not interrupt this task incase something fails!
@@ -185,4 +176,8 @@ pub async fn run(args: ArgsRef<RunArgs>) -> SystemResult {
             let _ = tool.manifest.save();
         });
     }
+
+    if !status.success() {
+        exit(status.code().unwrap_or(1));
+    }
 }

crates/core/src/tool.rs

@@ -1514,22 +1514,29 @@ impl Tool {
     pub async fn teardown(&mut self) -> miette::Result<bool> {
         self.cleanup().await?;
 
-        if self.uninstall().await? {
-            // Only remove if uninstall was successful
-            self.manifest.remove_version(self.get_resolved_version())?;
-
-            // If no more default version, delete the symlink,
-            // otherwise the OS will throw errors for missing target
-            if self.manifest.default_version.is_none() {
-                for bin in self.get_bin_locations()? {
-                    fs::remove_file(&self.proto.bin_dir.join(bin.path))?;
-                }
+        if !self.uninstall().await? {
+            return Ok(false);
+        }
+
+        // Only remove if uninstall was successful
+        self.manifest.remove_version(self.get_resolved_version())?;
+
+        // If no more default version, delete the symlink,
+        // otherwise the OS will throw errors for missing sources
+        if self.manifest.default_version.is_none() {
+            for bin in self.get_bin_locations()? {
+                fs::remove_file(bin.path)?;
             }
+        }
 
-            return Ok(true);
+        // If no more versions in general, delete all shims
+        if self.manifest.installed_versions.is_empty() {
+            for shim in self.get_shim_locations()? {
+                fs::remove_file(shim.path)?;
+            }
         }
 
-        Ok(false)
+        Ok(true)
     }
 
     /// Delete temporary files and downloads for the current version.

crates/pdk-test-utils/Cargo.toml

@@ -12,7 +12,7 @@ proto_core = { version = "0.21.2", path = "../core" }
 proto_pdk_api = { version = "0.9.0", path = "../pdk-api" }
 extism = { workspace = true }
 serde_json = { workspace = true }
-toml = { version = "0.8.2", optional = true }
+toml = { version = "0.8.6", optional = true }
 
 [features]
 default = []