uninstall: tweak behavior (#1144)

Fixes #1115

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] <TOOL@VERSION>...`](#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] <TOOL@VERSION>...`
+### `rtx uninstall [OPTIONS] [TOOL@VERSION]...`
 
 ```text
 Removes runtime versions
 
-Usage: uninstall [OPTIONS] <TOOL@VERSION>...
+Usage: uninstall [OPTIONS] [TOOL@VERSION]...
 
 Arguments:
-  <TOOL@VERSION>...
+  [TOOL@VERSION]...
           Tool(s) to remove
 
 Options:

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 <TOOL@VERSION>..."
+            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

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

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<ToolArg>,
 
     /// 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::<Vec<_>>();
-                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::<Vec<_>>())
-                })
-                .collect::<Result<Vec<_>>>()?
-                .into_iter()
-                .flatten()
-                .collect::<Vec<_>>();
+            self.get_requested_tool_versions(&config)?
+        };
+        let tool_versions = tool_versions
+            .into_iter()
+            .unique()
+            .sorted()
+            .collect::<Vec<_>>();
+        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<Vec<(Arc<dyn Plugin>, ToolVersion)>> {
+        let ts = ToolsetBuilder::new().build(config)?;
+        let tool_versions = ts
+            .list_installed_versions(config)?
+            .into_iter()
+            .collect::<Vec<_>>();
+        Ok(tool_versions)
+    }
+    fn get_requested_tool_versions(
+        &self,
+        config: &Config,
+    ) -> Result<Vec<(Arc<dyn Plugin>, 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::<Vec<_>>();
+                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::<Result<Vec<_>>>()?
+            .into_iter()
+            .flatten()
+            .collect::<Vec<_>>();
+        Ok(tool_versions)
+    }
 }
 
 static AFTER_LONG_HELP: &str = color_print::cstr!(

src/file.rs

@@ -36,8 +36,14 @@ pub fn remove_file<P: AsRef<Path>>(path: P) -> Result<()> {
 
 pub fn remove_dir<P: AsRef<Path>>(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<P: AsRef<Path>>(path: P) -> Result<()> {
@@ -207,6 +213,12 @@ pub fn make_executable(path: &Path) -> Result<()> {
     Ok(())
 }
 
+fn is_empty_dir(path: &Path) -> Result<bool> {
+    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<String>,

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<H: std::hash::Hasher>(&self, state: &mut H) {
+        self.name().hash(state)
+    }
+}
 impl PartialOrd for dyn Plugin {
     fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
         Some(self.cmp(other))

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(())
 }

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<Ordering> {
+        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<H: Hasher>(&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"