Feature: SupportdDisable of strategies for crate using Cargo.toml (#…

…1828)

* Refactor: Move `Strategy` to `binstalk-types`

Signed-off-by: Jiahao XU <[email protected]>

* Add serialisation test for `Strategy`

Signed-off-by: Jiahao XU <[email protected]>

* Add support to disable strategies via crate `Cargo.toml`

Signed-off-by: Jiahao XU <[email protected]>

* Add e2e-test

Signed-off-by: Jiahao XU <[email protected]>

* Fix `Cargo.toml` disabled strategy checking for compile strategy

Signed-off-by: Jiahao XU <[email protected]>

* Optimize `resolve_inner`: Cache meta override

Signed-off-by: Jiahao XU <[email protected]>

* Add compile-time length checking for `Strategy`

Signed-off-by: Jiahao XU <[email protected]>

* More optimization

Signed-off-by: Jiahao XU <[email protected]>

* Fix order of override: cli options alwayus takes precedence

Signed-off-by: Jiahao XU <[email protected]>

* Add missing manifest for e2e-test

Signed-off-by: Jiahao XU <[email protected]>

---------

Signed-off-by: Jiahao XU <[email protected]>

crates/bin/src/args.rs

@@ -13,12 +13,12 @@ use binstalk::{
     ops::resolve::{CrateName, VersionReqExt},
     registry::Registry,
 };
-use clap::{error::ErrorKind, CommandFactory, Parser, ValueEnum};
+use binstalk_manifests::cargo_toml_binstall::Strategy;
+use clap::{builder::PossibleValue, error::ErrorKind, CommandFactory, Parser, ValueEnum};
 use compact_str::CompactString;
 use log::LevelFilter;
 use semver::VersionReq;
 use strum::EnumCount;
-use strum_macros::EnumCount;
 use zeroize::Zeroizing;
 
 #[derive(Debug, Parser)]
@@ -162,13 +162,13 @@ pub struct Args {
         value_delimiter(','),
         env = "BINSTALL_STRATEGIES"
     )]
-    pub(crate) strategies: Vec<Strategy>,
+    pub(crate) strategies: Vec<StrategyWrapped>,
 
     /// Disable the strategies specified.
     /// If a strategy is specified in `--strategies` and `--disable-strategies`,
     /// then it will be removed.
     #[clap(help_heading = "Overrides", long, value_delimiter(','))]
-    pub(crate) disable_strategies: Vec<Strategy>,
+    pub(crate) disable_strategies: Vec<StrategyWrapped>,
 
     /// If `--github-token` or environment variable `GITHUB_TOKEN`/`GH_TOKEN`
     /// is not specified, then cargo-binstall will try to extract github token from
@@ -431,16 +431,24 @@ impl Default for RateLimit {
 }
 
 /// Strategy for installing the package
