config: added rwlock around plugins map (#1143)

* config: added rwlock around plugins map * Commit from GitHub Actions (rtx) * reduce write locking --------- Co-authored-by: rtx[bot] <[email protected]>
jdx · Dec 12, 2023 · 360491c · 360491c
1 parent b0e668f
commit 360491c
Show file tree

Hide file tree

Showing 42 changed files with 192 additions and 232 deletions.
diff --git a/src/cli/asdf.rs b/src/cli/asdf.rs
@@ -36,15 +36,15 @@ impl Asdf {
     }
 }
 
-fn list_versions(mut config: Config, out: &mut Output, args: &Vec<String>) -> Result<()> {
+fn list_versions(config: Config, out: &mut Output, args: &Vec<String>) -> Result<()> {
     if args[2] == "all" {
         let mut new_args: Vec<String> = vec!["rtx".into(), "ls-remote".into()];
         if args.len() >= 3 {
             new_args.push(args[3].clone());
         }
         return Cli::new().run(config, &new_args, out);
     }
-    let ts = ToolsetBuilder::new().build(&mut config)?;
+    let ts = ToolsetBuilder::new().build(&config)?;
     let mut versions = ts.list_installed_versions(&config)?;
     let plugin = match args.len() {
         3 => Some(&args[2]),

diff --git a/src/cli/bin_paths.rs b/src/cli/bin_paths.rs
@@ -10,8 +10,8 @@ use crate::toolset::ToolsetBuilder;
 pub struct BinPaths {}
 
 impl BinPaths {
-    pub fn run(self, mut config: Config, out: &mut Output) -> Result<()> {
-        let ts = ToolsetBuilder::new().build(&mut config)?;
+    pub fn run(self, config: Config, out: &mut Output) -> Result<()> {
+        let ts = ToolsetBuilder::new().build(&config)?;
         for p in ts.list_paths(&config) {
             rtxprintln!(out, "{}", p.display());
         }

diff --git a/src/cli/current.rs b/src/cli/current.rs
@@ -20,18 +20,16 @@ pub struct Current {
 }
 
 impl Current {
-    pub fn run(self, mut config: Config, out: &mut Output) -> Result<()> {
-        let ts = ToolsetBuilder::new().build(&mut config)?;
+    pub fn run(self, config: Config, out: &mut Output) -> Result<()> {
+        let ts = ToolsetBuilder::new().build(&config)?;
         match &self.plugin {
             Some(plugin_name) => {
                 let plugin_name = unalias_plugin(plugin_name);
-                match config.plugins.get(plugin_name) {
-                    Some(plugin) => self.one(&config, ts, out, plugin.clone()),
-                    None => {
-                        warn!("Plugin {} is not installed", plugin_name);
-                        Ok(())
-                    }
+                let plugin = config.get_or_create_plugin(plugin_name);
+                if !plugin.is_installed() {
+                    bail!("Plugin {} is not installed", plugin_name);
                 }
+                self.one(&config, ts, out, plugin.clone())
             }
             None => self.all(&config, ts, out),
         }

diff --git a/src/cli/direnv/envrc.rs b/src/cli/direnv/envrc.rs
@@ -17,8 +17,8 @@ use crate::{dirs, env};
 pub struct Envrc {}
 
 impl Envrc {
-    pub fn run(self, mut config: Config, out: &mut Output) -> Result<()> {
-        let ts = ToolsetBuilder::new().build(&mut config)?;
+    pub fn run(self, config: Config, out: &mut Output) -> Result<()> {
+        let ts = ToolsetBuilder::new().build(&config)?;
 
         let envrc_path = env::RTX_TMP_DIR
             .join("direnv")

diff --git a/src/cli/direnv/exec.rs b/src/cli/direnv/exec.rs
@@ -20,8 +20,8 @@ struct DirenvWatches {
 }
 
 impl DirenvExec {
-    pub fn run(self, mut config: Config, out: &mut Output) -> Result<()> {
-        let ts = ToolsetBuilder::new().build(&mut config)?;
+    pub fn run(self, config: Config, out: &mut Output) -> Result<()> {
+        let ts = ToolsetBuilder::new().build(&config)?;
 
         let mut cmd = env_cmd();
 

diff --git a/src/cli/doctor.rs b/src/cli/doctor.rs
@@ -22,8 +22,8 @@ use crate::{duration, env};
 pub struct Doctor {}
 
 impl Doctor {
-    pub fn run(self, mut config: Config, out: &mut Output) -> Result<()> {
-        let ts = ToolsetBuilder::new().build(&mut config)?;
+    pub fn run(self, config: Config, out: &mut Output) -> Result<()> {
+        let ts = ToolsetBuilder::new().build(&config)?;
         rtxprintln!(out, "{}", rtx_version());
         rtxprintln!(out, "{}", build_info());
         rtxprintln!(out, "{}", shell());
@@ -45,7 +45,7 @@ impl Doctor {
         );
 
         let mut checks = Vec::new();
-        for plugin in config.plugins.values() {
+        for plugin in config.list_plugins() {
             if !plugin.is_installed() {
                 checks.push(format!("plugin {} is not installed", &plugin.name()));
                 continue;
@@ -123,8 +123,8 @@ fn render_config_files(config: &Config) -> String {
 fn render_plugins(config: &Config) -> String {
     let mut s = style("plugins:\n").bold().to_string();
     let plugins = config
-        .plugins
-        .values()
+        .list_plugins()
+        .into_iter()
         .filter(|p| p.is_installed())
         .collect::<Vec<_>>();
     let max_plugin_name_len = plugins.iter().map(|p| p.name().len()).max().unwrap_or(0) + 2;

diff --git a/src/cli/env.rs b/src/cli/env.rs
@@ -27,11 +27,9 @@ pub struct Env {
 }
 
 impl Env {
-    pub fn run(self, mut config: Config, out: &mut Output) -> Result<()> {
-        let mut ts = ToolsetBuilder::new()
-            .with_args(&self.tool)
-            .build(&mut config)?;
-        ts.install_arg_versions(&mut config)?;
+    pub fn run(self, config: Config, out: &mut Output) -> Result<()> {
+        let mut ts = ToolsetBuilder::new().with_args(&self.tool).build(&config)?;
+        ts.install_arg_versions(&config)?;
 
         if self.json {
             self.output_json(config, out, ts)

diff --git a/src/cli/exec.rs b/src/cli/exec.rs
@@ -45,11 +45,9 @@ pub struct Exec {
 }
 
 impl Exec {
-    pub fn run(self, mut config: Config, _out: &mut Output) -> Result<()> {
-        let mut ts = ToolsetBuilder::new()
-            .with_args(&self.tool)
-            .build(&mut config)?;
-        ts.install_arg_versions(&mut config)?;
+    pub fn run(self, config: Config, _out: &mut Output) -> Result<()> {
+        let mut ts = ToolsetBuilder::new().with_args(&self.tool).build(&config)?;
+        ts.install_arg_versions(&config)?;
 
         let (program, args) = parse_command(&env::SHELL, &self.command, &self.c);
         let env = ts.env_with_path(&config);

diff --git a/src/cli/external.rs b/src/cli/external.rs
@@ -1,26 +1,19 @@
 use clap::{ArgMatches, Command};
 use color_eyre::eyre::Result;
-use itertools::Itertools;
 use rayon::prelude::*;
 
 use crate::config::Config;
 
 pub fn commands(config: &Config) -> Vec<Command> {
     config
-        .plugins
-        .values()
-        .collect_vec()
+        .list_plugins()
         .into_par_iter()
-        .flat_map(|p| match p.external_commands() {
-            Ok(commands) => commands,
-            Err(e) => {
-                warn!(
-                    "failed to load external commands for plugin {}: {:#}",
-                    p.name(),
-                    e
-                );
+        .flat_map(|p| {
+            p.external_commands().unwrap_or_else(|e| {
+                let p = p.name();
+                warn!("failed to load external commands for plugin {p}: {e:#}");
                 vec![]
-            }
+            })
         })
         .collect()
 }
@@ -36,7 +29,7 @@ pub fn execute(
         .find(|c| c.get_name() == plugin)
     {
         if let Some((subcommand, matches)) = args.subcommand() {
-            let plugin = config.plugins.get(&plugin.to_string()).unwrap();
+            let plugin = config.get_or_create_plugin(plugin);
             let args: Vec<String> = matches
                 .get_raw("args")
                 .unwrap_or_default()

diff --git a/src/cli/hook_env.rs b/src/cli/hook_env.rs
@@ -32,8 +32,8 @@ pub struct HookEnv {
 }
 
 impl HookEnv {
-    pub fn run(self, mut config: Config, out: &mut Output) -> Result<()> {
-        let ts = ToolsetBuilder::new().build(&mut config)?;
+    pub fn run(self, config: Config, out: &mut Output) -> Result<()> {
+        let ts = ToolsetBuilder::new().build(&config)?;
         let shell = get_shell(self.shell).expect("no shell provided, use `--shell=zsh`");
         out.stdout.write(hook_env::clear_old_env(&*shell));
         let mut env = ts.env(&config);

diff --git a/src/cli/install.rs b/src/cli/install.rs
@@ -42,23 +42,23 @@ impl Install {
 
         Ok(())
     }
-    fn install_runtimes(&self, mut config: Config, runtimes: &[ToolArg]) -> Result<()> {
+    fn install_runtimes(&self, config: Config, runtimes: &[ToolArg]) -> Result<()> {
         let mpr = MultiProgressReport::new(&config.settings);
         let mut ts = ToolsetBuilder::new()
             .with_latest_versions()
-            .build(&mut config)?;
-        let tool_versions = self.get_requested_tool_versions(&mut config, &ts, runtimes, &mpr)?;
+            .build(&config)?;
+        let tool_versions = self.get_requested_tool_versions(&config, &ts, runtimes, &mpr)?;
         if tool_versions.is_empty() {
             warn!("no runtimes to install");
             warn!("specify a version with `rtx install <PLUGIN>@<VERSION>`");
             return Ok(());
         }
-        ts.install_versions(&mut config, tool_versions, &mpr, self.force)
+        ts.install_versions(&config, tool_versions, &mpr, self.force)
     }
 
     fn get_requested_tool_versions(
         &self,
-        config: &mut Config,
+        config: &Config,
         ts: &Toolset,
         runtimes: &[ToolArg],
         mpr: &MultiProgressReport,
@@ -102,10 +102,10 @@ impl Install {
         Ok(tool_versions)
     }
 
-    fn install_missing_runtimes(&self, mut config: Config) -> Result<()> {
+    fn install_missing_runtimes(&self, config: Config) -> Result<()> {
         let mut ts = ToolsetBuilder::new()
             .with_latest_versions()
-            .build(&mut config)?;
+            .build(&config)?;
         let versions = ts
             .list_missing_versions(&config)
             .into_iter()
@@ -116,7 +116,7 @@ impl Install {
             return Ok(());
         }
         let mpr = MultiProgressReport::new(&config.settings);
-        ts.install_versions(&mut config, versions, &mpr, self.force)?;
+        ts.install_versions(&config, versions, &mpr, self.force)?;
         Ok(())
     }
 }

diff --git a/src/cli/latest.rs b/src/cli/latest.rs
@@ -26,7 +26,7 @@ pub struct Latest {
 }
 
 impl Latest {
-    pub fn run(self, mut config: Config, out: &mut Output) -> Result<()> {
+    pub fn run(self, config: Config, out: &mut Output) -> Result<()> {
         let mut prefix = match self.tool.tvr {
             None => self.asdf_version,
             Some(ToolVersionRequest::Version(_, version)) => Some(version),
@@ -37,7 +37,7 @@ impl Latest {
         };
 
         let plugin = config.get_or_create_plugin(&self.tool.plugin);
-        plugin.ensure_installed(&mut config, None, false)?;
+        plugin.ensure_installed(&config, None, false)?;
         if let Some(v) = prefix {
             prefix = Some(config.resolve_alias(plugin.name(), &v)?);
         }

diff --git a/src/cli/link.rs b/src/cli/link.rs
@@ -33,7 +33,7 @@ pub struct Link {
 }
 
 impl Link {
-    pub fn run(self, mut config: Config, _out: &mut Output) -> Result<()> {
+    pub fn run(self, config: Config, _out: &mut Output) -> Result<()> {
         let version = match self.tool.tvr {
             Some(ref tvr) => tvr.version(),
             None => {

diff --git a/src/cli/local.rs b/src/cli/local.rs
@@ -93,7 +93,7 @@ pub fn get_parent_path() -> Result<PathBuf> {
 
 #[allow(clippy::too_many_arguments)]
 pub fn local(
-    mut config: Config,
+    config: Config,
     out: &mut Output,
     path: &Path,
     runtime: Vec<ToolArg>,
@@ -128,7 +128,7 @@ pub fn local(
             return Ok(());
         }
         let pin = pin || (config.settings.asdf_compat && !fuzzy);
-        cf.add_runtimes(&mut config, &runtimes, pin)?;
+        cf.add_runtimes(&config, &runtimes, pin)?;
         let tools = runtimes.iter().map(|r| r.to_string()).join(" ");
         rtxprintln!(
             out,

diff --git a/src/cli/ls.rs b/src/cli/ls.rs
@@ -59,15 +59,15 @@ pub struct Ls {
 }
 
 impl Ls {
-    pub fn run(mut self, mut config: Config, out: &mut Output) -> Result<()> {
+    pub fn run(mut self, config: Config, out: &mut Output) -> Result<()> {
         self.plugin = self
             .plugin
             .clone()
             .or(self.plugin_flag.clone())
             .map(|p| PluginName::from(unalias_plugin(&p)));
         self.verify_plugin(&config)?;
 
-        let mut runtimes = self.get_runtime_list(&mut config)?;
+        let mut runtimes = self.get_runtime_list(&config)?;
         if self.current || self.global {
             // TODO: global is a little weird: it will show global versions as the active ones even if
             // they're overridden locally
@@ -94,8 +94,8 @@ impl Ls {
     fn verify_plugin(&self, config: &Config) -> Result<()> {
         match &self.plugin {
             Some(plugin_name) => {
-                let plugin = config.plugins.get(plugin_name);
-                if plugin.is_none() || !plugin.unwrap().is_installed() {
+                let plugin = config.get_or_create_plugin(plugin_name);
+                if !plugin.is_installed() {
                     return Err(PluginNotInstalled(plugin_name.clone()))?;
                 }
             }
@@ -209,12 +209,11 @@ impl Ls {
         Ok(())
     }
 
-    fn get_runtime_list(&self, config: &mut Config) -> Result<Vec<RuntimeRow>> {
+    fn get_runtime_list(&self, config: &Config) -> Result<Vec<RuntimeRow>> {
         let mut tsb = ToolsetBuilder::new().with_global_only(self.global);
 
         if let Some(plugin) = &self.plugin {
             tsb = tsb.with_tools(&[plugin]);
-            config.plugins.retain(|p, _| p == plugin);
         }
         let ts = tsb.build(config)?;
         let mut versions: HashMap<(String, String), (Arc<dyn Plugin>, ToolVersion)> = ts
@@ -233,6 +232,10 @@ impl Ls {
 
         let rvs: Vec<RuntimeRow> = versions
             .into_iter()
+            .filter(|((plugin_name, _), _)| match &self.plugin {
+                Some(p) => p.eq(plugin_name),
+                None => true,
+            })
             .sorted_by_cached_key(|((plugin_name, version), _)| {
                 (
                     plugin_name.clone(),

diff --git a/src/cli/ls_remote.rs b/src/cli/ls_remote.rs
@@ -33,8 +33,8 @@ pub struct LsRemote {
 }
 
 impl LsRemote {
-    pub fn run(self, mut config: Config, out: &mut Output) -> Result<()> {
-        if let Some(plugin) = self.get_plugin(&mut config)? {
+    pub fn run(self, config: Config, out: &mut Output) -> Result<()> {
+        if let Some(plugin) = self.get_plugin(&config)? {
             self.run_single(config, out, plugin)
         } else {
             self.run_all(config, out)
@@ -68,9 +68,8 @@ impl LsRemote {
 
     fn run_all(self, config: Config, out: &mut Output) -> Result<()> {
         let versions = config
-            .plugins
-            .values()
-            .par_bridge()
+            .list_plugins()
+            .into_par_iter()
             .map(|p| {
                 let versions = p.list_remote_versions(&config.settings)?;
                 Ok((p, versions))
@@ -87,7 +86,7 @@ impl LsRemote {
         Ok(())
     }
 
-    fn get_plugin(&self, config: &mut Config) -> Result<Option<Arc<dyn Plugin>>> {
+    fn get_plugin(&self, config: &Config) -> Result<Option<Arc<dyn Plugin>>> {
         match &self.plugin {
             Some(tool_arg) => {
                 let plugin = config.get_or_create_plugin(&tool_arg.plugin);

diff --git a/src/cli/outdated.rs b/src/cli/outdated.rs
@@ -22,10 +22,8 @@ pub struct Outdated {
 }
 
 impl Outdated {
-    pub fn run(self, mut config: Config, out: &mut Output) -> Result<()> {
-        let mut ts = ToolsetBuilder::new()
-            .with_args(&self.tool)
-            .build(&mut config)?;
+    pub fn run(self, config: Config, out: &mut Output) -> Result<()> {
+        let mut ts = ToolsetBuilder::new().with_args(&self.tool).build(&config)?;
         let tool_set = self
             .tool
             .iter()