Refactor: Extract new crate binstalk-bins (#1294)

To reduce `binstalk` codegen and enable reuse of it.

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

Cargo.lock

@@ -244,7 +244,7 @@ dependencies = [
 name = "binstalk"
 version = "0.15.0"
 dependencies = [
- "atomic-file-install",
+ "binstalk-bins",
  "binstalk-downloader",
  "binstalk-fetchers",
  "binstalk-registry",
@@ -254,13 +254,11 @@ dependencies = [
  "compact_str",
  "detect-targets",
  "either",
- "home",
  "itertools",
  "jobslot",
  "leon",
  "maybe-owned",
  "miette",
- "normalize-path",
  "semver",
  "strum",
  "target-lexicon",
@@ -271,6 +269,20 @@ dependencies = [
  "url",
 ]
 
+[[package]]
+name = "binstalk-bins"
+version = "0.0.0"
+dependencies = [
+ "atomic-file-install",
+ "binstalk-types",
+ "compact_str",
+ "leon",
+ "miette",
+ "normalize-path",
+ "thiserror",
+ "tracing",
+]
+
 [[package]]
 name = "binstalk-downloader"
 version = "0.7.0"
@@ -520,6 +532,7 @@ dependencies = [
  "file-format",
  "fs-lock",
  "gh-token",
+ "home",
  "log",
  "miette",
  "mimalloc",

Cargo.toml

@@ -3,6 +3,7 @@ members = [
     "crates/atomic-file-install",
     "crates/bin",
     "crates/binstalk",
+    "crates/binstalk-bins",
     "crates/binstalk-fetchers",
     "crates/binstalk-registry",
     "crates/binstalk-manifests",

crates/bin/Cargo.toml

@@ -30,6 +30,7 @@ dirs = "5.0.1"
 file-format = { version = "0.18.0", default-features = false }
 fs-lock = { version = "0.1.0", path = "../fs-lock" }
 gh-token = "0.1.2"
+home = "0.5.5"
 log = { version = "0.4.18", features = ["std"] }
 miette = "5.9.0"
 mimalloc = { version = "0.1.37", default-features = false, optional = true }

crates/bin/src/entry.rs

@@ -15,7 +15,6 @@ use binstalk::{
         remote::{Certificate, Client},
         tasks::AutoAbortJoinHandle,
     },
-    home::cargo_home,
     ops::{
         self,
         resolve::{CrateName, Resolution, ResolutionFetch, VersionReqExt},
@@ -25,6 +24,7 @@ use binstalk::{
 use binstalk_manifests::cargo_config::Config;
 use binstalk_manifests::cargo_toml_binstall::PkgOverride;
 use file_format::FileFormat;
+use home::cargo_home;
 use log::LevelFilter;
 use miette::{miette, Result, WrapErr};
 use tokio::task::block_in_place;

crates/bin/src/logging.rs

@@ -196,6 +196,7 @@ pub fn logging(log_level: LevelFilter, json_output: bool) {
     let allowed_targets = (log_level != LevelFilter::Trace).then_some([
         "atomic_file_install",
         "binstalk",
+        "binstalk_bins",
         "binstalk_downloader",
         "binstalk_fetchers",
         "binstalk_registry",

crates/binstalk-bins/Cargo.toml

@@ -0,0 +1,21 @@
+[package]
+name = "binstalk-bins"
+version = "0.0.0"
+edition = "2021"
+
+description = "The binstall binaries discovery and installation crate."
+repository = "https://github.com/cargo-bins/cargo-binstall"
+documentation = "https://docs.rs/binstalk-bins"
+rust-version = "1.65.0"
+authors = ["Jiahao XU <Jiahao_XU@outlook.com>"]
+license = "GPL-3.0-only"
+
+[dependencies]
+atomic-file-install = { version = "0.0.0", path = "../atomic-file-install" }
+binstalk-types = { version = "0.5.0", path = "../binstalk-types" }
+compact_str = { version = "0.7.0", features = ["serde"] }
+leon = { version = "2.0.1", path = "../leon" }
+miette = "5.9.0"
+normalize-path = { version = "0.2.1", path = "../normalize-path" }
+thiserror = "1.0.40"
+tracing = "0.1.37"

crates/binstalk-bins/src/lib.rs

@@ -1,23 +1,44 @@
 use std::{
     borrow::Cow,
-    fmt,
+    fmt, io,
     path::{self, Component, Path, PathBuf},
 };
 
+use atomic_file_install::{
+    atomic_install, atomic_install_noclobber, atomic_symlink_file, atomic_symlink_file_noclobber,
+};
+use binstalk_types::cargo_toml_binstall::{PkgFmt, PkgMeta};
 use compact_str::{format_compact, CompactString};
 use leon::Template;
+use miette::Diagnostic;
 use normalize_path::NormalizePath;
+use thiserror::Error as ThisError;
 use tracing::debug;
 
-use crate::{
+#[derive(Debug, ThisError, Diagnostic)]
-    errors::BinstallError,
+pub enum Error {
-    fs::{
+    /// bin-dir configuration provided generates source path outside
-        atomic_install, atomic_install_noclobber, atomic_symlink_file,
+    /// of the temporary dir.
-        atomic_symlink_file_noclobber,
+    #[error(
-    },
+        "bin-dir configuration provided generates source path outside of the temporary dir: {}", .0.display()
-    helpers::download::ExtractedFiles,
+    )]
-    manifests::cargo_toml_binstall::{PkgFmt, PkgMeta},
+    InvalidSourceFilePath(Box<Path>),
-};
+
+    /// bin-dir configuration provided generates empty source path.
+    #[error("bin-dir configuration provided generates empty source path")]
+    EmptySourceFilePath,
+
+    /// Bin file is not found.
+    #[error("bin file {} not found", .0.display())]
+    BinFileNotFound(Box<Path>),
+
+    #[error(transparent)]
+    Io(#[from] io::Error),
+
+    #[error("Failed to render template: {0}")]
+    #[diagnostic(transparent)]
+    TemplateRender(#[from] leon::RenderError),
+}
 
 /// Return true if the path does not look outside of current dir
 ///
@@ -33,7 +54,10 @@ fn is_valid_path(path: &Path) -> bool {
 
 /// Must be called after the archive is downloaded and extracted.
 /// This function might uses blocking I/O.
-pub fn infer_bin_dir_template(data: &Data, extracted_files: &ExtractedFiles) -> Cow<'static, str> {
+pub fn infer_bin_dir_template(
+    data: &Data,
+    has_dir: &mut dyn FnMut(&Path) -> bool,
+) -> Cow<'static, str> {
     let name = data.name;
     let target = data.target;
     let version = data.version;
@@ -58,7 +82,7 @@ pub fn infer_bin_dir_template(data: &Data, extracted_files: &ExtractedFiles) ->
     gen_possible_dirs
         .into_iter()
         .map(|gen_possible_dir| gen_possible_dir(name, target, version))
-        .find(|dirname| extracted_files.get_dir(Path::new(&dirname)).is_some())
+        .find(|dirname| has_dir(Path::new(&dirname)))
         .map(|mut dir| {
             dir.reserve_exact(1 + default_bin_dir_template.len());
             dir += "/";
@@ -84,7 +108,7 @@ impl BinFile {
         base_name: &str,
         tt: &Template<'_>,
         no_symlinks: bool,
-    ) -> Result<Self, BinstallError> {
+    ) -> Result<Self, Error> {
         let binary_ext = if data.target.contains("windows") {
             ".exe"
         } else {
@@ -115,13 +139,11 @@ impl BinFile {
             let path_normalized = Path::new(&path).normalize();
 
             if path_normalized.components().next().is_none() {
-                return Err(BinstallError::EmptySourceFilePath);
+                return Err(Error::EmptySourceFilePath);
             }
 
             if !is_valid_path(&path_normalized) {
-                return Err(BinstallError::InvalidSourceFilePath {
+                return Err(Error::InvalidSourceFilePath(path_normalized.into()));
-                    path: path_normalized,
-                });
             }
 
             (data.bin_path.join(&path_normalized), path_normalized)
@@ -176,18 +198,18 @@ impl BinFile {
     /// Return `Ok` if the source exists, otherwise `Err`.
     pub fn check_source_exists(
         &self,
-        extracted_files: &ExtractedFiles,
+        has_file: &mut dyn FnMut(&Path) -> bool,
-    ) -> Result<(), BinstallError> {
+    ) -> Result<(), Error> {
-        if extracted_files.has_file(&self.archive_source_path) {
+        if has_file(&self.archive_source_path) {
             Ok(())
         } else {
-            Err(BinstallError::BinFileNotFound(self.source.clone()))
+            Err(Error::BinFileNotFound((&*self.source).into()))
         }
     }
 
-    fn pre_install_bin(&self) -> Result<(), BinstallError> {
+    fn pre_install_bin(&self) -> Result<(), Error> {
         if !self.source.try_exists()? {
-            return Err(BinstallError::BinFileNotFound(self.source.clone()));
+            return Err(Error::BinFileNotFound((&*self.source).into()));
         }
 
         #[cfg(unix)]
@@ -199,7 +221,7 @@ impl BinFile {
         Ok(())
     }
 
-    pub fn install_bin(&self) -> Result<(), BinstallError> {
+    pub fn install_bin(&self) -> Result<(), Error> {
         self.pre_install_bin()?;
 
         debug!(
@@ -213,7 +235,7 @@ impl BinFile {
         Ok(())
     }
 
-    pub fn install_bin_noclobber(&self) -> Result<(), BinstallError> {
+    pub fn install_bin_noclobber(&self) -> Result<(), Error> {
         self.pre_install_bin()?;
 
         debug!(
@@ -227,7 +249,7 @@ impl BinFile {
         Ok(())
     }
 
-    pub fn install_link(&self) -> Result<(), BinstallError> {
+    pub fn install_link(&self) -> Result<(), Error> {
         if let Some(link) = &self.link {
             let dest = self.link_dest();
             debug!(
@@ -241,7 +263,7 @@ impl BinFile {
         Ok(())
     }
 
-    pub fn install_link_noclobber(&self) -> Result<(), BinstallError> {
+    pub fn install_link_noclobber(&self) -> Result<(), Error> {
         if let Some(link) = &self.link {
             let dest = self.link_dest();
             debug!(

crates/binstalk-fetchers/Cargo.toml

@@ -17,7 +17,7 @@ binstalk-types = { version = "0.5.0", path = "../binstalk-types" }
 compact_str = { version = "0.7.0" }
 either = "1.8.1"
 itertools = "0.11.0"
-leon = { version = "2.0.1", path = "../leon", features = ["miette"] }
+leon = { version = "2.0.1", path = "../leon" }
 leon-macros = { version = "1.0.0", path = "../leon-macros" }
 miette = "5.9.0"
 once_cell = "1.18.0"

crates/binstalk/Cargo.toml

@@ -10,7 +10,7 @@ edition = "2021"
 license = "GPL-3.0-only"
 
 [dependencies]
-atomic-file-install = { version = "0.0.0", path = "../atomic-file-install" }
+binstalk-bins = { version = "0.0.0", path = "../binstalk-bins" }
 binstalk-downloader = { version = "0.7.0", path = "../binstalk-downloader", default-features = false, features = ["gh-api-client"] }
 binstalk-fetchers = { version = "0.0.0", path = "../binstalk-fetchers" }
 binstalk-registry = { version = "0.0.0", path = "../binstalk-registry" }
@@ -20,13 +20,11 @@ command-group = { version = "2.1.0", features = ["with-tokio"] }
 compact_str = { version = "0.7.0", features = ["serde"] }
 detect-targets = { version = "0.1.10", path = "../detect-targets" }
 either = "1.8.1"
-home = "0.5.5"
 itertools = "0.11.0"
 jobslot = { version = "0.2.11", features = ["tokio"] }
 leon = { version = "2.0.1", path = "../leon" }
 maybe-owned = "0.3.4"
 miette = "5.9.0"
-normalize-path = { version = "0.2.1", path = "../normalize-path" }
 semver = { version = "1.0.17", features = ["serde"] }
 strum = "0.25.0"
 target-lexicon = { version = "0.12.11", features = ["std"] }

crates/binstalk/src/errors.rs

@@ -16,6 +16,7 @@ use tokio::task;
 use tracing::{error, warn};
 
 use crate::{
+    bins,
     helpers::{
         cargo_toml::Error as CargoTomlError, cargo_toml_workspace::Error as LoadManifestFromWSError,
     },
@@ -105,20 +106,6 @@ pub enum BinstallError {
     #[label(transparent)]
     FetchError(Box<FetchError>),
 
-    /// Failed to render template.
-    ///
-    /// - Code: `binstall::template`
-    /// - Exit: 69
-    #[error("Failed to render template: {0}")]
-    #[diagnostic(severity(error), code(binstall::template))]
-    #[source_code(transparent)]
-    #[label(transparent)]
-    TemplateRenderError(
-        #[from]
-        #[diagnostic_source]
-        leon::RenderError,
-    ),
-
     /// Failed to download or failed to decode the body.
     ///
     /// - Code: `binstall::download`
@@ -257,13 +244,17 @@ pub enum BinstallError {
     )]
     NoViableTargets,
 
-    /// Bin file is not found.
+    /// Failed to find or install binaries.
     ///
-    /// - Code: `binstall::binfile`
+    /// - Code: `binstall::bins`
     /// - Exit: 88
-    #[error("bin file {0} not found")]
+    #[error("failed to find or install binaries: {0}")]
-    #[diagnostic(severity(error), code(binstall::binfile))]
+    #[diagnostic(
-    BinFileNotFound(PathBuf),
+        severity(error),
+        code(binstall::targets::none_host),
+        help("Try to specify --target")
+    )]
+    BinFile(#[from] bins::Error),
 
     /// `Cargo.toml` of the crate does not have section "Package".
     ///
@@ -281,25 +272,6 @@ pub enum BinstallError {
     #[diagnostic(severity(error), code(binstall::SourceFilePath))]
     DuplicateSourceFilePath { path: PathBuf },
 
-    /// bin-dir configuration provided generates source path outside
-    /// of the temporary dir.
-    ///
-    /// - Code: `binstall::cargo_manifest`
-    /// - Exit: 91
-    #[error(
-        "bin-dir configuration provided generates source path outside of the temporary dir: {path}"
-    )]
-    #[diagnostic(severity(error), code(binstall::SourceFilePath))]
-    InvalidSourceFilePath { path: PathBuf },
-
-    /// bin-dir configuration provided generates empty source path.
-    ///
-    /// - Code: `binstall::cargo_manifest`
-    /// - Exit: 92
-    #[error("bin-dir configuration provided generates empty source path")]
-    #[diagnostic(severity(error), code(binstall::SourceFilePath))]
-    EmptySourceFilePath,
-
     /// Fallback to `cargo-install` is disabled.
     ///
     /// - Code: `binstall::no_fallback_to_cargo_install`
@@ -364,7 +336,6 @@ impl BinstallError {
             UrlParse(_) => 65,
             TemplateParseError(..) => 67,
             FetchError(..) => 68,
-            TemplateRenderError(..) => 69,
             Download(_) => 68,
             SubProcess { .. } => 70,
             Io(_) => 74,
@@ -377,11 +348,9 @@ impl BinstallError {
             SuperfluousVersionOption => 84,
             UnspecifiedBinaries => 86,
             NoViableTargets => 87,
-            BinFileNotFound(_) => 88,
+            BinFile(_) => 88,
             CargoTomlMissingPackage(_) => 89,
             DuplicateSourceFilePath { .. } => 90,
-            InvalidSourceFilePath { .. } => 91,
-            EmptySourceFilePath => 92,
             NoFallbackToCargoInstall => 94,
             InvalidPkgFmt(..) => 95,
             GhApiErr(..) => 96,

crates/binstalk/src/lib.rs

@@ -1,13 +1,11 @@
 #![cfg_attr(docsrs, feature(doc_auto_cfg))]
 
-mod bins;
 pub mod errors;
 pub mod helpers;
 pub mod ops;
 
-use atomic_file_install as fs;
+use binstalk_bins as bins;
 pub use binstalk_fetchers as fetchers;
 pub use binstalk_registry as registry;
 pub use binstalk_types as manifests;
 pub use detect_targets::{get_desired_targets, DesiredTargets, TARGET};
-pub use home;

crates/binstalk/src/ops/resolve.rs

@@ -261,7 +261,7 @@ async fn download_extract_and_verify(
         .iter()
         .zip(bin_files)
         .filter_map(|(bin, bin_file)| {
-            match bin_file.check_source_exists(&extracted_files) {
+            match bin_file.check_source_exists(&mut |p| extracted_files.has_file(p)) {
                 Ok(()) => Some(Ok(bin_file)),
 
                 // This binary is optional
@@ -284,7 +284,8 @@ async fn download_extract_and_verify(
                 }
             }
         })
-        .collect::<Result<Vec<bins::BinFile>, BinstallError>>()
+        .collect::<Result<Vec<bins::BinFile>, bins::Error>>()
+        .map_err(BinstallError::from)
 }
 
 fn collect_bin_files(
@@ -314,7 +315,9 @@ fn collect_bin_files(
         .bin_dir
         .as_deref()
         .map(Cow::Borrowed)
-        .unwrap_or_else(|| bins::infer_bin_dir_template(&bin_data, extracted_files));
+        .unwrap_or_else(|| {
+            bins::infer_bin_dir_template(&bin_data, &mut |p| extracted_files.get_dir(p).is_some())
+        });
 
     let template = Template::parse(&bin_dir)?;
 
@@ -323,7 +326,7 @@ fn collect_bin_files(
         .binaries
         .iter()
         .map(|bin| bins::BinFile::new(&bin_data, bin.name.as_str(), &template, no_symlinks))
-        .collect::<Result<Vec<_>, BinstallError>>()?;
+        .collect::<Result<Vec<_>, bins::Error>>()?;
 
     let mut source_set = BTreeSet::new();
 

crates/binstalk/src/ops/resolve/resolution.rs

@@ -51,7 +51,7 @@ impl Resolution {
 
 impl ResolutionFetch {
     pub fn install(self, opts: &Options) -> Result<CrateInfo, BinstallError> {
-        type InstallFp = fn(&bins::BinFile) -> Result<(), BinstallError>;
+        type InstallFp = fn(&bins::BinFile) -> Result<(), bins::Error>;
 
         let (install_bin, install_link): (InstallFp, InstallFp) = match (opts.no_track, opts.force)
         {

e2e-tests/no-track.sh

@@ -18,8 +18,8 @@ exit_code="$?"
 
 set -e
 
-if [ "$exit_code" != 74 ]; then
+if [ "$exit_code" != 88 ]; then
-    echo "Expected exit code 74 Io Error, but actual exit code $exit_code"
+    echo "Expected exit code 88 BinFile Error, but actual exit code $exit_code"
     exit 1
 fi