From 85769108100e73d56f392335421852006133b862 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Thu, 28 Sep 2023 14:27:03 +0200 Subject: [PATCH 1/6] escape backslash in Windows shell --- crates/rattler_shell/src/shell/mod.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/rattler_shell/src/shell/mod.rs b/crates/rattler_shell/src/shell/mod.rs index fa1007e82..3e896b61c 100644 --- a/crates/rattler_shell/src/shell/mod.rs +++ b/crates/rattler_shell/src/shell/mod.rs @@ -428,6 +428,12 @@ pub struct NuShell; impl Shell for NuShell { fn set_env_var(&self, f: &mut impl Write, env_var: &str, value: &str) -> std::fmt::Result { + // escape backslashes for Windows (make them double backslashes) + let value = if cfg!(windows) { + value.replace("\\", "\\\\") + } else { + value.to_string() + }; writeln!(f, "$env.{} = \"{}\"", env_var, value) } From a80e449502c9f1c748d8adbe4b50ad26a4fc0fc2 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Thu, 28 Sep 2023 14:32:04 +0200 Subject: [PATCH 2/6] additional nushell fixes --- crates/rattler_shell/src/shell/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/rattler_shell/src/shell/mod.rs b/crates/rattler_shell/src/shell/mod.rs index 3e896b61c..4851fd420 100644 --- a/crates/rattler_shell/src/shell/mod.rs +++ b/crates/rattler_shell/src/shell/mod.rs @@ -463,10 +463,10 @@ impl Shell for NuShell { self.set_env_var(f, "PATH", &format!("[{}]", path)) } PathModificationBehaviour::Prepend => { - writeln!(f, "let-env PATH = ($env.PATH | prepend [{}])", path) + writeln!(f, "$env.PATH = ($env.PATH | prepend [{}])", path) } PathModificationBehaviour::Append => { - writeln!(f, "let-env PATH = ($env.PATH | append [{}])", path) + writeln!(f, "$env.PATH = ($env.PATH | append [{}])", path) } } } From 5f2151628a55a44d6283fbc3409fc8d665813e9c Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Thu, 28 Sep 2023 14:49:46 +0200 Subject: [PATCH 3/6] rename PathModificationBehavior and expose it in ShellScript --- crates/rattler_shell/src/activation.rs | 34 +++++++++---------- crates/rattler_shell/src/shell/mod.rs | 46 ++++++++++++++++---------- py-rattler/src/shell.rs | 12 +++---- 3 files changed, 51 insertions(+), 41 deletions(-) diff --git a/crates/rattler_shell/src/activation.rs b/crates/rattler_shell/src/activation.rs index 7dcc9d063..b18799162 100644 --- a/crates/rattler_shell/src/activation.rs +++ b/crates/rattler_shell/src/activation.rs @@ -18,7 +18,7 @@ const ENV_START_SEPERATOR: &str = "<=== RATTLER ENV START ===>"; /// Type of modification done to the `PATH` variable #[derive(Default, Clone)] -pub enum PathModificationBehaviour { +pub enum PathModificationBehavior { /// Replaces the complete path variable with specified paths. #[default] Replace, @@ -39,7 +39,7 @@ pub struct ActivationVariables { pub path: Option>, /// The type of behaviour of what should happen with the defined paths. - pub path_modification_behaviour: PathModificationBehaviour, + pub path_modification_behaviour: PathModificationBehavior, } impl ActivationVariables { @@ -48,7 +48,7 @@ impl ActivationVariables { Ok(Self { conda_prefix: std::env::var("CONDA_PREFIX").ok().map(PathBuf::from), path: None, - path_modification_behaviour: PathModificationBehaviour::Prepend, + path_modification_behaviour: PathModificationBehavior::Prepend, }) } } @@ -485,7 +485,7 @@ mod tests { use tempdir::TempDir; #[cfg(unix)] - use crate::activation::PathModificationBehaviour; + use crate::activation::PathModificationBehavior; use crate::shell::ShellEnum; #[test] @@ -601,7 +601,7 @@ mod tests { #[cfg(unix)] fn get_script( shell_type: T, - path_modification_behaviour: PathModificationBehaviour, + path_modification_behaviour: PathModificationBehavior, ) -> String where T: Clone, @@ -631,25 +631,25 @@ mod tests { #[test] #[cfg(unix)] fn test_activation_script_bash() { - let script = get_script(shell::Bash, PathModificationBehaviour::Append); + let script = get_script(shell::Bash, PathModificationBehavior::Append); insta::assert_snapshot!("test_activation_script_bash_append", script); - let script = get_script(shell::Bash, PathModificationBehaviour::Replace); + let script = get_script(shell::Bash, PathModificationBehavior::Replace); insta::assert_snapshot!("test_activation_script_bash_replace", script); - let script = get_script(shell::Bash, PathModificationBehaviour::Prepend); + let script = get_script(shell::Bash, PathModificationBehavior::Prepend); insta::assert_snapshot!("test_activation_script_bash_prepend", script); } #[test] #[cfg(unix)] fn test_activation_script_zsh() { - let script = get_script(shell::Zsh, PathModificationBehaviour::Append); + let script = get_script(shell::Zsh, PathModificationBehavior::Append); insta::assert_snapshot!(script); } #[test] #[cfg(unix)] fn test_activation_script_fish() { - let script = get_script(shell::Fish, PathModificationBehaviour::Append); + let script = get_script(shell::Fish, PathModificationBehavior::Append); insta::assert_snapshot!(script); } @@ -658,17 +658,17 @@ mod tests { fn test_activation_script_powershell() { let script = get_script( shell::PowerShell::default(), - PathModificationBehaviour::Append, + PathModificationBehavior::Append, ); insta::assert_snapshot!("test_activation_script_powershell_append", script); let script = get_script( shell::PowerShell::default(), - PathModificationBehaviour::Prepend, + PathModificationBehavior::Prepend, ); insta::assert_snapshot!("test_activation_script_powershell_prepend", script); let script = get_script( shell::PowerShell::default(), - PathModificationBehaviour::Replace, + PathModificationBehavior::Replace, ); insta::assert_snapshot!("test_activation_script_powershell_replace", script); } @@ -676,18 +676,18 @@ mod tests { #[test] #[cfg(unix)] fn test_activation_script_cmd() { - let script = get_script(shell::CmdExe, PathModificationBehaviour::Append); + let script = get_script(shell::CmdExe, PathModificationBehavior::Append); insta::assert_snapshot!("test_activation_script_cmd_append", script); - let script = get_script(shell::CmdExe, PathModificationBehaviour::Replace); + let script = get_script(shell::CmdExe, PathModificationBehavior::Replace); insta::assert_snapshot!("test_activation_script_cmd_replace", script); - let script = get_script(shell::CmdExe, PathModificationBehaviour::Prepend); + let script = get_script(shell::CmdExe, PathModificationBehavior::Prepend); insta::assert_snapshot!("test_activation_script_cmd_prepend", script); } #[test] #[cfg(unix)] fn test_activation_script_xonsh() { - let script = get_script(shell::Xonsh, PathModificationBehaviour::Append); + let script = get_script(shell::Xonsh, PathModificationBehavior::Append); insta::assert_snapshot!(script); } diff --git a/crates/rattler_shell/src/shell/mod.rs b/crates/rattler_shell/src/shell/mod.rs index 4851fd420..385abb0c1 100644 --- a/crates/rattler_shell/src/shell/mod.rs +++ b/crates/rattler_shell/src/shell/mod.rs @@ -1,6 +1,6 @@ //! This module contains the [`Shell`] trait and implementations for various shells. -use crate::activation::PathModificationBehaviour; +use crate::activation::PathModificationBehavior; use enum_dispatch::enum_dispatch; use itertools::Itertools; use rattler_conda_types::Platform; @@ -55,7 +55,7 @@ pub trait Shell { &self, f: &mut impl Write, paths: &[PathBuf], - modification_behaviour: PathModificationBehaviour, + modification_behaviour: PathModificationBehavior, platform: &Platform, ) -> std::fmt::Result { let mut paths_vec = paths @@ -64,9 +64,9 @@ pub trait Shell { .collect_vec(); // Replace, Append, or Prepend the path variable to the paths. match modification_behaviour { - PathModificationBehaviour::Replace => (), - PathModificationBehaviour::Append => paths_vec.insert(0, self.format_env_var("PATH")), - PathModificationBehaviour::Prepend => paths_vec.push(self.format_env_var("PATH")), + PathModificationBehavior::Replace => (), + PathModificationBehavior::Append => paths_vec.insert(0, self.format_env_var("PATH")), + PathModificationBehavior::Prepend => paths_vec.push(self.format_env_var("PATH")), } // Create the shell specific list of paths. let paths_string = paths_vec.join(self.path_seperator(platform)); @@ -167,7 +167,7 @@ impl Shell for Bash { &self, f: &mut impl Write, paths: &[PathBuf], - modification_behaviour: PathModificationBehaviour, + modification_behaviour: PathModificationBehavior, platform: &Platform, ) -> std::fmt::Result { // Put paths in a vector of the correct format. @@ -193,9 +193,9 @@ impl Shell for Bash { // Replace, Append, or Prepend the path variable to the paths. match modification_behaviour { - PathModificationBehaviour::Replace => (), - PathModificationBehaviour::Prepend => paths_vec.push(self.format_env_var("PATH")), - PathModificationBehaviour::Append => paths_vec.insert(0, self.format_env_var("PATH")), + PathModificationBehavior::Replace => (), + PathModificationBehavior::Prepend => paths_vec.push(self.format_env_var("PATH")), + PathModificationBehavior::Append => paths_vec.insert(0, self.format_env_var("PATH")), } // Create the shell specific list of paths. let paths_string = paths_vec.join(self.path_seperator(platform)); @@ -430,7 +430,7 @@ impl Shell for NuShell { fn set_env_var(&self, f: &mut impl Write, env_var: &str, value: &str) -> std::fmt::Result { // escape backslashes for Windows (make them double backslashes) let value = if cfg!(windows) { - value.replace("\\", "\\\\") + value.replace('\\', "\\\\") } else { value.to_string() }; @@ -449,7 +449,7 @@ impl Shell for NuShell { &self, f: &mut impl Write, paths: &[PathBuf], - modification_behaviour: PathModificationBehaviour, + modification_behaviour: PathModificationBehavior, _platform: &Platform, ) -> std::fmt::Result { let path = paths @@ -459,13 +459,13 @@ impl Shell for NuShell { // Replace, Append, or Prepend the path variable to the paths. match modification_behaviour { - PathModificationBehaviour::Replace => { + PathModificationBehavior::Replace => { self.set_env_var(f, "PATH", &format!("[{}]", path)) } - PathModificationBehaviour::Prepend => { + PathModificationBehavior::Prepend => { writeln!(f, "$env.PATH = ($env.PATH | prepend [{}])", path) } - PathModificationBehaviour::Append => { + PathModificationBehavior::Append => { writeln!(f, "$env.PATH = ($env.PATH | append [{}])", path) } } @@ -660,12 +660,16 @@ impl ShellScript { } /// Set the PATH environment variable to the given paths. - pub fn set_path(&mut self, paths: &[PathBuf]) -> &mut Self { + pub fn set_path( + &mut self, + paths: &[PathBuf], + path_modification_behavior: PathModificationBehavior, + ) -> &mut Self { self.shell .set_path( &mut self.contents, paths, - PathModificationBehaviour::Prepend, + path_modification_behavior, &self.platform, ) .unwrap(); @@ -725,11 +729,17 @@ mod tests { #[test] fn test_path_seperator() { let mut script = ShellScript::new(Bash, Platform::Linux64); - script.set_path(&[PathBuf::from("/foo"), PathBuf::from("/bar")]); + script.set_path( + &[PathBuf::from("/foo"), PathBuf::from("/bar")], + PathModificationBehavior::Prepend, + ); assert!(script.contents.contains("/foo:/bar")); let mut script = ShellScript::new(Bash, Platform::Win64); - script.set_path(&[PathBuf::from("/foo"), PathBuf::from("/bar")]); + script.set_path( + &[PathBuf::from("/foo"), PathBuf::from("/bar")], + PathModificationBehavior::Prepend, + ); assert!(script.contents.contains("/foo;/bar")); } } diff --git a/py-rattler/src/shell.rs b/py-rattler/src/shell.rs index 84d1f7011..0dcf83d92 100644 --- a/py-rattler/src/shell.rs +++ b/py-rattler/src/shell.rs @@ -2,7 +2,7 @@ use crate::error::PyRattlerError; use crate::platform::PyPlatform; use pyo3::{exceptions::PyValueError, pyclass, pymethods, FromPyObject, PyAny, PyResult}; use rattler_shell::{ - activation::{ActivationResult, ActivationVariables, Activator, PathModificationBehaviour}, + activation::{ActivationResult, ActivationVariables, Activator, PathModificationBehavior}, shell::{Bash, CmdExe, Fish, PowerShell, Xonsh, Zsh}, }; use std::path::{Path, PathBuf}; @@ -23,12 +23,12 @@ impl From for PyActivationVariables { #[repr(transparent)] pub struct Wrap(pub T); -impl FromPyObject<'_> for Wrap { +impl FromPyObject<'_> for Wrap { fn extract(ob: &PyAny) -> PyResult { let parsed = match ob.extract::<&str>()? { - "prepend" => PathModificationBehaviour::Prepend, - "append" => PathModificationBehaviour::Append, - "replace" => PathModificationBehaviour::Replace, + "prepend" => PathModificationBehavior::Prepend, + "append" => PathModificationBehavior::Append, + "replace" => PathModificationBehavior::Replace, v => { return Err(PyValueError::new_err(format!( "keep must be one of {{'prepend', 'append', 'replace'}}, got {v}", @@ -46,7 +46,7 @@ impl PyActivationVariables { pub fn __init__( conda_prefix: Option, path: Option>, - path_modification_behaviour: Wrap, + path_modification_behaviour: Wrap, ) -> Self { let activation_vars = ActivationVariables { conda_prefix, From 85c0c7e09d95114422c84876be09cab5e60762a7 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Fri, 29 Sep 2023 13:04:09 +0200 Subject: [PATCH 4/6] do not quote list --- crates/rattler_shell/src/shell/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/rattler_shell/src/shell/mod.rs b/crates/rattler_shell/src/shell/mod.rs index 385abb0c1..fa3b105a9 100644 --- a/crates/rattler_shell/src/shell/mod.rs +++ b/crates/rattler_shell/src/shell/mod.rs @@ -460,7 +460,7 @@ impl Shell for NuShell { // Replace, Append, or Prepend the path variable to the paths. match modification_behaviour { PathModificationBehavior::Replace => { - self.set_env_var(f, "PATH", &format!("[{}]", path)) + writeln!(f, "$env.PATH = [{}]", path) } PathModificationBehavior::Prepend => { writeln!(f, "$env.PATH = ($env.PATH | prepend [{}])", path) From aa7b74cf91c343044404eddfa33562366fbfe762 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Fri, 29 Sep 2023 13:14:13 +0200 Subject: [PATCH 5/6] more fixes --- crates/rattler_shell/src/shell/mod.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/rattler_shell/src/shell/mod.rs b/crates/rattler_shell/src/shell/mod.rs index fa3b105a9..1e38617a6 100644 --- a/crates/rattler_shell/src/shell/mod.rs +++ b/crates/rattler_shell/src/shell/mod.rs @@ -422,6 +422,10 @@ impl Shell for Fish { } } +fn escape_backslashes(s: &str) -> String { + s.replace('\\', "\\\\") +} + /// A [`Shell`] implementation for the Bash shell. #[derive(Debug, Clone, Copy, Default)] pub struct NuShell; @@ -430,7 +434,7 @@ impl Shell for NuShell { fn set_env_var(&self, f: &mut impl Write, env_var: &str, value: &str) -> std::fmt::Result { // escape backslashes for Windows (make them double backslashes) let value = if cfg!(windows) { - value.replace('\\', "\\\\") + escape_backslashes(value) } else { value.to_string() }; @@ -454,7 +458,7 @@ impl Shell for NuShell { ) -> std::fmt::Result { let path = paths .iter() - .map(|path| format!("\"{}\"", path.to_string_lossy().into_owned())) + .map(|path| escape_backslashes(&format!("\"{}\"", path.to_string_lossy().into_owned()))) .join(", "); // Replace, Append, or Prepend the path variable to the paths. From 53fb566aa1acc61653a80a9ddef651bd066b0e80 Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Thu, 5 Oct 2023 13:53:14 +0200 Subject: [PATCH 6/6] always escape backslashes --- crates/rattler_shell/src/shell/mod.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/crates/rattler_shell/src/shell/mod.rs b/crates/rattler_shell/src/shell/mod.rs index 1e38617a6..c5f428f17 100644 --- a/crates/rattler_shell/src/shell/mod.rs +++ b/crates/rattler_shell/src/shell/mod.rs @@ -433,12 +433,7 @@ pub struct NuShell; impl Shell for NuShell { fn set_env_var(&self, f: &mut impl Write, env_var: &str, value: &str) -> std::fmt::Result { // escape backslashes for Windows (make them double backslashes) - let value = if cfg!(windows) { - escape_backslashes(value) - } else { - value.to_string() - }; - writeln!(f, "$env.{} = \"{}\"", env_var, value) + writeln!(f, "$env.{} = \"{}\"", env_var, escape_backslashes(value)) } fn unset_env_var(&self, f: &mut impl Write, env_var: &str) -> std::fmt::Result {