shims: insert rtx PATH entries just before the shim directory

Before, rtx would put the PATH entries at the front of PATH but that is too aggressive in some scenarios

Fixes #1061

e2e/assert.sh

@@ -33,3 +33,12 @@ assert_fail() {
 		exit 1
 	fi
 }
+
+assert_matches() {
+	local actual
+	actual="$(bash -c "$1")"
+	if [[ ! "$actual" =~ $2 ]]; then
+		echo "Expected '$2' to match '$actual'"
+		exit 1
+	fi
+}

e2e/test_shims

@@ -3,6 +3,11 @@ set -euo pipefail
 # shellcheck source-path=SCRIPTDIR
 source "$(dirname "$0")/assert.sh"
 
+export PATH="/XSTARTX:$RTX_DATA_DIR/shims:/XENDX$PATH"
 rtx i node
 rtx reshim
-assert "$RTX_DATA_DIR/shims/node -v" "v20.0.0"
+assert "node -v" "v20.0.0"
+
+# should still have this prefix since rtx should only modify PATH
+# starting at the shim directory
+assert_matches "node -p 'process.env.PATH'" "^/XSTARTX:"

src/main.rs

@@ -46,6 +46,7 @@ mod install_context;
 mod lock_file;
 mod logger;
 mod migrate;
+mod path_env;
 mod plugins;
 mod rand;
 mod runtime_symlinks;

src/path_env.rs

@@ -0,0 +1,122 @@
+use crate::dirs;
+use itertools::Itertools;
+use std::env::join_paths;
+use std::ffi::OsString;
+use std::fmt;
+use std::fmt::{Display, Formatter};
+use std::path::PathBuf;
+
+pub struct PathEnv {
+    pre: Vec<PathBuf>,
+    rtx: Vec<PathBuf>,
+    post: Vec<PathBuf>,
+    seen_shims: bool,
+}
+
+impl PathEnv {
+    pub fn new() -> Self {
+        Self {
+            pre: Vec::new(),
+            rtx: Vec::new(),
+            post: Vec::new(),
+            seen_shims: false,
+        }
+    }
+
+    pub fn add(&mut self, path: PathBuf) {
+        self.rtx.push(path);
+    }
+
+    pub fn to_vec(&self) -> Vec<PathBuf> {
+        let mut paths = self.pre.iter().chain(self.rtx.iter()).collect_vec();
+        if self.seen_shims {
+            paths.push(&dirs::SHIMS);
+        }
+        paths.into_iter().chain(self.post.iter()).cloned().collect()
+    }
+
+    pub fn join(&self) -> OsString {
+        join_paths(self.to_vec()).unwrap()
+    }
+}
+
+impl Display for PathEnv {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.join().to_string_lossy())
+    }
+}
+
+impl FromIterator<PathBuf> for PathEnv {
+    fn from_iter<T: IntoIterator<Item = PathBuf>>(paths: T) -> Self {
+        let mut path_env = Self::new();
+
+        for path in paths {
+            if path_env.seen_shims {
+                path_env.post.push(path);
+            } else if path == *dirs::SHIMS {
+                path_env.seen_shims = true;
+            } else {
+                path_env.pre.push(path);
+            }
+        }
+        if !path_env.seen_shims {
+            path_env.post = path_env.pre;
+            path_env.pre = Vec::new();
+        }
+
+        path_env
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_path_env() {
+        let mut path_env = PathEnv::from_iter(
+            [
+                "/before-1",
+                "/before-2",
+                "/before-3",
+                dirs::SHIMS.to_str().unwrap(),
+                "/after-1",
+                "/after-2",
+                "/after-3",
+            ]
+            .map(PathBuf::from),
+        );
+        path_env.add("/1".into());
+        path_env.add("/2".into());
+        path_env.add("/3".into());
+        assert_eq!(
+            path_env.to_string(),
+            format!(
+                "/before-1:/before-2:/before-3:/1:/2:/3:{}:/after-1:/after-2:/after-3",
+                dirs::SHIMS.to_str().unwrap()
+            )
+        );
+    }
+
+    #[test]
+    fn test_path_env_no_rtx() {
+        let mut path_env = PathEnv::from_iter(
+            [
+                "/before-1",
+                "/before-2",
+                "/before-3",
+                "/after-1",
+                "/after-2",
+                "/after-3",
+            ]
+            .map(PathBuf::from),
+        );
+        path_env.add("/1".into());
+        path_env.add("/2".into());
+        path_env.add("/3".into());
+        assert_eq!(
+            path_env.to_string(),
+            format!("/1:/2:/3:/before-1:/before-2:/before-3:/after-1:/after-2:/after-3")
+        );
+    }
+}

src/toolset/mod.rs

@@ -1,5 +1,4 @@
 use std::collections::{BTreeMap, BTreeSet, HashMap};
-use std::env::join_paths;
 use std::fmt::{Display, Formatter};
 use std::path::PathBuf;
 use std::sync::{Arc, Mutex};
@@ -20,6 +19,7 @@ pub use tool_version_request::ToolVersionRequest;
 use crate::config::Config;
 use crate::env;
 use crate::install_context::InstallContext;
+use crate::path_env::PathEnv;
 use crate::plugins::PluginName;
 use crate::runtime_symlinks;
 use crate::shims;
@@ -261,12 +261,18 @@ impl Toolset {
             .collect()
     }
     pub fn env_with_path(&self, config: &Config) -> BTreeMap<String, String> {
+        let mut path_env = PathEnv::from_iter(env::PATH.clone());
+        for p in config.path_dirs.clone() {
+            path_env.add(p);
+        }
+        for p in self.list_paths(config) {
+            path_env.add(p);
+        }
         let mut env = self.env(config);
-        let mut path_env = self.path_env(config);
         if let Some(path) = env.get("PATH") {
-            path_env = format!("{}:{}", path, path_env);
+            path_env.add(PathBuf::from(path));
         }
-        env.insert("PATH".to_string(), path_env);
+        env.insert("PATH".to_string(), path_env.to_string());
         env
     }
     pub fn env(&self, config: &Config) -> BTreeMap<String, String> {
@@ -298,13 +304,6 @@ impl Toolset {
         entries.extend(config.env.clone());
         entries
     }
-    pub fn path_env(&self, config: &Config) -> String {
-        let installs = self.list_paths(config);
-        join_paths([config.path_dirs.clone(), installs, env::PATH.clone()].concat())
-            .unwrap()
-            .to_string_lossy()
-            .into()
-    }
     pub fn list_paths(&self, config: &Config) -> Vec<PathBuf> {
         self.list_current_installed_versions(config)
             .into_par_iter()