Optimize BinstallError: Reduce size from 40B to 32B (#581)

* Optimize `BinstallError::CratesIoApi`: Extract new type `errors::CratesIoApiError` and box it
   Also improve `<CratesIoApiError as Display>::fmt` impl.
* Optimize `BinstallError::SubProcess`: Use `Box<str>` instead of `String`
* Optimize `BinstallError::CargoManifest`: Box `CargoTomlError`
* Optimize `BinstallError::VersionParse`: Extract `VersionParseError` and box it
   Also improve `<VersionParseError as Display>::fmt` impl.
* Optimize `BinstallError::CrateContext`: Extract `CrateContextError` and box it in
* Optimize `install_from_source`: Only format `cmd` on err

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>

crates/binstalk/src/drivers/crates_io.rs

@@ -6,7 +6,7 @@ use semver::VersionReq;
 use tracing::debug;
 
 use crate::{
-    errors::BinstallError,
+    errors::{BinstallError, CratesIoApiError},
     helpers::{
         download::Download,
         remote::{Client, Url},
@@ -33,14 +33,12 @@ pub async fn fetch_crate_cratesio(
     debug!("Looking up crate information");
 
     // Fetch online crate information
-    let base_info =
-        crates_io_api_client
-            .get_crate(name)
-            .await
-            .map_err(|err| BinstallError::CratesIoApi {
-                crate_name: name.into(),
-                err: Box::new(err),
-            })?;
+    let base_info = crates_io_api_client.get_crate(name).await.map_err(|err| {
+        Box::new(CratesIoApiError {
+            crate_name: name.into(),
+            err,
+        })
+    })?;
 
     // Locate matching version
     let version_iter = base_info.versions.iter().filter(|v| !v.yanked);

crates/binstalk/src/errors.rs

@@ -8,14 +8,38 @@ use binstalk_downloader::{
     download::{DownloadError, ZipError},
     remote::{Error as RemoteError, HttpError, ReqwestError},
 };
+use cargo_toml::Error as CargoTomlError;
 use compact_str::CompactString;
-use crates_io_api::Error as CratesIoApiError;
 use miette::{Diagnostic, Report};
 use thiserror::Error;
 use tinytemplate::error::Error as TinyTemplateError;
 use tokio::task;
 use tracing::{error, warn};
 
+#[derive(Debug, Error)]
+#[error("crates.io API error for {crate_name}: {err}")]
+pub struct CratesIoApiError {
+    pub crate_name: CompactString,
+    #[source]
+    pub err: crates_io_api::Error,
+}
+
+#[derive(Debug, Error)]
+#[error("version string '{v}' is not semver: {err}")]
+pub struct VersionParseError {
+    pub v: CompactString,
+    #[source]
+    pub err: semver::Error,
+}
+
+#[derive(Debug, Error)]
+#[error("For crate {crate_name}: {err}")]
+pub struct CrateContextError {
+    crate_name: CompactString,
+    #[source]
+    err: BinstallError,
+}
+
 /// Error kinds emitted by cargo-binstall.
 #[derive(Error, Diagnostic, Debug)]
 #[non_exhaustive]
@@ -93,7 +117,10 @@ pub enum BinstallError {
     /// - Exit: 70
     #[error("subprocess {command} errored with {status}")]
     #[diagnostic(severity(error), code(binstall::subprocess))]
-    SubProcess { command: String, status: ExitStatus },
+    SubProcess {
+        command: Box<str>,
+        status: ExitStatus,
+    },
 
     /// A generic I/O error.
     ///
@@ -109,17 +136,13 @@ pub enum BinstallError {
     ///
     /// - Code: `binstall::crates_io_api`
     /// - Exit: 76
-    #[error("crates.io API error")]
+    #[error(transparent)]
     #[diagnostic(
         severity(error),
         code(binstall::crates_io_api),
-        help("Check that the crate name you provided is correct.\nYou can also search for a matching crate at: https://lib.rs/search?q={crate_name}")
+        help("Check that the crate name you provided is correct.\nYou can also search for a matching crate at: https://lib.rs/search?q={}", .0.crate_name)
     )]
-    CratesIoApi {
-        crate_name: CompactString,
-        #[source]
-        err: Box<CratesIoApiError>,
-    },
+    CratesIoApi(#[from] Box<CratesIoApiError>),
 
     /// The override path to the cargo manifest is invalid or cannot be resolved.
     ///
@@ -143,7 +166,7 @@ pub enum BinstallError {
         code(binstall::cargo_manifest),
         help("If you used --manifest-path, check the Cargo.toml syntax.")
     )]
-    CargoManifest(#[from] cargo_toml::Error),
+    CargoManifest(Box<CargoTomlError>),
 
     /// A version is not valid semver.
     ///
@@ -152,13 +175,9 @@ pub enum BinstallError {
     ///
     /// - Code: `binstall::version::parse`
     /// - Exit: 80
-    #[error("version string '{v}' is not semver")]
+    #[error(transparent)]
     #[diagnostic(severity(error), code(binstall::version::parse))]
-    VersionParse {
-        v: CompactString,
-        #[source]
-        err: semver::Error,
-    },
+    VersionParse(#[from] Box<VersionParseError>),
 
     /// No available version matches the requirements.
     ///
@@ -302,12 +321,8 @@ pub enum BinstallError {
     NoFallbackToCargoInstall,
 
     /// A wrapped error providing the context of which crate the error is about.
-    #[error("For crate {crate_name}: {error}")]
-    CrateContext {
-        #[source]
-        error: Box<BinstallError>,
-        crate_name: CompactString,
-    },
+    #[error(transparent)]
+    CrateContext(Box<CrateContextError>),
 }
 
 impl BinstallError {
@@ -339,7 +354,7 @@ impl BinstallError {
             EmptySourceFilePath => 92,
             InvalidStrategies(..) => 93,
             NoFallbackToCargoInstall => 94,
-            CrateContext { error, .. } => error.exit_number(),
+            CrateContext(context) => context.err.exit_number(),
         };
 
         // reserved codes
@@ -361,10 +376,10 @@ impl BinstallError {
 
     /// Add crate context to the error
     pub fn crate_context(self, crate_name: impl Into<CompactString>) -> Self {
-        Self::CrateContext {
-            error: Box::new(self),
+        Self::CrateContext(Box::new(CrateContextError {
+            err: self,
             crate_name: crate_name.into(),
-        }
+        }))
     }
 }
 
@@ -438,3 +453,9 @@ impl From<TinyTemplateError> for BinstallError {
         BinstallError::Template(Box::new(e))
     }
 }
+
+impl From<CargoTomlError> for BinstallError {
+    fn from(e: CargoTomlError) -> Self {
+        BinstallError::CargoManifest(Box::new(e))
+    }
+}

crates/binstalk/src/ops/install.rs

@@ -131,8 +131,6 @@ async fn install_from_source(
         cmd.arg("--force");
     }
 
-    let command_string = format!("{cmd:?}");
-
     let mut child = jobserver_client.configure_and_run(&mut cmd, |cmd| cmd.spawn())?;
 
     debug!("Spawned command pid={:?}", child.id());
@@ -144,7 +142,7 @@ async fn install_from_source(
     } else {
         error!("Cargo errored! {status:?}");
         Err(BinstallError::SubProcess {
-            command: command_string,
+            command: format!("{cmd:?}").into_boxed_str(),
             status,
         })
     }

crates/binstalk/src/ops/resolve.rs

@@ -18,7 +18,7 @@ use super::Options;
 use crate::{
     bins,
     drivers::fetch_crate_cratesio,
-    errors::BinstallError,
+    errors::{BinstallError, VersionParseError},
     fetchers::{Data, Fetcher, TargetData},
     helpers::remote::Client,
     manifests::cargo_toml_binstall::{Meta, PkgMeta, PkgOverride},
@@ -409,10 +409,11 @@ impl PackageInfo {
         let new_version = match Version::parse(&new_version_str) {
             Ok(new_version) => new_version,
             Err(err) => {
-                return Err(BinstallError::VersionParse {
+                return Err(Box::new(VersionParseError {
                     v: new_version_str,
                     err,
                 })
+                .into())
             }
         };