From 832a6c13ae1c2f3d0bf7d3b2d77377022c8789bb Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Fri, 19 Jul 2024 00:19:01 +1000 Subject: [PATCH 01/10] Refactor: Move `Strategy` to `binstalk-types` Signed-off-by: Jiahao XU --- crates/bin/src/args.rs | 43 +++++++++++-------- crates/bin/src/entry.rs | 12 +++--- .../binstalk-types/src/cargo_toml_binstall.rs | 36 ++++++++++++++++ 3 files changed, 65 insertions(+), 26 deletions(-) diff --git a/crates/bin/src/args.rs b/crates/bin/src/args.rs index e81f19709..4f7cf65a1 100644 --- a/crates/bin/src/args.rs +++ b/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, + pub(crate) strategies: Vec, /// 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, + pub(crate) disable_strategies: Vec, /// 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,20 @@ 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 ValueEnum for StrategyWrapped { + fn value_variants<'a>() -> &'a [Self] { + &[ + Self(Strategy::CrateMetaData), + Self(Strategy::QuickInstall), + Self(Strategy::Compile), + ] + } + fn to_possible_value(&self) -> Option { + Some(PossibleValue::new(self.0.to_str())) + } } pub fn parse() -> Args { @@ -532,7 +536,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 +547,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 +577,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, diff --git a/crates/bin/src/entry.rs b/crates/bin/src/entry.rs index 48288aa2c..3ae83141a 100644 --- a/crates/bin/src/entry.rs +++ b/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 => { diff --git a/crates/binstalk-types/src/cargo_toml_binstall.rs b/crates/binstalk-types/src/cargo_toml_binstall.rs index 2c9af131d..02c504f7d 100644 --- a/crates/binstalk-types/src/cargo_toml_binstall.rs +++ b/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, } +/// 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` From ac19952110ca0d02d96e017412ca965543426610 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 22 Jul 2024 23:24:02 +1000 Subject: [PATCH 02/10] Add serialisation test for `Strategy` Signed-off-by: Jiahao XU --- Cargo.lock | 1 + crates/binstalk-types/Cargo.toml | 3 +++ .../binstalk-types/src/cargo_toml_binstall.rs | 17 +++++++++++++++++ 3 files changed, 21 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index b229b76b6..276791a3c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -425,6 +425,7 @@ dependencies = [ "once_cell", "semver", "serde", + "serde_json", "strum", "strum_macros", "url", diff --git a/crates/binstalk-types/Cargo.toml b/crates/binstalk-types/Cargo.toml index 8560eb7c2..54e5d0ed7 100644 --- a/crates/binstalk-types/Cargo.toml +++ b/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" diff --git a/crates/binstalk-types/src/cargo_toml_binstall.rs b/crates/binstalk-types/src/cargo_toml_binstall.rs index 02c504f7d..1684ac606 100644 --- a/crates/binstalk-types/src/cargo_toml_binstall.rs +++ b/crates/binstalk-types/src/cargo_toml_binstall.rs @@ -177,3 +177,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()) + ) + }); + } +} From f7643e1da10a50b53bacc6a902b6b74a02a9ce95 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 22 Jul 2024 23:54:50 +1000 Subject: [PATCH 03/10] Add support to disable strategies via crate `Cargo.toml` Signed-off-by: Jiahao XU --- crates/bin/src/entry.rs | 1 + crates/binstalk-fetchers/src/gh_crate_meta.rs | 5 +++++ crates/binstalk-fetchers/src/lib.rs | 5 ++++- crates/binstalk-fetchers/src/quickinstall.rs | 6 +++++- crates/binstalk-types/src/cargo_toml_binstall.rs | 12 ++++++++++++ crates/binstalk/src/ops/resolve.rs | 9 +++++++++ 6 files changed, 36 insertions(+), 2 deletions(-) diff --git a/crates/bin/src/entry.rs b/crates/bin/src/entry.rs index 3ae83141a..770c72fb3 100644 --- a/crates/bin/src/entry.rs +++ b/crates/bin/src/entry.rs @@ -85,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, }; diff --git a/crates/binstalk-fetchers/src/gh_crate_meta.rs b/crates/binstalk-fetchers/src/gh_crate_meta.rs index 51cd012cd..d1df06370 100644 --- a/crates/binstalk-fetchers/src/gh_crate_meta.rs +++ b/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 } diff --git a/crates/binstalk-fetchers/src/lib.rs b/crates/binstalk-fetchers/src/lib.rs index 5d4f692a0..49a868949 100644 --- a/crates/binstalk-fetchers/src/lib.rs +++ b/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; diff --git a/crates/binstalk-fetchers/src/quickinstall.rs b/crates/binstalk-fetchers/src/quickinstall.rs index 2862fb895..ad5d2f7d0 100644 --- a/crates/binstalk-fetchers/src/quickinstall.rs +++ b/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 } diff --git a/crates/binstalk-types/src/cargo_toml_binstall.rs b/crates/binstalk-types/src/cargo_toml_binstall.rs index 1684ac606..e1ab03c66 100644 --- a/crates/binstalk-types/src/cargo_toml_binstall.rs +++ b/crates/binstalk-types/src/cargo_toml_binstall.rs @@ -73,6 +73,9 @@ pub struct PkgMeta { /// Package signing configuration pub signing: Option, + /// Stratgies to disable + pub disabled_strategies: Option>, + /// Target specific overrides pub overrides: BTreeMap, } @@ -118,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(), } } @@ -142,6 +151,9 @@ pub struct PkgOverride { /// Path template override for binary files in packages pub bin_dir: Option, + /// Stratgies to disable + pub disabled_strategies: Option>, + /// Package signing configuration pub signing: Option, } diff --git a/crates/binstalk/src/ops/resolve.rs b/crates/binstalk/src/ops/resolve.rs index 5c6379d9a..36be67e3e 100644 --- a/crates/binstalk/src/ops/resolve.rs +++ b/crates/binstalk/src/ops/resolve.rs @@ -134,6 +134,8 @@ async fn resolve_inner( }, )) .filter_map(|(f, target_data)| { + let disabled_strategies = target_data.meta.disabled_strategies.clone(); + let fetcher = f( opts.client.clone(), gh_api_client.clone(), @@ -141,6 +143,13 @@ async fn resolve_inner( target_data, opts.signature_policy, ); + + if let Some(disabled_strategies) = disabled_strategies.as_deref() { + if disabled_strategies.contains(&fetcher.strategy()) { + return None; + } + } + filter_fetcher_by_name_predicate(fetcher.fetcher_name()).then_some(fetcher) }), ) From b004360576cb92354a104c3b852e64100922cc20 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 23 Jul 2024 00:06:01 +1000 Subject: [PATCH 04/10] Add e2e-test Signed-off-by: Jiahao XU --- e2e-tests/strategies.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/e2e-tests/strategies.sh b/e2e-tests/strategies.sh index 1a91c894b..0248d6c37 100755 --- a/e2e-tests/strategies.sh +++ b/e2e-tests/strategies.sh @@ -36,3 +36,16 @@ fi ## Test compile-only strategy "./$1" binstall --no-confirm --strategies compile cargo-quickinstall@0.2.8 + +## Test --disable-strategies +set +e + +"./$1" binstall --no-confirm --manifest-path "manifests/strategies-test-Cargo.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 From 898796b91d2b8c8513862913e2591fa619013615 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 23 Jul 2024 00:21:49 +1000 Subject: [PATCH 05/10] Fix `Cargo.toml` disabled strategy checking for compile strategy Signed-off-by: Jiahao XU --- crates/binstalk/src/ops/resolve.rs | 33 +++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/crates/binstalk/src/ops/resolve.rs b/crates/binstalk/src/ops/resolve.rs index 36be67e3e..799324efd 100644 --- a/crates/binstalk/src/ops/resolve.rs +++ b/crates/binstalk/src/ops/resolve.rs @@ -8,7 +8,10 @@ use std::{ }; use binstalk_fetchers::FETCHER_GH_CRATE_META; -use binstalk_types::crate_info::{CrateSource, SourceType}; +use binstalk_types::{ + cargo_toml_binstall::Strategy, + crate_info::{CrateSource, SourceType}, +}; use compact_str::{CompactString, ToCompactString}; use itertools::Itertools; use leon::Template; @@ -240,14 +243,30 @@ async fn resolve_inner( } } - if opts.cargo_install_fallback { - Ok(Resolution::InstallFromSource(ResolutionSource { - name: package_info.name, - version: package_info.version_str, - })) + if !opts.cargo_install_fallback { + return Err(BinstallError::NoFallbackToCargoInstall); + } + + let target_meta = if let Some((_, target)) = desired_targets.get(0) { + package_info.meta.merge_overrides( + iter::once(&opts.cli_overrides).chain(package_info.overrides.get(*target)), + ) } else { - Err(BinstallError::NoFallbackToCargoInstall) + package_info + .meta + .merge_overrides(iter::once(&opts.cli_overrides)) + }; + + if let Some(disabled_strategies) = target_meta.disabled_strategies.as_deref() { + if disabled_strategies.contains(&Strategy::Compile) { + return Err(BinstallError::NoFallbackToCargoInstall); + } } + + return Ok(Resolution::InstallFromSource(ResolutionSource { + name: package_info.name, + version: package_info.version_str, + })); } /// * `fetcher` - `fetcher.find()` must have returned `Ok(true)`. From ebafba4e9bf4b077e5501fe37924a7e2338d05c2 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 23 Jul 2024 00:30:57 +1000 Subject: [PATCH 06/10] Optimize `resolve_inner`: Cache meta override Signed-off-by: Jiahao XU --- crates/binstalk/src/ops/resolve.rs | 40 +++++++++++++++--------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/crates/binstalk/src/ops/resolve.rs b/crates/binstalk/src/ops/resolve.rs index 799324efd..71ad8288a 100644 --- a/crates/binstalk/src/ops/resolve.rs +++ b/crates/binstalk/src/ops/resolve.rs @@ -88,13 +88,23 @@ async fn resolve_inner( return Ok(Resolution::AlreadyUpToDate); }; + let meta = package_info + .meta + .merge_overrides(iter::once(&opts.cli_overrides)); + let desired_targets = opts .desired_targets .get() .await .iter() - .map(|target| TargetTriple::from_str(target).map(|triple| (triple, target))) - .collect::, _>>()?; + .map(|target| { + Ok(( + TargetTriple::from_str(target)?, + target, + meta.merge_overrides(package_info.overrides.get(target)), + )) + }) + .collect::, BinstallError>>()?; let resolvers = &opts.resolvers; let binary_name = match package_info.binaries.as_slice() { @@ -119,19 +129,14 @@ async fn resolve_inner( resolvers .iter() .cartesian_product(desired_targets.clone().into_iter().map( - |(triple, target)| { + |(triple, target, target_meta)| { debug!("Building metadata for target: {target}"); - let target_meta = package_info.meta.merge_overrides( - iter::once(&opts.cli_overrides) - .chain(package_info.overrides.get(target)), - ); - debug!("Found metadata: {target_meta:?}"); Arc::new(TargetData { target: target.clone(), - meta: target_meta, + meta: target_meta.clone(), target_related_info: triple, }) }, @@ -247,15 +252,10 @@ async fn resolve_inner( return Err(BinstallError::NoFallbackToCargoInstall); } - let target_meta = if let Some((_, target)) = desired_targets.get(0) { - package_info.meta.merge_overrides( - iter::once(&opts.cli_overrides).chain(package_info.overrides.get(*target)), - ) - } else { - package_info - .meta - .merge_overrides(iter::once(&opts.cli_overrides)) - }; + let target_meta = desired_targets + .first() + .map(|(_, _, target_meta)| target_meta) + .unwrap_or(&meta); if let Some(disabled_strategies) = target_meta.disabled_strategies.as_deref() { if disabled_strategies.contains(&Strategy::Compile) { @@ -263,10 +263,10 @@ async fn resolve_inner( } } - return Ok(Resolution::InstallFromSource(ResolutionSource { + Ok(Resolution::InstallFromSource(ResolutionSource { name: package_info.name, version: package_info.version_str, - })); + })) } /// * `fetcher` - `fetcher.find()` must have returned `Ok(true)`. From 5e62012ce3cb9c27627dec3192682a0657a12c93 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 23 Jul 2024 00:43:32 +1000 Subject: [PATCH 07/10] Add compile-time length checking for `Strategy` Signed-off-by: Jiahao XU --- crates/bin/src/args.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/crates/bin/src/args.rs b/crates/bin/src/args.rs index 4f7cf65a1..061fbdab2 100644 --- a/crates/bin/src/args.rs +++ b/crates/bin/src/args.rs @@ -434,13 +434,17 @@ impl Default for RateLimit { #[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(Strategy::CrateMetaData), - Self(Strategy::QuickInstall), - Self(Strategy::Compile), - ] + Self::VARIANTS } fn to_possible_value(&self) -> Option { Some(PossibleValue::new(self.0.to_str())) @@ -598,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()); } From a34f86f989bebd537a70493643c9fb54a2e34651 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 23 Jul 2024 00:55:45 +1000 Subject: [PATCH 08/10] More optimization Signed-off-by: Jiahao XU --- crates/binstalk/src/ops/resolve.rs | 40 +++++++++++++----------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/crates/binstalk/src/ops/resolve.rs b/crates/binstalk/src/ops/resolve.rs index 71ad8288a..b23fde568 100644 --- a/crates/binstalk/src/ops/resolve.rs +++ b/crates/binstalk/src/ops/resolve.rs @@ -98,11 +98,17 @@ async fn resolve_inner( .await .iter() .map(|target| { - Ok(( - TargetTriple::from_str(target)?, - target, - meta.merge_overrides(package_info.overrides.get(target)), - )) + debug!("Building metadata for target: {target}"); + + let meta = meta.merge_overrides(package_info.overrides.get(target)); + + debug!("Found metadata: {meta:?}"); + + Ok(Arc::new(TargetData { + target: target.clone(), + meta, + target_related_info: TargetTriple::from_str(target)?, + })) }) .collect::, BinstallError>>()?; let resolvers = &opts.resolvers; @@ -128,31 +134,19 @@ async fn resolve_inner( handles.extend( resolvers .iter() - .cartesian_product(desired_targets.clone().into_iter().map( - |(triple, target, target_meta)| { - debug!("Building metadata for target: {target}"); - - debug!("Found metadata: {target_meta:?}"); - - Arc::new(TargetData { - target: target.clone(), - meta: target_meta.clone(), - target_related_info: triple, - }) - }, - )) + .cartesian_product(&desired_targets) .filter_map(|(f, target_data)| { - let disabled_strategies = target_data.meta.disabled_strategies.clone(); - let fetcher = f( opts.client.clone(), gh_api_client.clone(), data.clone(), - target_data, + target_data.clone(), opts.signature_policy, ); - if let Some(disabled_strategies) = disabled_strategies.as_deref() { + if let Some(disabled_strategies) = + target_data.meta.disabled_strategies.as_deref() + { if disabled_strategies.contains(&fetcher.strategy()) { return None; } @@ -254,7 +248,7 @@ async fn resolve_inner( let target_meta = desired_targets .first() - .map(|(_, _, target_meta)| target_meta) + .map(|target_data| &target_data.meta) .unwrap_or(&meta); if let Some(disabled_strategies) = target_meta.disabled_strategies.as_deref() { From 3f99d23010a361923933db81cd83c75d73c44988 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 23 Jul 2024 23:37:56 +1000 Subject: [PATCH 09/10] Fix order of override: cli options alwayus takes precedence Signed-off-by: Jiahao XU --- crates/binstalk/src/ops/resolve.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/binstalk/src/ops/resolve.rs b/crates/binstalk/src/ops/resolve.rs index b23fde568..426e478bc 100644 --- a/crates/binstalk/src/ops/resolve.rs +++ b/crates/binstalk/src/ops/resolve.rs @@ -88,10 +88,6 @@ async fn resolve_inner( return Ok(Resolution::AlreadyUpToDate); }; - let meta = package_info - .meta - .merge_overrides(iter::once(&opts.cli_overrides)); - let desired_targets = opts .desired_targets .get() @@ -100,7 +96,9 @@ async fn resolve_inner( .map(|target| { debug!("Building metadata for target: {target}"); - let meta = meta.merge_overrides(package_info.overrides.get(target)); + let meta = package_info.meta.merge_overrides( + iter::once(&opts.cli_overrides).chain(package_info.overrides.get(target)), + ); debug!("Found metadata: {meta:?}"); @@ -246,6 +244,10 @@ async fn resolve_inner( return Err(BinstallError::NoFallbackToCargoInstall); } + let meta = package_info + .meta + .merge_overrides(iter::once(&opts.cli_overrides)); + let target_meta = desired_targets .first() .map(|target_data| &target_data.meta) From f399499a48d89bdb366bbfc5acad9c64529b3d01 Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Tue, 23 Jul 2024 23:59:56 +1000 Subject: [PATCH 10/10] Add missing manifest for e2e-test Signed-off-by: Jiahao XU --- .../manifests/strategies-test-Cargo.toml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 e2e-tests/manifests/strategies-test-Cargo.toml diff --git a/e2e-tests/manifests/strategies-test-Cargo.toml b/e2e-tests/manifests/strategies-test-Cargo.toml new file mode 100644 index 000000000..f4235a84c --- /dev/null +++ b/e2e-tests/manifests/strategies-test-Cargo.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", "compile"]