From ab3acf5830c423d666a5916c407f0604ad5909a5 Mon Sep 17 00:00:00 2001 From: jdx <216188+jdx@users.noreply.github.com> Date: Tue, 12 Dec 2023 14:54:04 -0600 Subject: [PATCH] uninstall: tweak behavior (#1144) Fixes #1115 --- README.md | 8 +-- completions/rtx.bash | 2 +- e2e/test_nodejs | 2 +- src/cli/uninstall.rs | 124 +++++++++++++++++++++--------------- src/file.rs | 16 ++++- src/plugins/mod.rs | 6 ++ src/runtime_symlinks.rs | 4 +- src/toolset/tool_version.rs | 30 ++++++++- 8 files changed, 128 insertions(+), 64 deletions(-) diff --git a/README.md b/README.md index 4ca3cb990..07561c16c 100644 --- a/README.md +++ b/README.md @@ -168,7 +168,7 @@ v20.0.0 - [`rtx sync node <--brew|--nvm|--nodenv>`](#rtx-sync-node---brew--nvm--nodenv) - [`rtx sync python --pyenv`](#rtx-sync-python---pyenv) - [`rtx trust [OPTIONS] [CONFIG_FILE]`](#rtx-trust-options-config_file) - - [`rtx uninstall [OPTIONS] ...`](#rtx-uninstall-options-toolversion) + - [`rtx uninstall [OPTIONS] [TOOL@VERSION]...`](#rtx-uninstall-options-toolversion) - [`rtx upgrade [OPTIONS] [TOOL@VERSION]...`](#rtx-upgrade-options-toolversion) - [`rtx use [OPTIONS] [TOOL@VERSION]...`](#rtx-use-options-toolversion) - [`rtx version`](#rtx-version) @@ -2630,15 +2630,15 @@ Examples: $ rtx trust ``` -### `rtx uninstall [OPTIONS] ...` +### `rtx uninstall [OPTIONS] [TOOL@VERSION]...` ```text Removes runtime versions -Usage: uninstall [OPTIONS] ... +Usage: uninstall [OPTIONS] [TOOL@VERSION]... Arguments: - ... + [TOOL@VERSION]... Tool(s) to remove Options: diff --git a/completions/rtx.bash b/completions/rtx.bash index c275b26a1..89744e55e 100644 --- a/completions/rtx.bash +++ b/completions/rtx.bash @@ -3397,7 +3397,7 @@ _rtx() { return 0 ;; rtx__uninstall) - opts="-a -n -j -q -r -v -y -h --all --dry-run --jobs --debug --log-level --trace --quiet --raw --verbose --yes --help ..." + opts="-a -n -j -q -r -v -y -h --all --dry-run --jobs --debug --log-level --trace --quiet --raw --verbose --yes --help [TOOL@VERSION]..." if [[ ${cur} == -* || ${COMP_CWORD} -eq 2 ]] ; then COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) return 0 diff --git a/e2e/test_nodejs b/e2e/test_nodejs index bca296c08..e861976aa 100755 --- a/e2e/test_nodejs +++ b/e2e/test_nodejs @@ -30,7 +30,7 @@ rtx plugin uninstall nodejs RTX_DISABLE_TOOLS=node assert_not_contains "rtx plugins --core" "node" export RTX_NODE_BUILD=0 -rtx uninstall node +rtx uninstall -a node rtx i node assert "rtx x -- node --version" "v20.0.0" # rtx uninstall node diff --git a/src/cli/uninstall.rs b/src/cli/uninstall.rs index b20ab9a04..387ebbc65 100644 --- a/src/cli/uninstall.rs +++ b/src/cli/uninstall.rs @@ -1,9 +1,13 @@ use console::style; use eyre::{Result, WrapErr}; +use itertools::Itertools; +use rayon::prelude::*; +use std::sync::Arc; use crate::cli::args::tool::{ToolArg, ToolArgParser}; use crate::config::Config; use crate::output::Output; +use crate::plugins::Plugin; use crate::toolset::{ToolVersion, ToolVersionRequest, ToolsetBuilder}; use crate::ui::multi_progress_report::MultiProgressReport; use crate::{runtime_symlinks, shims}; @@ -13,11 +17,11 @@ use crate::{runtime_symlinks, shims}; #[clap(verbatim_doc_comment, alias = "remove", alias = "rm", after_long_help = AFTER_LONG_HELP)] pub struct Uninstall { /// Tool(s) to remove - #[clap(required = true, value_name = "TOOL@VERSION", value_parser = ToolArgParser)] + #[clap(value_name = "TOOL@VERSION", value_parser = ToolArgParser, required_unless_present = "all")] tool: Vec, /// Delete all installed versions - #[clap(long, short = 'a')] + #[clap(long, short)] all: bool, /// Do not actually delete anything @@ -27,57 +31,18 @@ pub struct Uninstall { impl Uninstall { pub fn run(self, config: Config, _out: &mut Output) -> Result<()> { - let runtimes = ToolArg::double_tool_condition(&self.tool); - - let mut tool_versions = vec![]; - if self.all { - for runtime in runtimes { - let tool = config.get_or_create_plugin(&runtime.plugin); - let query = runtime.tvr.map(|tvr| tvr.version()).unwrap_or_default(); - let tvs = tool - .list_installed_versions()? - .into_iter() - .filter(|v| v.starts_with(&query)) - .map(|v| { - let tvr = ToolVersionRequest::new(tool.name().into(), &v); - let tv = ToolVersion::new(tool.clone(), tvr, Default::default(), v); - (tool.clone(), tv) - }) - .collect::>(); - if tvs.is_empty() { - warn!("no versions found for {}", style(&tool).cyan().for_stderr()); - } - tool_versions.extend(tvs); - } + let tool_versions = if self.tool.is_empty() && self.all { + self.get_all_tool_versions(&config)? } else { - tool_versions = runtimes - .into_iter() - .map(|a| { - let tool = config.get_or_create_plugin(&a.plugin); - let tvs = match a.tvr { - Some(tvr) => { - vec![tvr.resolve(&config, tool.clone(), Default::default(), false)?] - } - None => { - let ts = ToolsetBuilder::new().build(&config)?; - match ts.versions.get(&a.plugin) { - Some(tvl) => tvl.versions.clone(), - None => bail!( - "no versions found for {}", - style(&tool).cyan().for_stderr() - ), - } - } - }; - Ok(tvs - .into_iter() - .map(|tv| (tool.clone(), tv)) - .collect::>()) - }) - .collect::>>()? - .into_iter() - .flatten() - .collect::>(); + self.get_requested_tool_versions(&config)? + }; + let tool_versions = tool_versions + .into_iter() + .unique() + .sorted() + .collect::>(); + if !self.all && tool_versions.len() > 1 { + bail!("multiple tools specified, use --all to uninstall all versions"); } let mpr = MultiProgressReport::new(&config.settings); @@ -93,7 +58,11 @@ impl Uninstall { pr.error(err.to_string()); return Err(eyre!(err).wrap_err(format!("failed to uninstall {tv}"))); } - pr.finish_with_message("uninstalled"); + if self.dry_run { + pr.finish_with_message("uninstalled (dry-run)"); + } else { + pr.finish_with_message("uninstalled"); + } } let ts = ToolsetBuilder::new().build(&config)?; @@ -102,6 +71,55 @@ impl Uninstall { Ok(()) } + + fn get_all_tool_versions( + &self, + config: &Config, + ) -> Result, ToolVersion)>> { + let ts = ToolsetBuilder::new().build(config)?; + let tool_versions = ts + .list_installed_versions(config)? + .into_iter() + .collect::>(); + Ok(tool_versions) + } + fn get_requested_tool_versions( + &self, + config: &Config, + ) -> Result, ToolVersion)>> { + let runtimes = ToolArg::double_tool_condition(&self.tool); + let tool_versions = runtimes + .into_par_iter() + .map(|a| { + let tool = config.get_or_create_plugin(&a.plugin); + let query = a.tvr.as_ref().map(|tvr| tvr.version()).unwrap_or_default(); + let mut tvs = tool + .list_installed_versions()? + .into_iter() + .filter(|v| v.starts_with(&query)) + .map(|v| { + let tvr = ToolVersionRequest::new(tool.name().into(), &v); + let tv = ToolVersion::new(tool.clone(), tvr, Default::default(), v); + (tool.clone(), tv) + }) + .collect::>(); + if let Some(tvr) = &a.tvr { + tvs.push(( + tool.clone(), + tvr.resolve(config, tool.clone(), Default::default(), false)?, + )); + } + if tvs.is_empty() { + warn!("no versions found for {}", style(&tool).cyan().for_stderr()); + } + Ok(tvs) + }) + .collect::>>()? + .into_iter() + .flatten() + .collect::>(); + Ok(tool_versions) + } } static AFTER_LONG_HELP: &str = color_print::cstr!( diff --git a/src/file.rs b/src/file.rs index fa114ebe3..cb76ea442 100644 --- a/src/file.rs +++ b/src/file.rs @@ -36,8 +36,14 @@ pub fn remove_file>(path: P) -> Result<()> { pub fn remove_dir>(path: P) -> Result<()> { let path = path.as_ref(); - trace!("rmdir {}", display_path(path)); - fs::remove_dir(path).wrap_err_with(|| format!("failed rmdir: {}", display_path(path))) + (|| -> Result<()> { + if path.exists() && is_empty_dir(path)? { + trace!("rmdir {}", display_path(path)); + fs::remove_dir(path)?; + } + Ok(()) + })() + .wrap_err_with(|| format!("failed to remove_dir: {}", display_path(path))) } pub fn remove_all_with_warning>(path: P) -> Result<()> { @@ -207,6 +213,12 @@ pub fn make_executable(path: &Path) -> Result<()> { Ok(()) } +fn is_empty_dir(path: &Path) -> Result { + path.read_dir() + .map(|mut i| i.next().is_none()) + .wrap_err_with(|| format!("failed to read_dir: {}", display_path(path))) +} + pub struct FindUp { current_dir: PathBuf, current_dir_filenames: Vec, diff --git a/src/plugins/mod.rs b/src/plugins/mod.rs index 2b5c3cb4c..19218e6bd 100644 --- a/src/plugins/mod.rs +++ b/src/plugins/mod.rs @@ -1,6 +1,7 @@ use std::collections::{BTreeMap, HashMap}; use std::fmt::{Debug, Display}; use std::fs::File; +use std::hash::Hash; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -409,6 +410,11 @@ impl PartialEq for dyn Plugin { self.get_type() == other.get_type() && self.name() == other.name() } } +impl Hash for dyn Plugin { + fn hash(&self, state: &mut H) { + self.name().hash(state) + } +} impl PartialOrd for dyn Plugin { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) diff --git a/src/runtime_symlinks.rs b/src/runtime_symlinks.rs index 19e108c12..b52f946d3 100644 --- a/src/runtime_symlinks.rs +++ b/src/runtime_symlinks.rs @@ -30,8 +30,8 @@ pub fn rebuild(config: &Config) -> Result<()> { make_symlink(&to, &from)?; } remove_missing_symlinks(plugin.clone())?; - // attempt to remove the installs dir (will fail if not empty) - let _ = file::remove_dir(&installs_dir); + // remove install dir if empty + file::remove_dir(&installs_dir)?; } Ok(()) } diff --git a/src/toolset/tool_version.rs b/src/toolset/tool_version.rs index ee51a7bfc..b54521136 100644 --- a/src/toolset/tool_version.rs +++ b/src/toolset/tool_version.rs @@ -1,5 +1,7 @@ +use std::cmp::Ordering; use std::fmt::{Display, Formatter}; use std::fs; +use std::hash::{Hash, Hasher}; use std::path::PathBuf; use std::sync::Arc; @@ -13,7 +15,7 @@ use crate::plugins::{Plugin, PluginName}; use crate::toolset::{ToolVersionOptions, ToolVersionRequest}; /// represents a single version of a tool for a particular plugin -#[derive(Debug, Clone, PartialEq, Eq, Hash)] +#[derive(Debug, Clone)] pub struct ToolVersion { pub request: ToolVersionRequest, pub plugin_name: PluginName, @@ -244,6 +246,32 @@ impl Display for ToolVersion { } } +impl PartialEq for ToolVersion { + fn eq(&self, other: &Self) -> bool { + self.plugin_name == other.plugin_name && self.version == other.version + } +} +impl Eq for ToolVersion {} +impl PartialOrd for ToolVersion { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} +impl Ord for ToolVersion { + fn cmp(&self, other: &Self) -> Ordering { + match self.plugin_name.cmp(&other.plugin_name) { + Ordering::Equal => self.version.cmp(&other.version), + o => o, + } + } +} +impl Hash for ToolVersion { + fn hash(&self, state: &mut H) { + self.plugin_name.hash(state); + self.version.hash(state); + } +} + /// subtracts sub from orig and removes suffix /// e.g. version_sub("18.2.3", "2") -> "16" /// e.g. version_sub("18.2.3", "0.1") -> "18.1"