-#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd, ValueEnum, EnumCount)]
-#[repr(u8)]
-pub(crate) enum Strategy {
-    /// Attempt to download official pre-built artifacts using
-    /// information provided in `Cargo.toml`.
-    CrateMetaData,
-    /// Query third-party QuickInstall for the crates.
-    QuickInstall,
-    /// Build the crates from source using `cargo-build`.
-    Compile,
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
+pub(crate) struct StrategyWrapped(pub(crate) Strategy);
+
+impl StrategyWrapped {
+    const VARIANTS: &'static [Self; 3] = &[
+        Self(Strategy::CrateMetaData),
+        Self(Strategy::QuickInstall),
+        Self(Strategy::Compile),
+    ];
+}
+
+impl ValueEnum for StrategyWrapped {
+    fn value_variants<'a>() -> &'a [Self] {
+        Self::VARIANTS
+    }
+    fn to_possible_value(&self) -> Option<PossibleValue> {
+        Some(PossibleValue::new(self.0.to_str()))
+    }
 }
 
 pub fn parse() -> Args {
@@ -532,7 +540,7 @@ You cannot use --{option} and specify multiple packages at the same time. Do one
     let mut is_variant_present = [false; Strategy::COUNT];
 
     for strategy in &opts.strategies {
-        let index = *strategy as u8 as usize;
+        let index = strategy.0 as u8 as usize;
         if is_variant_present[index] {
             new_dup_strategy_err().exit()
         } else {
@@ -543,9 +551,9 @@ You cannot use --{option} and specify multiple packages at the same time. Do one
     // Default strategies if empty
     if opts.strategies.is_empty() {
         opts.strategies = vec![
-            Strategy::CrateMetaData,
-            Strategy::QuickInstall,
-            Strategy::Compile,
+            StrategyWrapped(Strategy::CrateMetaData),
+            StrategyWrapped(Strategy::QuickInstall),
+            StrategyWrapped(Strategy::Compile),
         ];
     }
 
@@ -573,7 +581,8 @@ You cannot use --{option} and specify multiple packages at the same time. Do one
     }
 
     // Ensure that Strategy::Compile is specified as the last strategy
-    if opts.strategies[..(opts.strategies.len() - 1)].contains(&Strategy::Compile) {
+    if opts.strategies[..(opts.strategies.len() - 1)].contains(&StrategyWrapped(Strategy::Compile))
+    {
         command
             .error(
                 ErrorKind::InvalidValue,
@@ -593,10 +602,14 @@ You cannot use --{option} and specify multiple packages at the same time. Do one
 
 #[cfg(test)]
 mod test {
+    use strum::VariantArray;
+
     use super::*;
 
     #[test]
     fn verify_cli() {
         Args::command().debug_assert()
     }
+
+    const _: () = assert!(Strategy::VARIANTS.len() == StrategyWrapped::VARIANTS.len());
 }

crates/bin/src/entry.rs

@@ -21,7 +21,9 @@ use binstalk::{
     },
 };
 use binstalk_manifests::{
-    cargo_config::Config, cargo_toml_binstall::PkgOverride, crates_manifests::Manifests,
+    cargo_config::Config,
+    cargo_toml_binstall::{PkgOverride, Strategy},
+    crates_manifests::Manifests,
 };
 use file_format::FileFormat;
 use home::cargo_home;
@@ -30,11 +32,7 @@ use miette::{miette, Report, Result, WrapErr};
 use tokio::task::block_in_place;
 use tracing::{debug, error, info, warn};
 
-use crate::{
-    args::{Args, Strategy},
-    gh_token, git_credentials, install_path,
-    ui::confirm,
-};
+use crate::{args::Args, gh_token, git_credentials, install_path, ui::confirm};
 
 pub fn install_crates(
     args: Args,
@@ -46,7 +44,7 @@ pub fn install_crates(
     let resolvers: Vec<_> = args
         .strategies
         .into_iter()
-        .filter_map(|strategy| match strategy {
+        .filter_map(|strategy| match strategy.0 {
             Strategy::CrateMetaData => Some(GhCrateMeta::new as Resolver),
             Strategy::QuickInstall => Some(QuickInstall::new as Resolver),
             Strategy::Compile => {
@@ -87,6 +85,7 @@ pub fn install_crates(
         pkg_url: args.pkg_url,
         pkg_fmt: args.pkg_fmt,
         bin_dir: args.bin_dir,
+        disabled_strategies: None,
         signing: None,
     };
 

crates/binstalk-fetchers/src/gh_crate_meta.rs

@@ -1,6 +1,7 @@
 use std::{borrow::Cow, fmt, iter, path::Path, sync::Arc};
 
 use binstalk_git_repo_api::gh_api_client::{GhApiError, GhReleaseArtifact, GhReleaseArtifactUrl};
+use binstalk_types::cargo_toml_binstall::Strategy;
 use compact_str::{CompactString, ToCompactString};
 use either::Either;
 use leon::Template;
@@ -396,6 +397,10 @@ impl super::Fetcher for GhCrateMeta {
         FETCHER_GH_CRATE_META
     }
 
+    fn strategy(&self) -> Strategy {
+        Strategy::CrateMetaData
+    }
+
     fn is_third_party(&self) -> bool {
         false
     }

crates/binstalk-fetchers/src/lib.rs

@@ -4,7 +4,7 @@ use std::{path::Path, sync::Arc, time::Duration};
 
 use binstalk_downloader::{download::DownloadError, remote::Error as RemoteError};
 use binstalk_git_repo_api::gh_api_client::{GhApiError, GhRepo, RepoInfo as GhRepoInfo};
-use binstalk_types::cargo_toml_binstall::SigningAlgorithm;
+use binstalk_types::cargo_toml_binstall::{SigningAlgorithm, Strategy};
 use thiserror::Error as ThisError;
 use tokio::{sync::OnceCell, task::JoinError, time::sleep};
 pub use url::ParseError as UrlParseError;
@@ -134,6 +134,9 @@ pub trait Fetcher: Send + Sync {
     /// [`Fetcher::fetch_and_extract`].
     fn fetcher_name(&self) -> &'static str;
 
+    /// The strategy used by this fetcher
+    fn strategy(&self) -> Strategy;
+
     /// Should return true if the remote is from a third-party source
     fn is_third_party(&self) -> bool;
 

crates/binstalk-fetchers/src/quickinstall.rs

@@ -5,7 +5,7 @@ use std::{
 };
 
 use binstalk_downloader::remote::Method;
-use binstalk_types::cargo_toml_binstall::{PkgFmt, PkgMeta, PkgSigning};
+use binstalk_types::cargo_toml_binstall::{PkgFmt, PkgMeta, PkgSigning, Strategy};
 use tokio::sync::OnceCell;
 use tracing::{error, info, trace};
 use url::Url;
@@ -252,6 +252,10 @@ by rust officially."#,
         "QuickInstall"
     }
 
+    fn strategy(&self) -> Strategy {
+        Strategy::QuickInstall
+    }
+
     fn is_third_party(&self) -> bool {
         true
     }

crates/binstalk-types/Cargo.toml

@@ -18,3 +18,6 @@ serde = { version = "1.0.163", features = ["derive"] }
 strum = "0.26.1"
 strum_macros = "0.26.1"
 url = { version = "2.3.1", features = ["serde"] }
+
+[dev-dependencies]
+serde_json = "1"

crates/binstalk-types/src/cargo_toml_binstall.rs

@@ -5,6 +5,7 @@
 use std::{borrow::Cow, collections::BTreeMap};
 
 use serde::{Deserialize, Serialize};
+use strum_macros::{EnumCount, VariantArray};
 
 mod package_formats;
 #[doc(inline)]
@@ -19,6 +20,41 @@ pub struct Meta {
     pub binstall: Option<PkgMeta>,
 }
 
+/// Strategies to use for binary discovery
+#[derive(
+    Debug,
+    Copy,
+    Clone,
+    Eq,
+    PartialEq,
+    Ord,
+    PartialOrd,
+    EnumCount,
+    VariantArray,
+    Deserialize,
+    Serialize,
+)]
+#[serde(rename_all = "kebab-case")]
+pub enum Strategy {
+    /// Attempt to download official pre-built artifacts using
+    /// information provided in `Cargo.toml`.
+    CrateMetaData,
+    /// Query third-party QuickInstall for the crates.
+    QuickInstall,
+    /// Build the crates from source using `cargo-build`.
+    Compile,
+}
+
+impl Strategy {
+    pub const fn to_str(self) -> &'static str {
+        match self {
+            Strategy::CrateMetaData => "crate-meta-data",
+            Strategy::QuickInstall => "quick-install",
+            Strategy::Compile => "compile",
+        }
+    }
+}
+
 /// Metadata for binary installation use.
 ///
 /// Exposed via `[package.metadata]` in `Cargo.toml`
@@ -37,6 +73,9 @@ pub struct PkgMeta {
     /// Package signing configuration
     pub signing: Option<PkgSigning>,
 
+    /// Stratgies to disable
+    pub disabled_strategies: Option<Box<[Strategy]>>,
+
     /// Target specific overrides
     pub overrides: BTreeMap<String, PkgOverride>,
 }
@@ -82,10 +121,16 @@ impl PkgMeta {
                 .or_else(|| self.bin_dir.clone()),
 
             signing: pkg_overrides
+                .clone()
                 .into_iter()
                 .find_map(|pkg_override| pkg_override.signing.clone())
                 .or_else(|| self.signing.clone()),
 
+            disabled_strategies: pkg_overrides
+                .into_iter()
+                .find_map(|pkg_override| pkg_override.disabled_strategies.clone())
+                .or_else(|| self.disabled_strategies.clone()),
+
             overrides: Default::default(),
         }
     }
@@ -106,6 +151,9 @@ pub struct PkgOverride {
     /// Path template override for binary files in packages
     pub bin_dir: Option<String>,
 
+    /// Stratgies to disable
+    pub disabled_strategies: Option<Box<[Strategy]>>,
+
     /// Package signing configuration
     pub signing: Option<PkgSigning>,
 }
@@ -141,3 +189,20 @@ pub enum SigningAlgorithm {
     /// [minisign](https://jedisct1.github.io/minisign/)
     Minisign,
 }
+
+#[cfg(test)]
+mod tests {
+    use strum::VariantArray;
+
+    use super::*;
+
+    #[test]
+    fn test_strategy_ser() {
+        Strategy::VARIANTS.iter().for_each(|strategy| {
+            assert_eq!(
+                serde_json::to_string(&strategy).unwrap(),
+                format!(r#""{}""#, strategy.to_str())
+            )
+        });
+    }
+}