From 19445e42b6e2b3ae9a2c900561129f3e78daa5f5 Mon Sep 17 00:00:00 2001 From: Camelid Date: Tue, 27 Oct 2020 13:20:46 -0700 Subject: [PATCH 1/9] Allow selecting toolchains to uninstall with regex $ rustup toolchain list stable-x86_64-apple-darwin (default) nightly-2020-04-18-x86_64-apple-darwin nightly-2020-04-20-x86_64-apple-darwin nightly-x86_64-apple-darwin $ rustup toolchain uninstall --regex 'nightly-*' info: uninstalling toolchain 'nightly-2020-04-18-x86_64-apple-darwin' info: toolchain 'nightly-2020-04-18-x86_64-apple-darwin' uninstalled info: uninstalling toolchain 'nightly-2020-04-20-x86_64-apple-darwin' info: toolchain 'nightly-2020-04-20-x86_64-apple-darwin' uninstalled info: uninstalling toolchain 'nightly-x86_64-apple-darwin' info: toolchain 'nightly-x86_64-apple-darwin' uninstalled $ rustup toolchain list stable-x86_64-apple-darwin (default) --- src/cli/rustup_mode.rs | 30 +++++++++++++++++++++++++++--- src/config.rs | 32 ++++++++++++++++++++++---------- 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 3e2b7bb33d..0bdba5540b 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -404,6 +404,12 @@ pub fn cli() -> App<'static, 'static> { SubCommand::with_name("uninstall") .about("Uninstall a toolchain") .alias("remove") + .arg( + Arg::with_name("regex") + .help("Use a regular expression to select the toolchains to uninstall") + .long("regex") + .takes_value(false), + ) .arg( Arg::with_name("toolchain") .help(TOOLCHAIN_ARG_HELP) @@ -1312,10 +1318,28 @@ fn toolchain_link(cfg: &Cfg, m: &ArgMatches<'_>) -> Result { } fn toolchain_remove(cfg: &mut Cfg, m: &ArgMatches<'_>) -> Result { - for toolchain in m.values_of("toolchain").unwrap() { - let toolchain = cfg.get_toolchain(toolchain, false)?; - toolchain.remove()?; + if m.is_present("regex") { + assert!( + m.values_of("toolchain").unwrap().len() == 1, + "exactly one regex filter must be supplied" + ); + + let regex = regex::Regex::from_str(m.values_of("toolchain").unwrap().next().unwrap()) + .expect("invalid regex"); + + for toolchain in cfg.get_toolchains_from_regex(regex)? { + toolchain.remove()?; + } + } else { + for toolchain in m.values_of("toolchain").unwrap() { + if m.is_present("regex") { + } else { + } + let toolchain = cfg.get_toolchain(toolchain, false)?; + toolchain.remove()?; + } } + Ok(utils::ExitCode(0)) } diff --git a/src/config.rs b/src/config.rs index 2d26d7bdcd..5662bf5ae6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,6 +1,7 @@ use std::borrow::Cow; use std::fmt::{self, Display}; use std::io; +use std::iter; use std::path::{Path, PathBuf}; use std::process::Command; use std::str::FromStr; @@ -362,6 +363,14 @@ impl Cfg { Toolchain::from(self, name) } + pub fn get_toolchains_from_regex(&self, regex: regex::Regex) -> Result>> { + Ok(self + .list_toolchains_iter()? + .filter(|toolchain| regex.is_match(toolchain)) + .map(|toolchain| Toolchain::from(self, &toolchain).unwrap()) + .collect()) + } + pub fn verify_toolchain(&self, name: &str) -> Result> { let toolchain = self.get_toolchain(name, false)?; toolchain.verify()?; @@ -694,18 +703,21 @@ impl Cfg { } pub fn list_toolchains(&self) -> Result> { - if utils::is_directory(&self.toolchains_dir) { - let mut toolchains: Vec<_> = utils::read_dir("toolchains", &self.toolchains_dir)? - .filter_map(io::Result::ok) - .filter(|e| e.file_type().map(|f| !f.is_file()).unwrap_or(false)) - .filter_map(|e| e.file_name().into_string().ok()) - .collect(); - - utils::toolchain_sort(&mut toolchains); + let mut toolchains: Vec<_> = self.list_toolchains_iter()?.collect(); + utils::toolchain_sort(&mut toolchains); + Ok(toolchains) + } - Ok(toolchains) + fn list_toolchains_iter(&self) -> Result>> { + if utils::is_directory(&self.toolchains_dir) { + Ok(Box::new( + utils::read_dir("toolchains", &self.toolchains_dir)? + .filter_map(io::Result::ok) + .filter(|e| e.file_type().map(|f| !f.is_file()).unwrap_or(false)) + .filter_map(|e| e.file_name().into_string().ok()), + )) } else { - Ok(Vec::new()) + Ok(Box::new(iter::empty())) } } From 7962dfe5de5078e4f575b93362ee0e2a3ece2be3 Mon Sep 17 00:00:00 2001 From: Camelid Date: Tue, 27 Oct 2020 13:25:27 -0700 Subject: [PATCH 2/9] Remove mistakenly-committed code --- src/cli/rustup_mode.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 0bdba5540b..195d6482ee 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -1332,9 +1332,6 @@ fn toolchain_remove(cfg: &mut Cfg, m: &ArgMatches<'_>) -> Result Date: Tue, 27 Oct 2020 13:43:27 -0700 Subject: [PATCH 3/9] Add `use regex::Regex` instead of using full path --- src/cli/rustup_mode.rs | 3 ++- src/config.rs | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 195d6482ee..f5f17bbe87 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -7,6 +7,7 @@ use std::process::Command; use std::str::FromStr; use clap::{App, AppSettings, Arg, ArgGroup, ArgMatches, Shell, SubCommand}; +use regex::Regex; use super::common; use super::errors::*; @@ -1324,7 +1325,7 @@ fn toolchain_remove(cfg: &mut Cfg, m: &ArgMatches<'_>) -> Result Result>> { + pub fn get_toolchains_from_regex(&self, regex: Regex) -> Result>> { Ok(self .list_toolchains_iter()? .filter(|toolchain| regex.is_match(toolchain)) From 7ce4203283b7d1874d0ea81fc5fcecf12e5bc56a Mon Sep 17 00:00:00 2001 From: Camelid Date: Wed, 28 Oct 2020 13:49:53 -0700 Subject: [PATCH 4/9] Return `impl Iterator` instead of `Vec` --- src/config.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/config.rs b/src/config.rs index f08a9aac19..126ccb8de6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -364,12 +364,14 @@ impl Cfg { Toolchain::from(self, name) } - pub fn get_toolchains_from_regex(&self, regex: Regex) -> Result>> { + pub fn get_toolchains_from_regex( + &self, + regex: Regex, + ) -> Result>> { Ok(self .list_toolchains_iter()? - .filter(|toolchain| regex.is_match(toolchain)) - .map(|toolchain| Toolchain::from(self, &toolchain).unwrap()) - .collect()) + .filter(move |toolchain| regex.is_match(toolchain)) + .map(move |toolchain| Toolchain::from(self, &toolchain).unwrap())) } pub fn verify_toolchain(&self, name: &str) -> Result> { From 8c94393d5401d1e6024117a8d801658f36eae353 Mon Sep 17 00:00:00 2001 From: Camelid Date: Wed, 28 Oct 2020 14:17:29 -0700 Subject: [PATCH 5/9] Accept glob pattern instead of regex --- Cargo.lock | 7 +++++++ Cargo.toml | 1 + src/cli/errors.rs | 1 + src/cli/rustup_mode.rs | 26 +++++++++++--------------- src/config.rs | 8 ++++---- 5 files changed, 24 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2694e081c6..0932ee4cd2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -859,6 +859,12 @@ dependencies = [ "syn", ] +[[package]] +name = "glob" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b919933a397b79c37e33b77bb2aa3dc8eb6e165ad809e58ff75bc7db2e34574" + [[package]] name = "h2" version = "0.2.6" @@ -1815,6 +1821,7 @@ dependencies = [ "error-chain", "flate2", "git-testament", + "glob", "home", "lazy_static", "libc", diff --git a/Cargo.toml b/Cargo.toml index 1a453c1254..5cb4ace1cb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,6 +29,7 @@ download = {path = "download"} error-chain = "0.12" flate2 = "1" git-testament = "0.1.4" +glob = "0.3" home = {git = "https://github.com/rbtcollins/home", rev = "a243ee2fbee6022c57d56f5aa79aefe194eabe53"} lazy_static = "1" libc = "0.2" diff --git a/src/cli/errors.rs b/src/cli/errors.rs index 7b5a6ba849..3f9643acc2 100644 --- a/src/cli/errors.rs +++ b/src/cli/errors.rs @@ -24,6 +24,7 @@ error_chain! { Temp(temp::Error); Io(io::Error); Term(term::Error); + Glob(glob::PatternError); } errors { diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index f5f17bbe87..f51021e812 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -7,7 +7,7 @@ use std::process::Command; use std::str::FromStr; use clap::{App, AppSettings, Arg, ArgGroup, ArgMatches, Shell, SubCommand}; -use regex::Regex; +use glob::Pattern; use super::common; use super::errors::*; @@ -406,9 +406,10 @@ pub fn cli() -> App<'static, 'static> { .about("Uninstall a toolchain") .alias("remove") .arg( - Arg::with_name("regex") - .help("Use a regular expression to select the toolchains to uninstall") - .long("regex") + Arg::with_name("pattern") + .help("Treat arguments as glob patterns") + .short("p") + .long("pattern") .takes_value(false), ) .arg( @@ -1319,17 +1320,12 @@ fn toolchain_link(cfg: &Cfg, m: &ArgMatches<'_>) -> Result { } fn toolchain_remove(cfg: &mut Cfg, m: &ArgMatches<'_>) -> Result { - if m.is_present("regex") { - assert!( - m.values_of("toolchain").unwrap().len() == 1, - "exactly one regex filter must be supplied" - ); - - let regex = Regex::from_str(m.values_of("toolchain").unwrap().next().unwrap()) - .expect("invalid regex"); - - for toolchain in cfg.get_toolchains_from_regex(regex)? { - toolchain.remove()?; + if m.is_present("pattern") { + for pattern_str in m.values_of("toolchain").unwrap() { + let pattern = Pattern::new(pattern_str)?; + for toolchain in cfg.get_toolchains_from_glob(pattern)? { + toolchain.remove()?; + } } } else { for toolchain in m.values_of("toolchain").unwrap() { diff --git a/src/config.rs b/src/config.rs index 126ccb8de6..5cd01d2d7a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -7,8 +7,8 @@ use std::process::Command; use std::str::FromStr; use std::sync::Arc; +use glob::Pattern; use pgp::{Deserializable, SignedPublicKey}; -use regex::Regex; use serde::Deserialize; use crate::dist::download::DownloadCfg; @@ -364,13 +364,13 @@ impl Cfg { Toolchain::from(self, name) } - pub fn get_toolchains_from_regex( + pub fn get_toolchains_from_glob( &self, - regex: Regex, + pattern: Pattern, ) -> Result>> { Ok(self .list_toolchains_iter()? - .filter(move |toolchain| regex.is_match(toolchain)) + .filter(move |toolchain| pattern.matches(toolchain)) .map(move |toolchain| Toolchain::from(self, &toolchain).unwrap())) } From 75a35b0c402556a5dae820a0168949e3a8ce490d Mon Sep 17 00:00:00 2001 From: Camelid Date: Sat, 31 Oct 2020 14:22:12 -0700 Subject: [PATCH 6/9] Tell user when pattern did not match any toolchains --- src/cli/rustup_mode.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index f51021e812..502121c43b 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -1322,8 +1322,15 @@ fn toolchain_link(cfg: &Cfg, m: &ArgMatches<'_>) -> Result { fn toolchain_remove(cfg: &mut Cfg, m: &ArgMatches<'_>) -> Result { if m.is_present("pattern") { for pattern_str in m.values_of("toolchain").unwrap() { - let pattern = Pattern::new(pattern_str)?; - for toolchain in cfg.get_toolchains_from_glob(pattern)? { + let pattern = Pattern::new(&pattern_str)?; + + let mut toolchains = cfg.get_toolchains_from_glob(pattern)?.peekable(); + if toolchains.peek().is_none() { + info!("no toolchains matched pattern '{}'", pattern_str); + return Ok(utils::ExitCode(0)); + } + + for toolchain in toolchains { toolchain.remove()?; } } From 693632cc6a28ce8d77fa728efcffd7deedab4401 Mon Sep 17 00:00:00 2001 From: Camelid Date: Sun, 1 Nov 2020 13:42:05 -0800 Subject: [PATCH 7/9] Default to treating toolchain as a pattern First treat the toolchain as a pattern; if it doesn't match anything, then treat it as a partial toolchain specifier. --- src/cli/rustup_mode.rs | 27 +++++++-------------------- tests/mock/clitools.rs | 4 ++-- 2 files changed, 9 insertions(+), 22 deletions(-) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 502121c43b..0ba64b322d 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -405,13 +405,6 @@ pub fn cli() -> App<'static, 'static> { SubCommand::with_name("uninstall") .about("Uninstall a toolchain") .alias("remove") - .arg( - Arg::with_name("pattern") - .help("Treat arguments as glob patterns") - .short("p") - .long("pattern") - .takes_value(false), - ) .arg( Arg::with_name("toolchain") .help(TOOLCHAIN_ARG_HELP) @@ -1320,23 +1313,17 @@ fn toolchain_link(cfg: &Cfg, m: &ArgMatches<'_>) -> Result { } fn toolchain_remove(cfg: &mut Cfg, m: &ArgMatches<'_>) -> Result { - if m.is_present("pattern") { - for pattern_str in m.values_of("toolchain").unwrap() { - let pattern = Pattern::new(&pattern_str)?; - - let mut toolchains = cfg.get_toolchains_from_glob(pattern)?.peekable(); - if toolchains.peek().is_none() { - info!("no toolchains matched pattern '{}'", pattern_str); - return Ok(utils::ExitCode(0)); - } + for pattern_str in m.values_of("toolchain").unwrap() { + let pattern = Pattern::new(&pattern_str)?; + let mut toolchains = cfg.get_toolchains_from_glob(pattern)?.peekable(); + + if toolchains.peek().is_some() { for toolchain in toolchains { toolchain.remove()?; } - } - } else { - for toolchain in m.values_of("toolchain").unwrap() { - let toolchain = cfg.get_toolchain(toolchain, false)?; + } else { + let toolchain = cfg.get_toolchain(pattern_str, false)?; toolchain.remove()?; } } diff --git a/tests/mock/clitools.rs b/tests/mock/clitools.rs index 820bad8c2e..ff25155e03 100644 --- a/tests/mock/clitools.rs +++ b/tests/mock/clitools.rs @@ -99,9 +99,9 @@ pub fn setup(s: Scenario, f: &dyn Fn(&mut Config)) { env::remove_var("RUSTUP_TOOLCHAIN"); env::remove_var("SHELL"); env::remove_var("ZDOTDIR"); - // clap does it's own terminal colour probing, and that isn't + // clap does its own terminal colour probing, and that isn't // trait-controllable, but it does honour the terminal. To avoid testing - // claps code, lie about whatever terminal this process was started under. + // clap's code, lie about whatever terminal this process was started under. env::set_var("TERM", "dumb"); match env::var("RUSTUP_BACKTRACE") { From 63bc755fe14144b6a29fd62e0e9012ccf8db67c2 Mon Sep 17 00:00:00 2001 From: Camelid Date: Sun, 1 Nov 2020 13:53:30 -0800 Subject: [PATCH 8/9] Add comments --- src/cli/rustup_mode.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cli/rustup_mode.rs b/src/cli/rustup_mode.rs index 0ba64b322d..19fe26e59f 100644 --- a/src/cli/rustup_mode.rs +++ b/src/cli/rustup_mode.rs @@ -1319,10 +1319,12 @@ fn toolchain_remove(cfg: &mut Cfg, m: &ArgMatches<'_>) -> Result Date: Sun, 1 Nov 2020 14:13:43 -0800 Subject: [PATCH 9/9] Add test --- tests/cli-rustup.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/cli-rustup.rs b/tests/cli-rustup.rs index 782948a560..108a4c5423 100644 --- a/tests/cli-rustup.rs +++ b/tests/cli-rustup.rs @@ -1224,6 +1224,21 @@ fn toolchain_uninstall_is_like_uninstall() { }); } +#[test] +fn toolchain_uninstall_pattern() { + setup(&|config| { + expect_ok(config, &["rustup", "uninstall", "stable-*"]); + expect_ok(config, &["rustup", "uninstall", "nightly-*"]); + let mut cmd = clitools::cmd(config, "rustup", &["show"]); + clitools::env(config, &mut cmd); + let out = cmd.output().unwrap(); + assert!(out.status.success()); + let stdout = String::from_utf8(out.stdout).unwrap(); + assert!(!stdout.contains(for_host!("'stable-{}'"))); + assert!(!stdout.contains(for_host!("'nightly-2015-01-01-{}'"))); + }); +} + #[test] fn toolchain_update_is_like_update_except_that_bare_install_is_an_error() { setup(&|config| {