Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Jul 23, 2024
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

51 changes: 32 additions & 19 deletions crates/bin/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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`.
passcod marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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),
];
}

Expand Down Expand Up @@ -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,
Expand All @@ -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());
}
13 changes: 6 additions & 7 deletions crates/bin/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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 => {
Expand Down Expand Up @@ -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,
};

Expand Down
5 changes: 5 additions & 0 deletions crates/binstalk-fetchers/src/gh_crate_meta.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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
}
Expand Down
5 changes: 4 additions & 1 deletion crates/binstalk-fetchers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down
6 changes: 5 additions & 1 deletion crates/binstalk-fetchers/src/quickinstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -252,6 +252,10 @@ by rust officially."#,
"QuickInstall"
}

fn strategy(&self) -> Strategy {
Strategy::QuickInstall
}

fn is_third_party(&self) -> bool {
true
}
Expand Down
3 changes: 3 additions & 0 deletions crates/binstalk-types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
65 changes: 65 additions & 0 deletions crates/binstalk-types/src/cargo_toml_binstall.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use std::{borrow::Cow, collections::BTreeMap};

use serde::{Deserialize, Serialize};
use strum_macros::{EnumCount, VariantArray};

mod package_formats;
#[doc(inline)]
Expand All @@ -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`
Expand All @@ -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>,
}
Expand Down Expand Up @@ -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(),
}
}
Expand All @@ -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>,
}
Expand Down Expand Up @@ -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())
)
});
}
}
Loading
Loading