From 0c13c83f39114ed7029789bf912ab275f62517a7 Mon Sep 17 00:00:00 2001
From: Miles Johnson <mileswjohnson@gmail.com>
Date: Mon, 18 Sep 2023 15:38:49 -0700
Subject: [PATCH 1/4] Improve install/uninstall handling.

---
 crates/core/src/error.rs  |  8 ++++++++
 crates/core/src/tool.rs   | 42 ++++++++++++++++++++++++---------------
 crates/pdk-api/src/api.rs | 14 ++++++++++---
 3 files changed, 45 insertions(+), 19 deletions(-)

diff --git a/crates/core/src/error.rs b/crates/core/src/error.rs
index 4aedce35c..812d32e3e 100644
--- a/crates/core/src/error.rs
+++ b/crates/core/src/error.rs
@@ -13,6 +13,10 @@ pub enum ProtoError {
     #[error("Tool inventory directory has been overridden but is not an absolute path. Only absolute paths are supported.")]
     AbsoluteInventoryDir,
 
+    #[diagnostic(code(proto::tool::install_failed))]
+    #[error("Failed to install {tool}. {error}")]
+    InstallFailed { tool: String, error: String },
+
     #[diagnostic(code(proto::misc::offline))]
     #[error("Internet connection required, unable to download and install tools.")]
     InternetConnectionRequired,
@@ -46,6 +50,10 @@ pub enum ProtoError {
         command: String,
     },
 
+    #[diagnostic(code(proto::tool::uninstall_failed))]
+    #[error("Failed to uninstall {tool}. {error}")]
+    UninstallFailed { tool: String, error: String },
+
     #[diagnostic(code(proto::tool::unknown))]
     #[error(
         "{} is not a built-in tool or has not been configured as a plugin, unable to proceed.", .id.style(Style::Id)
diff --git a/crates/core/src/tool.rs b/crates/core/src/tool.rs
index f40707cb3..6f2c9ea5c 100644
--- a/crates/core/src/tool.rs
+++ b/crates/core/src/tool.rs
@@ -794,13 +794,18 @@ impl Tool {
                 },
             )?;
 
-            if !result.skip_install {
-                return Ok(result.installed);
+            if !result.installed && !result.skip_install {
+                return Err(ProtoError::InstallFailed {
+                    tool: self.get_name().to_owned(),
+                    error: result.error.unwrap_or_default(),
+                }
+                .into());
             }
-        }
 
         // Install from a prebuilt archive
