Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add path variable modification for the shells #232

Merged
merged 8 commits into from
Jun 27, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 48 additions & 14 deletions crates/rattler_shell/src/activation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ use crate::shell::Shell;
use indexmap::IndexMap;
use rattler_conda_types::Platform;

/// Type of modification done to the `PATH` variable
pub enum PathModificationBehaviour {
/// Replaces the complete path variable with specified paths.
Replace,
/// Appends the new path variables to the path. E.g. '$PATH:/new/path'
Append,
/// Prepends the new path variables to the path. E.g. '/new/path:$PATH'
Prepend,
}

/// A struct that contains the values of the environment variables that are relevant for the activation process.
/// The values are stored as strings. Currently, only the `PATH` and `CONDA_PREFIX` environment variables are used.
pub struct ActivationVariables {
Expand All @@ -20,16 +30,18 @@ pub struct ActivationVariables {

/// The value of the `PATH` environment variable that contains the paths to the executables
pub path: Option<Vec<PathBuf>>,

/// The type of behaviour of what should happen with the defined paths.
pub path_modification_behaviour: PathModificationBehaviour,
}

impl ActivationVariables {
/// Create a new `ActivationVariables` struct from the environment variables.
pub fn from_env() -> Result<Self, std::env::VarError> {
Ok(Self {
conda_prefix: std::env::var("CONDA_PREFIX").ok().map(PathBuf::from),
path: std::env::var("PATH")
.ok()
.map(|p| std::env::split_paths(&p).collect::<Vec<_>>()),
path: None,
path_modification_behaviour: PathModificationBehaviour::Prepend,
})
}
}
Expand Down Expand Up @@ -340,7 +352,11 @@ impl<T: Shell + Clone> Activator<T> {
let path = [self.paths.clone(), path].concat();

self.shell_type
.set_path(&mut script, path.as_slice())
.set_path(
&mut script,
path.as_slice(),
variables.path_modification_behaviour,
)
.map_err(ActivationError::FailedToWriteActivationScript)?;

// deliberately not taking care of `CONDA_SHLVL` or any other complications at this point
Expand Down Expand Up @@ -370,12 +386,15 @@ impl<T: Shell + Clone> Activator<T> {

#[cfg(test)]
mod tests {
use crate::shell;
use crate::{activation, shell};
use std::str::FromStr;

use super::*;
use tempdir::TempDir;

#[cfg(unix)]
use activation::PathModificationBehaviour;

#[test]
fn test_collect_scripts() {
let tdir = TempDir::new("test").unwrap();
Expand Down Expand Up @@ -487,7 +506,10 @@ mod tests {
}

#[cfg(unix)]
fn get_script<T: Shell>(shell_type: T) -> String
fn get_script<T: Shell>(
shell_type: T,
path_modification_behaviour: PathModificationBehaviour,
) -> String
where
T: Clone,
{
Expand All @@ -505,6 +527,7 @@ mod tests {
PathBuf::from("/sbin"),
PathBuf::from("/usr/local/bin"),
]),
path_modification_behaviour,
})
.unwrap();
let prefix = tdir.path().to_str().unwrap();
Expand All @@ -515,42 +538,53 @@ mod tests {
#[test]
#[cfg(unix)]
fn test_activation_script_bash() {
let script = get_script(shell::Bash);
insta::assert_snapshot!(script);
let script = get_script(shell::Bash, PathModificationBehaviour::Append);
insta::assert_snapshot!("test_activation_script_bash_append", script);
let script = get_script(shell::Bash, PathModificationBehaviour::Replace);
insta::assert_snapshot!("test_activation_script_bash_replace", script);
let script = get_script(shell::Bash, PathModificationBehaviour::Prepend);
insta::assert_snapshot!("test_activation_script_bash_prepend", script);
}

#[test]
#[cfg(unix)]
fn test_activation_script_zsh() {
let script = get_script(shell::Zsh);
let script = get_script(shell::Zsh, PathModificationBehaviour::Append);
insta::assert_snapshot!(script);
}

#[test]
#[cfg(unix)]
fn test_activation_script_fish() {
let script = get_script(shell::Fish);
let script = get_script(shell::Fish, PathModificationBehaviour::Append);
insta::assert_snapshot!(script);
}

#[test]
#[cfg(unix)]
fn test_activation_script_powershell() {
let script = get_script(shell::PowerShell::default());
let script = get_script(
shell::PowerShell::default(),
PathModificationBehaviour::Append,
);
insta::assert_snapshot!(script);
}

#[test]
#[cfg(unix)]
fn test_activation_script_cmd() {
let script = get_script(shell::CmdExe);
insta::assert_snapshot!(script);
let script = get_script(shell::CmdExe, PathModificationBehaviour::Append);
insta::assert_snapshot!("test_activation_script_cmd_append", script);
let script = get_script(shell::CmdExe, PathModificationBehaviour::Replace);
insta::assert_snapshot!("test_activation_script_cmd_replace", script);
let script = get_script(shell::CmdExe, PathModificationBehaviour::Prepend);
insta::assert_snapshot!("test_activation_script_cmd_prepend", script);
}

#[test]
#[cfg(unix)]
fn test_activation_script_xonsh() {
let script = get_script(shell::Xonsh);
let script = get_script(shell::Xonsh, PathModificationBehaviour::Append);
insta::assert_snapshot!(script);
}
}
89 changes: 77 additions & 12 deletions crates/rattler_shell/src/shell/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! This module contains the [`Shell`] trait and implementations for various shells.

use crate::activation::PathModificationBehaviour;
use enum_dispatch::enum_dispatch;
use itertools::Itertools;
use std::process::Command;
Expand Down Expand Up @@ -46,9 +47,26 @@ pub trait Shell {
}

/// Set the PATH variable to the given paths.
fn set_path(&self, f: &mut impl Write, paths: &[PathBuf]) -> std::fmt::Result {
let path = std::env::join_paths(paths).unwrap();
self.set_env_var(f, "PATH", path.to_str().unwrap())
fn set_path(
&self,
f: &mut impl Write,
paths: &[PathBuf],
modification_behaviour: PathModificationBehaviour,
) -> std::fmt::Result {
let mut paths_vec = paths
.iter()
.map(|path| path.to_string_lossy().into_owned())
.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")),
}
// Create the shell specific list of paths.
let paths_string = paths_vec.join(self.path_seperator());

self.set_env_var(f, "PATH", paths_string.as_str())
}

/// The extension that shell scripts for this interpreter usually use.
Expand All @@ -59,6 +77,16 @@ pub trait Shell {

/// Constructs a [`Command`] that will execute the specified script by this shell.
fn create_run_script_command(&self, path: &Path) -> Command;

/// Path seperator
fn path_seperator(&self) -> &str {
":"
}

/// Format the environment variable for the shell.
fn format_env_var(&self, var_name: &str) -> String {
format!("${{{var_name}}}")
}
}

/// Convert a native PATH on Windows to a Unix style path usign cygpath.
Expand Down Expand Up @@ -105,16 +133,35 @@ impl Shell for Bash {
writeln!(f, ". \"{}\"", path.to_string_lossy())
}

fn set_path(&self, f: &mut impl Write, paths: &[PathBuf]) -> std::fmt::Result {
let path = std::env::join_paths(paths).unwrap();

// check if we are on Windows, and if yes, convert native path to unix for (Git) Bash
if cfg!(windows) {
let path = native_path_to_unix(path.to_str().unwrap()).unwrap();
return self.set_env_var(f, "PATH", &path);
fn set_path(
&self,
f: &mut impl Write,
paths: &[PathBuf],
modification_behaviour: PathModificationBehaviour,
) -> std::fmt::Result {
// Put paths in a vector of the correct format.
let mut paths_vec = paths
.iter()
.map(|path| {
// check if we are on Windows, and if yes, convert native path to unix for (Git) Bash
if cfg!(windows) {
native_path_to_unix(path.to_string_lossy().as_ref()).unwrap()
} else {
path.to_string_lossy().into_owned()
}
})
.collect_vec();

// 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")),
}
// Create the shell specific list of paths.
let paths_string = paths_vec.join(self.path_seperator());

self.set_env_var(f, "PATH", path.to_str().unwrap())
self.set_env_var(f, "PATH", paths_string.as_str())
}

fn extension(&self) -> &str {
Expand Down Expand Up @@ -241,6 +288,14 @@ impl Shell for CmdExe {
cmd.arg("/D").arg("/C").arg(path);
cmd
}

fn path_seperator(&self) -> &str {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately PowerShell on Unix uses : as path seperator.

";"
}

fn format_env_var(&self, var_name: &str) -> String {
format!("%{var_name}%")
}
}

/// A [`Shell`] implementation for PowerShell.
Expand Down Expand Up @@ -275,6 +330,14 @@ impl Shell for PowerShell {
cmd.arg(path);
cmd
}

fn path_seperator(&self) -> &str {
";"
}

fn format_env_var(&self, var_name: &str) -> String {
format!("$Env:{var_name}")
}
}

/// A [`Shell`] implementation for the Fish shell.
Expand Down Expand Up @@ -453,7 +516,9 @@ impl<T: Shell> ShellScript<T> {

/// Set the PATH environment variable to the given paths.
pub fn set_path(&mut self, paths: &[PathBuf]) -> &mut Self {
self.shell.set_path(&mut self.contents, paths).unwrap();
self.shell
.set_path(&mut self.contents, paths, PathModificationBehaviour::Append)
.unwrap();
self
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
source: crates/rattler_shell/src/activation.rs
expression: script
---
export PATH="__PREFIX__/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin"
export PATH="__PREFIX__/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:${PATH}"
export CONDA_PREFIX="__PREFIX__"
. "__PREFIX__/etc/conda/activate.d/script1.sh"

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
source: crates/rattler_shell/src/activation.rs
expression: script
---
@SET "PATH=__PREFIX__/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin"
@SET "PATH=%PATH%;__PREFIX__/bin;/usr/bin;/bin;/usr/sbin;/sbin;/usr/local/bin"
@SET "CONDA_PREFIX=__PREFIX__"

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
source: crates/rattler_shell/src/activation.rs
expression: script
---
set -gx PATH "__PREFIX__/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin"
set -gx PATH "${PATH}:__PREFIX__/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin"
set -gx CONDA_PREFIX "__PREFIX__"

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
source: crates/rattler_shell/src/activation.rs
expression: script
---
$Env:PATH = "__PREFIX__/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin"
$Env:PATH = "$Env:PATH;__PREFIX__/bin;/usr/bin;/bin;/usr/sbin;/sbin;/usr/local/bin"
$Env:CONDA_PREFIX = "__PREFIX__"

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
source: crates/rattler_shell/src/activation.rs
expression: script
---
$PATH = "__PREFIX__/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin"
$PATH = "${PATH}:__PREFIX__/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin"
$CONDA_PREFIX = "__PREFIX__"
source-bash "__PREFIX__/etc/conda/activate.d/script1.sh"

Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
source: crates/rattler_shell/src/activation.rs
expression: script
---
export PATH="__PREFIX__/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin"
export PATH="${PATH}:__PREFIX__/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin"
export CONDA_PREFIX="__PREFIX__"
. "__PREFIX__/etc/conda/activate.d/script1.sh"

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
source: crates/rattler_shell/src/activation.rs
expression: script
---
export PATH="${PATH}:__PREFIX__/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin"
export CONDA_PREFIX="__PREFIX__"
. "__PREFIX__/etc/conda/activate.d/script1.sh"

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
source: crates/rattler_shell/src/activation.rs
expression: script
---
export PATH="__PREFIX__/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:${PATH}"
export CONDA_PREFIX="__PREFIX__"
. "__PREFIX__/etc/conda/activate.d/script1.sh"

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
source: crates/rattler_shell/src/activation.rs
expression: script
---
export PATH="__PREFIX__/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin"
export CONDA_PREFIX="__PREFIX__"
. "__PREFIX__/etc/conda/activate.d/script1.sh"

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
source: crates/rattler_shell/src/activation.rs
expression: script
---
@SET "PATH=%PATH%;__PREFIX__/bin;/usr/bin;/bin;/usr/sbin;/sbin;/usr/local/bin"
@SET "CONDA_PREFIX=__PREFIX__"

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
source: crates/rattler_shell/src/activation.rs
expression: script
---
@SET "PATH=__PREFIX__/bin;/usr/bin;/bin;/usr/sbin;/sbin;/usr/local/bin;%PATH%"
@SET "CONDA_PREFIX=__PREFIX__"

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
source: crates/rattler_shell/src/activation.rs
expression: script
---
@SET "PATH=__PREFIX__/bin;/usr/bin;/bin;/usr/sbin;/sbin;/usr/local/bin"
@SET "CONDA_PREFIX=__PREFIX__"