feature: Merge disable strategies (#1868)

* feat: Merge --disable-strategies with ones in cargo manifest

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

* Update doc

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

* Update e2e-test-strategies

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

* Fix typo in option doc in crates/bin/src/args.rs

Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

---------

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
Signed-off-by: Jiahao XU <30436523+NobodyXu@users.noreply.github.com>

SUPPORT.md

@@ -13,6 +13,7 @@ As an example, the configuration would be like this:
 pkg-url = "{ repo }/releases/download/v{ version }/{ name }-{ target }-v{ version }{ archive-suffix }"
 bin-dir = "{ name }-{ target }-v{ version }/{ bin }{ binary-ext }"
 pkg-fmt = "tgz"
+disabled-strategies = ["quick-install", "compile"]
 ```
 
 With the following configuration keys:
@@ -20,7 +21,11 @@ With the following configuration keys:
 - `pkg-url` specifies the package download URL for a given target/version, templated
 - `bin-dir` specifies the binary path within the package, templated (with an `.exe` suffix on windows)
 - `pkg-fmt` overrides the package format for download/extraction (defaults to: `tgz`)
-- `disabled-strategies` to disable specific strategies (e.g. `crate-meta-data` for trying to find pre-built on your repository, `quick-install` for pre-built from third-party cargo-bins/cargo-quickinstall, `compile` for falling back to `cargo-install`) for your crate (defaults to empty array). The user can override this by explicitly specifying --strategies on the command line.
+- `disabled-strategies` to disable specific strategies (e.g. `crate-meta-data` for trying to find pre-built on your repository,
+  `quick-install` for pre-built from third-party cargo-bins/cargo-quickinstall, `compile` for falling back to `cargo-install`)
+  for your crate (defaults to empty array).
+  If `--strategies` is passed on the command line, then the `disabled-strategies` in `package.metadata` will be ignored.
+  Otherwise, the `disabled-strategies` in `package.metadata` and `--disable-strategies` will be merged.
 
 
 `pkg-url` and `bin-dir` are templated to support different names for different versions / architectures / etc.

crates/bin/src/args.rs

@@ -1,7 +1,7 @@
 use std::{
     env,
     ffi::OsString,
-    fmt,
+    fmt, mem,
     num::{NonZeroU16, NonZeroU64, ParseIntError},
     path::PathBuf,
     str::FromStr,
@@ -155,6 +155,10 @@ pub struct Args {
     /// Specify the strategies to be used,
     /// binstall will run the strategies specified in order.
     ///
+    /// If this option is specified, then cargo-binstall will ignore
+    /// `disabled-strategies` in `package.metadata` in the cargo manifest
+    /// of the installed packages.
+    ///
     /// Default value is "crate-meta-data,quick-install,compile".
     #[clap(
         help_heading = "Overrides",
@@ -167,6 +171,10 @@ pub struct Args {
     /// Disable the strategies specified.
     /// If a strategy is specified in `--strategies` and `--disable-strategies`,
     /// then it will be removed.
+    ///
+    /// If `--strategies` is not specified, then the strategies specified in this
+    /// option will be merged with the  disabled-strategies` in `package.metadata`
+    /// in the cargo manifest of the installed packages.
     #[clap(
         help_heading = "Overrides",
         long,
@@ -570,7 +578,7 @@ You cannot use --{option} and specify multiple packages at the same time. Do one
         }
     }
 
-    let has_strategies_override = !opts.strategies.is_empty();
+    let ignore_disabled_strategies = !opts.strategies.is_empty();
 
     // Default strategies if empty
     if opts.strategies.is_empty() {
@@ -626,15 +634,14 @@ You cannot use --{option} and specify multiple packages at the same time. Do one
         pkg_url: opts.pkg_url.take(),
         pkg_fmt: opts.pkg_fmt.take(),
         bin_dir: opts.bin_dir.take(),
-        disabled_strategies: (!opts.disable_strategies.is_empty() || has_strategies_override).then(
-            || {
-                opts.disable_strategies
-                    .iter()
-                    .map(|strategy| strategy.0)
-                    .collect::<Vec<_>>()
-                    .into_boxed_slice()
-            },
+        disabled_strategies: Some(
+            mem::take(&mut opts.disable_strategies)
+                .into_iter()
+                .map(|strategy| strategy.0)
+                .collect::<Vec<_>>()
+                .into_boxed_slice(),
         ),
+        ignore_disabled_strategies,
         signing: None,
     };
 

crates/binstalk-types/src/cargo_toml_binstall.rs

@@ -101,6 +101,11 @@ impl PkgMeta {
     where
         It: IntoIterator<Item = &'a PkgOverride> + Clone,
     {
+        let ignore_disabled_strategies = pkg_overrides
+            .clone()
+            .into_iter()
+            .any(|pkg_override| pkg_override.ignore_disabled_strategies);
+
         Self {
             pkg_url: pkg_overrides
                 .clone()
@@ -126,10 +131,22 @@ impl PkgMeta {
                 .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()),
+            disabled_strategies: if ignore_disabled_strategies {
+                None
+            } else {
+                let mut disabled_strategies = pkg_overrides
+                    .into_iter()
+                    .filter_map(|pkg_override| pkg_override.disabled_strategies.as_deref())
+                    .flatten()
+                    .chain(self.disabled_strategies.as_deref().into_iter().flatten())
+                    .copied()
+                    .collect::<Vec<Strategy>>();
+
+                disabled_strategies.sort_unstable();
+                disabled_strategies.dedup();
+
+                Some(disabled_strategies.into_boxed_slice())
+            },
 
             overrides: Default::default(),
         }
@@ -156,6 +173,9 @@ pub struct PkgOverride {
 
     /// Package signing configuration
     pub signing: Option<PkgSigning>,
+
+    #[serde(skip)]
+    pub ignore_disabled_strategies: bool,
 }
 
 #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]

e2e-tests/manifests/strategies-test-Cargo2.toml

@@ -0,0 +1,19 @@
+[package]
+name = "cargo-update"
+repository = "https://github.com/nabijaczleweli/cargo-update"
+version = "11.1.2"
+
+[[bin]]
+name = "cargo-install-update"
+path = "src/main.rs"
+test = false
+doc = false
+
+[[bin]]
+name = "cargo-install-update-config"
+path = "src/main-config.rs"
+test = false
+doc = false
+
+[package.metadata.binstall]
+disabled-strategies = ["quick-install"]

e2e-tests/strategies.sh

@@ -50,10 +50,17 @@ if [ "$exit_code" != 94 ]; then
     exit 1
 fi
 
-set -euxo pipefail
+set +e
+
+"./$1" binstall --disable-strategies compile --no-confirm --manifest-path "manifests/strategies-test-Cargo2.toml" cargo-update@11.1.2
+exit_code="$?"
+
+set -e
+
+if [ "$exit_code" != 94 ]; then
+    echo "Expected exit code 94, but actual exit code $exit_code"
+    exit 1
+fi
 
 ## Test --strategies overriding `disabled-strategies=["compile"]` in Cargo.toml
  "./$1" binstall --no-confirm --manifest-path "manifests/strategies-test-override-Cargo.toml" --strategies compile cargo-quickinstall@0.2.10
-
-## Test --disable-strategies overriding `disabled-strategies=["compile"]` in Cargo.toml
- "./$1" binstall --no-confirm --manifest-path "manifests/strategies-test-override-Cargo.toml" --disable-strategies crate-meta-data,quick-install --force cargo-quickinstall@0.2.10