-        self.install_from_prebuilt(&install_dir).await?;
+        } else {
+            self.install_from_prebuilt(&install_dir).await?;
+        }
 
         self.on_installed
             .emit(InstalledEvent {
@@ -835,11 +840,11 @@ impl Tool {
         )?;
 
         if !result.installed {
-            return Err(ProtoError::Message(
-                result
-                    .error
-                    .unwrap_or_else(|| "Unknown install failure!".to_string()),
-            ))?;
+            return Err(ProtoError::InstallFailed {
+                tool: dependency.to_owned(),
+                error: result.error.unwrap_or_default(),
+            }
+            .into());
         }
 
         self.on_installed_global
@@ -880,8 +885,12 @@ impl Tool {
                 },
             )?;
 
-            if !result.skip_uninstall && !result.uninstalled {
-                return Ok(false);
+            if !result.uninstalled && !result.skip_uninstall {
+                return Err(ProtoError::UninstallFailed {
+                    tool: self.get_name().to_owned(),
+                    error: result.error.unwrap_or_default(),
+                }
+                .into());
             }
         }
 
@@ -922,11 +931,11 @@ impl Tool {
         )?;
 
         if !result.uninstalled {
-            return Err(ProtoError::Message(
-                result
-                    .error
-                    .unwrap_or_else(|| "Unknown uninstall failure!".to_string()),
-            ))?;
+            return Err(ProtoError::UninstallFailed {
+                tool: dependency.to_owned(),
+                error: result.error.unwrap_or_default(),
+            }
+            .into());
         }
 
         self.on_uninstalled_global
@@ -1148,6 +1157,7 @@ impl Tool {
 
         self.version
             .as_ref()
+            // Canary can be overwritten so treat as not-installed
             .is_some_and(|v| !v.is_latest() && !v.is_canary())
             && dir.exists()
             && !fs::is_dir_locked(dir)
diff --git a/crates/pdk-api/src/api.rs b/crates/pdk-api/src/api.rs
index f75c207d7..1a05fe91b 100644
--- a/crates/pdk-api/src/api.rs
+++ b/crates/pdk-api/src/api.rs
@@ -149,6 +149,10 @@ json_struct!(
 json_struct!(
     /// Output returned by the `native_install` function.
     pub struct NativeInstallOutput {
+        /// Error message if the install failed.
+        #[serde(skip_serializing_if = "Option::is_none")]
+        pub error: Option<String>,
+
         /// Whether the install was successful.
         pub installed: bool,
 
@@ -168,6 +172,10 @@ json_struct!(
 json_struct!(
     /// Output returned by the `native_uninstall` function.
     pub struct NativeUninstallOutput {
+        /// Error message if the uninstall failed.
+        #[serde(skip_serializing_if = "Option::is_none")]
+        pub error: Option<String>,
+
         /// Whether the install was successful.
         pub uninstalled: bool,
 
@@ -299,12 +307,12 @@ json_struct!(
 json_struct!(
     /// Output returned by the `install_global` function.
     pub struct InstallGlobalOutput {
-        /// Whether the install was successful.
-        pub installed: bool,
-
         /// Error message if the install failed.
         #[serde(skip_serializing_if = "Option::is_none")]
         pub error: Option<String>,
+
+        /// Whether the install was successful.
+        pub installed: bool,
     }
 );
 

From 5c19c1598695fa865a503c82c7bd581bc0c8b48b Mon Sep 17 00:00:00 2001
From: Miles Johnson <mileswjohnson@gmail.com>
Date: Mon, 18 Sep 2023 15:47:59 -0700
Subject: [PATCH 2/4] Update commands.

---
 CHANGELOG.md                         | 10 ++++++++++
 Cargo.lock                           | 12 ++++++------
 Cargo.toml                           |  2 +-
 crates/cli/src/commands/install.rs   |  7 ++++++-
 crates/cli/src/commands/uninstall.rs | 14 ++++++++------
 5 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index aeabeb1a4..1e5bc2a88 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -10,6 +10,16 @@
 - [Rust](https://github.com/moonrepo/rust-plugin/blob/master/CHANGELOG.md)
 - [Schema](https://github.com/moonrepo/schema-plugin/blob/master/CHANGELOG.md)
 
+## Unreleased
+
+#### 🐞 Fixes
+
+- Fixed an issue where failed installs/uninstalls would exit with a zero exit code.
+
+#### ⚙️ Internal
+
+- Fixed an issue where install/uninstall events weren't always firing.
+
 ## 0.18.1
 
 #### 🐞 Fixes
diff --git a/Cargo.lock b/Cargo.lock
index 7ce4a3503..8babbebe6 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -474,9 +474,9 @@ dependencies = [
 
 [[package]]
 name = "clap"
-version = "4.4.3"
+version = "4.4.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "84ed82781cea27b43c9b106a979fe450a13a31aab0500595fb3fc06616de08e6"
+checksum = "b1d7b8d5ec32af0fadc644bf1fd509a688c2103b185644bb1e29d164e0703136"
 dependencies = [
  "clap_builder",
  "clap_derive",
@@ -484,9 +484,9 @@ dependencies = [
 
 [[package]]
 name = "clap_builder"
-version = "4.4.2"
+version = "4.4.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2bb9faaa7c2ef94b2743a21f5a29e6f0010dff4caa69ac8e9d6cf8b6fa74da08"
+checksum = "5179bb514e4d7c2051749d8fcefa2ed6d06a9f4e6d69faf3805f5d80b8cf8d56"
 dependencies = [
  "anstream",
  "anstyle",
@@ -500,7 +500,7 @@ version = "4.4.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "4110a1e6af615a9e6d0a36f805d5c99099f8bab9b8042f5bc1fa220a4a89e36f"
 dependencies = [
- "clap 4.4.3",
+ "clap 4.4.4",
 ]
 
 [[package]]
@@ -2331,7 +2331,7 @@ name = "proto_cli"
 version = "0.18.1"
 dependencies = [
  "chrono",
- "clap 4.4.3",
+ "clap 4.4.4",
  "clap_complete",
  "convert_case",
  "dialoguer",
diff --git a/Cargo.toml b/Cargo.toml
index cc39d7bd2..ad8117465 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -5,7 +5,7 @@ default-members = ["crates/cli"]
 
 [workspace.dependencies]
 cached = "0.45.1"
-clap = "4.4.3"
+clap = "4.4.4"
 clap_complete = "4.4.1"
 convert_case = "0.6.0"
 extism = "0.5.0"
diff --git a/crates/cli/src/commands/install.rs b/crates/cli/src/commands/install.rs
index a018dada7..941c5d601 100644
--- a/crates/cli/src/commands/install.rs
+++ b/crates/cli/src/commands/install.rs
@@ -101,7 +101,8 @@ pub async fn internal_install(args: InstallArgs) -> SystemResult {
         tool.get_resolved_version()
     ));
 
-    tool.setup(&version).await?;
+    let installed = tool.setup(&version).await?;
+
     tool.cleanup().await?;
 
     if args.pin {
@@ -110,6 +111,10 @@ pub async fn internal_install(args: InstallArgs) -> SystemResult {
 
     pb.finish_and_clear();
 
+    if !installed {
+        return Ok(());
+    }
+
     info!(
         "{} has been installed to {}!",
         tool.get_name(),
diff --git a/crates/cli/src/commands/uninstall.rs b/crates/cli/src/commands/uninstall.rs
index 8a95ee4ec..624eb47df 100644
--- a/crates/cli/src/commands/uninstall.rs
+++ b/crates/cli/src/commands/uninstall.rs
@@ -43,13 +43,15 @@ pub async fn uninstall(args: ArgsRef<UninstallArgs>) {
         tool.get_resolved_version()
     ));
 
-    tool.teardown().await?;
+    let uninstalled = tool.teardown().await?;
 
     pb.finish_and_clear();
 
-    info!(
-        "{} {} has been uninstalled!",
-        tool.get_name(),
-        tool.get_resolved_version(),
-    );
+    if uninstalled {
+        info!(
+            "{} {} has been uninstalled!",
+            tool.get_name(),
+            tool.get_resolved_version(),
+        );
+    }
 }

From 527b9ac09c75f8c291360e68bb1ee94a24d02cf9 Mon Sep 17 00:00:00 2001
From: Miles Johnson <mileswjohnson@gmail.com>
Date: Mon, 18 Sep 2023 15:57:21 -0700
Subject: [PATCH 3/4] Polish.

---
 crates/cli/src/commands/install.rs   |  10 +--
 crates/cli/src/commands/uninstall.rs |  14 ++--
 crates/core/src/tool.rs              |   1 +
 plugins/Cargo.lock                   | 120 +++++++++++++++++----------
 4 files changed, 87 insertions(+), 58 deletions(-)

diff --git a/crates/cli/src/commands/install.rs b/crates/cli/src/commands/install.rs
index 941c5d601..c78f475d4 100644
--- a/crates/cli/src/commands/install.rs
+++ b/crates/cli/src/commands/install.rs
@@ -103,18 +103,16 @@ pub async fn internal_install(args: InstallArgs) -> SystemResult {
 
     let installed = tool.setup(&version).await?;
 
-    tool.cleanup().await?;
-
-    if args.pin {
-        pin_global(&mut tool)?;
-    }
-
     pb.finish_and_clear();
 
     if !installed {
         return Ok(());
     }
 
+    if args.pin {
+        pin_global(&mut tool)?;
+    }
+
     info!(
         "{} has been installed to {}!",
         tool.get_name(),
diff --git a/crates/cli/src/commands/uninstall.rs b/crates/cli/src/commands/uninstall.rs
index 624eb47df..cff093cab 100644
--- a/crates/cli/src/commands/uninstall.rs
+++ b/crates/cli/src/commands/uninstall.rs
@@ -47,11 +47,13 @@ pub async fn uninstall(args: ArgsRef<UninstallArgs>) {
 
     pb.finish_and_clear();
 
-    if uninstalled {
-        info!(
-            "{} {} has been uninstalled!",
-            tool.get_name(),
-            tool.get_resolved_version(),
-        );
+    if !uninstalled {
+        return Ok(());
     }
+
+    info!(
+        "{} {} has been uninstalled!",
+        tool.get_name(),
+        tool.get_resolved_version(),
+    );
 }
diff --git a/crates/core/src/tool.rs b/crates/core/src/tool.rs
index 6f2c9ea5c..d78d71a55 100644
--- a/crates/core/src/tool.rs
+++ b/crates/core/src/tool.rs
@@ -1204,6 +1204,7 @@ impl Tool {
         self.resolve_version(initial_version).await?;
 
         if self.install().await? {
+            self.cleanup().await?;
             self.locate_bins().await?;
             self.setup_shims(true).await?;
 
diff --git a/plugins/Cargo.lock b/plugins/Cargo.lock
index f44149769..5a6d42696 100644
--- a/plugins/Cargo.lock
+++ b/plugins/Cargo.lock
@@ -258,28 +258,24 @@ checksum = "a2bd12c1caf447e69cd4528f47f94d203fd2582878ecb9e9465484c4148a8223"
 
 [[package]]
 name = "cached"
-version = "0.44.0"
+version = "0.45.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b195e4fbc4b6862bbd065b991a34750399c119797efff72492f28a5864de8700"
+checksum = "90eb5776f28a149524d1d8623035760b4454ec881e8cf3838fa8d7e1b11254b3"
 dependencies = [
- "async-trait",
  "cached_proc_macro",
  "cached_proc_macro_types",
- "futures",
  "hashbrown 0.13.2",
  "instant",
  "once_cell",
  "thiserror",
- "tokio",
 ]
 
 [[package]]
 name = "cached_proc_macro"
-version = "0.17.0"
+version = "0.18.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b48814962d2fd604c50d2b9433c2a41a0ab567779ee2c02f7fba6eca1221f082"
+checksum = "7da8245dd5f576a41c3b76247b54c15b0e43139ceeb4f732033e15be7c005176"
 dependencies = [
- "cached_proc_macro_types",
  "darling 0.14.4",
  "proc-macro2",
  "quote",
@@ -466,6 +462,16 @@ dependencies = [
  "unicode-xid",
 ]
 
+[[package]]
+name = "core-foundation"
+version = "0.9.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "194a7a9e6de53fa55116934067c844d9d749312f75c6f6d0980e8c252f8c2146"
+dependencies = [
+ "core-foundation-sys",
+ "libc",
+]
+
 [[package]]
 name = "core-foundation-sys"
 version = "0.8.4"
@@ -1081,20 +1087,6 @@ dependencies = [
  "windows-sys 0.48.0",
 ]
 
-[[package]]
-name = "futures"
-version = "0.3.28"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "23342abe12aba583913b2e62f22225ff9c950774065e4bfb61a19cd9770fec40"
-dependencies = [
- "futures-channel",
- "futures-core",
- "futures-io",
- "futures-sink",
- "futures-task",
- "futures-util",
-]
-
 [[package]]
 name = "futures-channel"
 version = "0.3.28"
@@ -1102,7 +1094,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "955518d47e09b25bbebc7a18df10b81f0c766eaf4c4f1cccef2fca5f2a4fb5f2"
 dependencies = [
  "futures-core",
- "futures-sink",
 ]
 
 [[package]]
@@ -1111,12 +1102,6 @@ version = "0.3.28"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "4bca583b7e26f571124fe5b7561d49cb2868d79116cfa0eefce955557c6fee8c"
 
-[[package]]
-name = "futures-io"
-version = "0.3.28"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "4fff74096e71ed47f8e023204cfd0aa1289cd54ae5430a9523be060cdb849964"
-
 [[package]]
 name = "futures-macro"
 version = "0.3.28"
@@ -1148,7 +1133,6 @@ checksum = "26b01e40b772d54cf6c6d721c1d1abd0647a0106a12ecaa1c186273392a69533"
 dependencies = [
  "futures-core",
  "futures-macro",
- "futures-sink",
  "futures-task",
  "pin-project-lite",
  "pin-utils",
@@ -1907,6 +1891,12 @@ dependencies = [
  "stable_deref_trait",
 ]
 
+[[package]]
+name = "openssl-probe"
+version = "0.1.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ff011a302c396a5197692431fc1948019154afc178baf7d8e37367442a4601cf"
+
 [[package]]
 name = "option-ext"
 version = "0.2.0"
@@ -2054,7 +2044,7 @@ dependencies = [
 
 [[package]]
 name = "proto_core"
-version = "0.16.4"
+version = "0.18.1"
 dependencies = [
  "cached",
  "extism",
@@ -2082,7 +2072,7 @@ dependencies = [
 
 [[package]]
 name = "proto_pdk"
-version = "0.6.5"
+version = "0.7.2"
 dependencies = [
  "anyhow",
  "extism-pdk",
@@ -2094,7 +2084,7 @@ dependencies = [
 
 [[package]]
 name = "proto_pdk_api"
-version = "0.6.3"
+version = "0.7.1"
 dependencies = [
  "anyhow",
  "semver",
@@ -2106,7 +2096,7 @@ dependencies = [
 
 [[package]]
 name = "proto_pdk_test_utils"
-version = "0.5.9"
+version = "0.7.0"
 dependencies = [
  "extism",
  "proto_core",
@@ -2117,7 +2107,7 @@ dependencies = [
 
 [[package]]
 name = "proto_wasm_plugin"
-version = "0.6.3"
+version = "0.6.5"
 dependencies = [
  "extism",
  "proto_pdk_api",
@@ -2325,6 +2315,7 @@ dependencies = [
  "percent-encoding",
  "pin-project-lite",
  "rustls",
+ "rustls-native-certs",
  "rustls-pemfile",
  "serde",
  "serde_json",
@@ -2336,7 +2327,6 @@ dependencies = [
  "wasm-bindgen",
  "wasm-bindgen-futures",
  "web-sys",
- "webpki-roots 0.25.2",
  "winreg",
 ]
 
@@ -2430,6 +2420,18 @@ dependencies = [
  "sct",
 ]
 
+[[package]]
+name = "rustls-native-certs"
+version = "0.6.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a9aace74cb666635c918e9c12bc0d348266037aa8eb599b5cba565709a8dff00"
+dependencies = [
+ "openssl-probe",
+ "rustls-pemfile",
+ "schannel",
+ "security-framework",
+]
+
 [[package]]
 name = "rustls-pemfile"
 version = "1.0.3"
@@ -2474,6 +2476,15 @@ dependencies = [
  "winapi-util",
 ]
 
+[[package]]
+name = "schannel"
+version = "0.1.22"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0c3733bf4cf7ea0880754e19cb5a462007c4a8c1914bff372ccc95b464f1df88"
+dependencies = [
+ "windows-sys 0.48.0",
+]
+
 [[package]]
 name = "scopeguard"
 version = "1.1.0"
@@ -2490,6 +2501,29 @@ dependencies = [
  "untrusted",
 ]
 
+[[package]]
+name = "security-framework"
+version = "2.9.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "05b64fb303737d99b81884b2c63433e9ae28abebe5eb5045dcdd175dc2ecf4de"
+dependencies = [
+ "bitflags 1.3.2",
+ "core-foundation",
+ "core-foundation-sys",
+ "libc",
+ "security-framework-sys",
+]
+
+[[package]]
+name = "security-framework-sys"
+version = "2.9.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e932934257d3b408ed8f30db49d85ea163bfe74961f017f405b025af298f0c7a"
+dependencies = [
+ "core-foundation-sys",
+ "libc",
+]
+
 [[package]]
 name = "semver"
 version = "1.0.18"
@@ -2531,9 +2565,9 @@ dependencies = [
 
 [[package]]
 name = "serde_json"
-version = "1.0.105"
+version = "1.0.107"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "693151e1ac27563d6dbcec9dee9fbd5da8539b20fa14ad3752b2e6d363ace360"
+checksum = "6b420ce6e3d8bd882e9b243c6eed35dbc9a6110c9769e74b584e0d68d1f20c65"
 dependencies = [
  "itoa",
  "ryu",
@@ -3158,7 +3192,7 @@ dependencies = [
  "rustls",
  "rustls-webpki 0.100.2",
  "url",
- "webpki-roots 0.23.1",
+ "webpki-roots",
 ]
 
 [[package]]
@@ -3217,7 +3251,7 @@ dependencies = [
 
 [[package]]
 name = "warpgate"
-version = "0.5.3"
+version = "0.5.6"
 dependencies = [
  "extism",
  "miette",
@@ -3778,12 +3812,6 @@ dependencies = [
  "rustls-webpki 0.100.2",
 ]
 
-[[package]]
-name = "webpki-roots"
-version = "0.25.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "14247bb57be4f377dfb94c72830b8ce8fc6beac03cf4bf7b9732eadd414123fc"
-
 [[package]]
 name = "wiggle"
 version = "11.0.1"

From 5fa95fb1319d84ab9544c0aa0a63f80dcad9aad0 Mon Sep 17 00:00:00 2001
From: Miles Johnson <mileswjohnson@gmail.com>
Date: Mon, 18 Sep 2023 16:22:20 -0700
Subject: [PATCH 4/4] Fix python.

---
 crates/core/src/tool.rs | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/crates/core/src/tool.rs b/crates/core/src/tool.rs
index d78d71a55..9f64011ea 100644
--- a/crates/core/src/tool.rs
+++ b/crates/core/src/tool.rs
@@ -775,6 +775,7 @@ impl Tool {
 
         let install_dir = self.get_tool_dir();
         let _install_lock = fs::lock_directory(&install_dir).await?;
+        let mut installed = false;
 
         self.on_installing
             .emit(InstallingEvent {
@@ -800,10 +801,15 @@ impl Tool {
                     error: result.error.unwrap_or_default(),
                 }
                 .into());
+
+            // If native install fails, attempt other installers
+            } else {
+                installed = result.installed;
             }
+        }
 
         // Install from a prebuilt archive
-        } else {
+        if !installed {
             self.install_from_prebuilt(&install_dir).await?;
         }