From 49444107c2a2e4e4e9f175cc00a641927b47b1e1 Mon Sep 17 00:00:00 2001 From: rami3l Date: Thu, 21 Dec 2023 12:42:46 +0800 Subject: [PATCH 1/2] refactor(cli): rewrite `rustup-init` with `clap_derive` --- Cargo.lock | 19 ++ Cargo.toml | 2 +- rustup-init.sh | 12 +- src/cli/setup_mode.rs | 209 ++++++++---------- src/toolchain/names.rs | 2 +- .../rustup-init_help_flag_stdout.toml | 12 +- .../rustup-init_sh_help_flag_stdout.toml | 12 +- 7 files changed, 128 insertions(+), 140 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d8d5f5ceed..21773044fa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -321,6 +321,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "90bc066a67923782aa8515dbaea16946c5bcc5addbd668bb80af688e53e548a0" dependencies = [ "clap_builder", + "clap_derive", ] [[package]] @@ -345,6 +346,18 @@ dependencies = [ "clap", ] +[[package]] +name = "clap_derive" +version = "4.5.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "528131438037fd55894f62d6e9f068b8f45ac57ffa77517819645d10aed04f64" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "clap_lex" version = "0.7.0" @@ -845,6 +858,12 @@ version = "0.14.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" +[[package]] +name = "heck" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea" + [[package]] name = "hermit-abi" version = "0.3.9" diff --git a/Cargo.toml b/Cargo.toml index 84ff2ef2e9..7875d19573 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -47,7 +47,7 @@ test = ["dep:walkdir"] anyhow.workspace = true cfg-if = "1.0" chrono = { version = "0.4", default-features = false, features = ["std"] } -clap = { version = "4", features = ["wrap_help"] } +clap = { version = "4", features = ["derive", "wrap_help"] } clap_complete = "4" download = { path = "download", default-features = false } effective-limits = "0.5.5" diff --git a/rustup-init.sh b/rustup-init.sh index c49b6ab454..d65834ac7f 100755 --- a/rustup-init.sh +++ b/rustup-init.sh @@ -43,16 +43,16 @@ Options: -q, --quiet Disable progress output -y - Disable confirmation prompt. - --default-host + Disable confirmation prompt + --default-host Choose a default host triple - --default-toolchain + --default-toolchain Choose a default toolchain to install. Use 'none' to not install any toolchains at all - --profile + --profile [default: default] [possible values: minimal, default, complete] - -c, --component ... + -c, --components ... Component name to also install - -t, --target ... + -t, --targets ... Target name to also install --no-update-default-toolchain Don't update any existing default toolchain after install diff --git a/src/cli/setup_mode.rs b/src/cli/setup_mode.rs index cada019e5a..4dcbad940a 100644 --- a/src/cli/setup_mode.rs +++ b/src/cli/setup_mode.rs @@ -1,153 +1,122 @@ use anyhow::Result; -use clap::{builder::PossibleValuesParser, value_parser, Arg, ArgAction, Command}; +use clap::{builder::PossibleValuesParser, Parser}; use crate::{ cli::{ common, self_update::{self, InstallOpts}, }, - currentprocess::{argsource::ArgSource, filesource::StdoutSource}, + currentprocess::filesource::StdoutSource, dist::dist::Profile, process, toolchain::names::MaybeOfficialToolchainName, utils::utils, }; +/// The installer for rustup +#[derive(Debug, Parser)] +#[command( + name = "rustup-init", + bin_name = "rustup-init[EXE]", + version = common::version(), + before_help = format!("rustup-init {}", common::version()), +)] +struct RustupInit { + /// Enable verbose output + #[arg(short, long)] + verbose: bool, + + /// Disable progress output + #[arg(short, long)] + quiet: bool, + + /// Disable confirmation prompt + #[arg(short = 'y')] + no_prompt: bool, + + /// Choose a default host triple + #[arg(long)] + default_host: Option, + + /// Choose a default toolchain to install. Use 'none' to not install any toolchains at all + #[arg(long)] + default_toolchain: Option, + + #[arg( + long, + value_parser = PossibleValuesParser::new(Profile::names()), + default_value = Profile::default_name(), + )] + profile: String, + + /// Component name to also install + #[arg(short, long, value_delimiter = ',', num_args = 1..)] + components: Vec, + + /// Target name to also install + #[arg(short, long, value_delimiter = ',', num_args = 1..)] + targets: Vec, + + /// Don't update any existing default toolchain after install + #[arg(long)] + no_update_default_toolchain: bool, + + /// Don't configure the PATH environment variable + #[arg(long)] + no_modify_path: bool, + + /// Secret command used during self-update. Not for users + #[arg(long, hide = true)] + self_replace: bool, + + /// Internal testament dump used during CI. Not for users + #[arg(long, hide = true)] + dump_testament: bool, +} + #[cfg_attr(feature = "otel", tracing::instrument)] pub fn main() -> Result { use clap::error::ErrorKind; - let args: Vec<_> = process().args().collect(); - let arg1 = args.get(1).map(|a| &**a); + let RustupInit { + verbose, + quiet, + no_prompt, + default_host, + default_toolchain, + profile, + components, + targets, + no_update_default_toolchain, + no_modify_path, + self_replace, + dump_testament, + } = match RustupInit::try_parse() { + Ok(args) => args, + Err(e) if [ErrorKind::DisplayHelp, ErrorKind::DisplayVersion].contains(&e.kind()) => { + write!(process().stdout().lock(), "{e}")?; + return Ok(utils::ExitCode(0)); + } + Err(e) => return Err(e.into()), + }; - // Secret command used during self-update. Not for users. - if arg1 == Some("--self-replace") { + if self_replace { return self_update::self_replace(); } - // Internal testament dump used during CI. Not for users. - if arg1 == Some("--dump-testament") { + if dump_testament { common::dump_testament()?; return Ok(utils::ExitCode(0)); } - // NOTICE: If you change anything here, please make the same changes in rustup-init.sh - let cli = Command::new("rustup-init") - .version(common::version()) - .before_help(format!("rustup-init {}", common::version())) - .about("The installer for rustup") - .arg( - Arg::new("verbose") - .short('v') - .long("verbose") - .help("Enable verbose output") - .action(ArgAction::SetTrue), - ) - .arg( - Arg::new("quiet") - .conflicts_with("verbose") - .short('q') - .long("quiet") - .help("Disable progress output") - .action(ArgAction::SetTrue), - ) - .arg( - Arg::new("no-prompt") - .short('y') - .help("Disable confirmation prompt.") - .action(ArgAction::SetTrue), - ) - .arg( - Arg::new("default-host") - .long("default-host") - .num_args(1) - .help("Choose a default host triple"), - ) - .arg( - Arg::new("default-toolchain") - .long("default-toolchain") - .num_args(1) - .help("Choose a default toolchain to install. Use 'none' to not install any toolchains at all") - .value_parser(value_parser!(MaybeOfficialToolchainName)) - ) - .arg( - Arg::new("profile") - .long("profile") - .value_parser(PossibleValuesParser::new(Profile::names())) - .default_value(Profile::default_name()), - ) - .arg( - Arg::new("components") - .help("Component name to also install") - .long("component") - .short('c') - .num_args(1..) - .use_value_delimiter(true) - .action(ArgAction::Append), - ) - .arg( - Arg::new("targets") - .help("Target name to also install") - .long("target") - .short('t') - .num_args(1..) - .use_value_delimiter(true) - .action(ArgAction::Append), - ) - .arg( - Arg::new("no-update-default-toolchain") - .long("no-update-default-toolchain") - .help("Don't update any existing default toolchain after install") - .action(ArgAction::SetTrue), - ) - .arg( - Arg::new("no-modify-path") - .long("no-modify-path") - .help("Don't configure the PATH environment variable") - .action(ArgAction::SetTrue), - ); - - let matches = match cli.try_get_matches_from(process().args_os()) { - Ok(matches) => matches, - Err(e) if [ErrorKind::DisplayHelp, ErrorKind::DisplayVersion].contains(&e.kind()) => { - write!(process().stdout().lock(), "{e}")?; - return Ok(utils::ExitCode(0)); - } - Err(e) => return Err(e.into()), - }; - let no_prompt = matches.get_flag("no-prompt"); - let verbose = matches.get_flag("verbose"); - let quiet = matches.get_flag("quiet"); - let default_host = matches - .get_one::("default-host") - .map(ToOwned::to_owned); - let default_toolchain = matches - .get_one::("default-toolchain") - .map(ToOwned::to_owned); - let profile = matches - .get_one::("profile") - .expect("Unreachable: Clap should supply a default"); - let no_modify_path = matches.get_flag("no-modify-path"); - let no_update_toolchain = matches.get_flag("no-update-default-toolchain"); - - let components: Vec<_> = matches - .get_many::("components") - .map(|v| v.map(|s| &**s).collect()) - .unwrap_or_else(Vec::new); - - let targets: Vec<_> = matches - .get_many::("targets") - .map(|v| v.map(|s| &**s).collect()) - .unwrap_or_else(Vec::new); - let opts = InstallOpts { default_host_triple: default_host, default_toolchain, profile: profile.to_owned(), no_modify_path, - no_update_toolchain, - components: &components, - targets: &targets, + no_update_toolchain: no_update_default_toolchain, + components: &components.iter().map(|s| &**s).collect::>(), + targets: &targets.iter().map(|s| &**s).collect::>(), }; if profile == "complete" { diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index cbae1669ed..f472ac1db8 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -240,7 +240,7 @@ pub(crate) fn maybe_resolvable_toolchainame_parser( /// ResolvableToolchainName + none, for overriding default-has-a-value /// situations in the CLI with an official toolchain name or none -#[derive(Clone)] +#[derive(Debug, Clone)] pub(crate) enum MaybeOfficialToolchainName { None, Some(PartialToolchainDesc), diff --git a/tests/suite/cli-ui/rustup-init/rustup-init_help_flag_stdout.toml b/tests/suite/cli-ui/rustup-init/rustup-init_help_flag_stdout.toml index 6439bc795f..2331f46f81 100644 --- a/tests/suite/cli-ui/rustup-init/rustup-init_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup-init/rustup-init_help_flag_stdout.toml @@ -14,16 +14,16 @@ Options: -q, --quiet Disable progress output -y - Disable confirmation prompt. - --default-host + Disable confirmation prompt + --default-host Choose a default host triple - --default-toolchain + --default-toolchain Choose a default toolchain to install. Use 'none' to not install any toolchains at all - --profile + --profile [default: default] [possible values: minimal, default, complete] - -c, --component ... + -c, --components ... Component name to also install - -t, --target ... + -t, --targets ... Target name to also install --no-update-default-toolchain Don't update any existing default toolchain after install diff --git a/tests/suite/cli-ui/rustup-init/rustup-init_sh_help_flag_stdout.toml b/tests/suite/cli-ui/rustup-init/rustup-init_sh_help_flag_stdout.toml index bddf924bb5..c6c74e0438 100644 --- a/tests/suite/cli-ui/rustup-init/rustup-init_sh_help_flag_stdout.toml +++ b/tests/suite/cli-ui/rustup-init/rustup-init_sh_help_flag_stdout.toml @@ -14,16 +14,16 @@ Options: -q, --quiet Disable progress output -y - Disable confirmation prompt. - --default-host + Disable confirmation prompt + --default-host Choose a default host triple - --default-toolchain + --default-toolchain Choose a default toolchain to install. Use 'none' to not install any toolchains at all - --profile + --profile [default: default] [possible values: minimal, default, complete] - -c, --component ... + -c, --components ... Component name to also install - -t, --target ... + -t, --targets ... Target name to also install --no-update-default-toolchain Don't update any existing default toolchain after install From 75c0fffd2dc0ef221a5f5f3ed594c0bae8cd35f5 Mon Sep 17 00:00:00 2001 From: rami3l Date: Fri, 19 Jan 2024 21:18:41 +0800 Subject: [PATCH 2/2] refactor(cli): reorder `if` statement in `cli::setup_mode::main()` --- src/cli/setup_mode.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cli/setup_mode.rs b/src/cli/setup_mode.rs index 4dcbad940a..fea154f3f9 100644 --- a/src/cli/setup_mode.rs +++ b/src/cli/setup_mode.rs @@ -109,19 +109,19 @@ pub fn main() -> Result { return Ok(utils::ExitCode(0)); } + if &profile == "complete" { + warn!("{}", common::WARN_COMPLETE_PROFILE); + } + let opts = InstallOpts { default_host_triple: default_host, default_toolchain, - profile: profile.to_owned(), + profile, no_modify_path, no_update_toolchain: no_update_default_toolchain, components: &components.iter().map(|s| &**s).collect::>(), targets: &targets.iter().map(|s| &**s).collect::>(), }; - if profile == "complete" { - warn!("{}", common::WARN_COMPLETE_PROFILE); - } - self_update::install(no_prompt, verbose, quiet, opts) }