From d05f2b258b8ef8823ca1ca9496984caa4c16c686 Mon Sep 17 00:00:00 2001 From: Pavel Dikov <66369686+paveldikov@users.noreply.github.com> Date: Wed, 31 Jul 2024 17:18:11 +0100 Subject: [PATCH] fix(venv): make relocatable activation scripts support ksh (#5640) It transpires that detecting the directory a script was sourced from is non-trivial across `bash`, `ksh` and `zsh`. The previous version was a one-liner and supported `bash` and `zsh` but not `ksh`. It is possible to keep the one-liner and add `ksh` support, but that is mutually-exclusive with `zsh`. Therefore, the only way to square this circle is to add an `if` block. A silver lining here is that although longer, the script is probably easier to follow as there is less code-golfing going on. --- crates/uv-shell/src/lib.rs | 12 +++++++++++- crates/uv-virtualenv/src/activator/activate | 16 ++++++++++++---- crates/uv-virtualenv/src/virtualenv.rs | 2 +- crates/uv/src/commands/venv.rs | 2 +- crates/uv/tests/venv.rs | 2 +- 5 files changed, 26 insertions(+), 8 deletions(-) diff --git a/crates/uv-shell/src/lib.rs b/crates/uv-shell/src/lib.rs index 06a05517d..a5722a546 100644 --- a/crates/uv-shell/src/lib.rs +++ b/crates/uv-shell/src/lib.rs @@ -21,6 +21,8 @@ pub enum Shell { Nushell, /// C SHell (csh) Csh, + /// Korn SHell (ksh) + Ksh, } impl Shell { @@ -43,6 +45,8 @@ impl Shell { Some(Shell::Bash) } else if std::env::var_os("ZSH_VERSION").is_some() { Some(Shell::Zsh) + } else if std::env::var_os("KSH_VERSION").is_some() { + Some(Shell::Ksh) } else if let Some(env_shell) = std::env::var_os("SHELL") { Shell::from_shell_path(env_shell) } else if cfg!(windows) { @@ -99,6 +103,10 @@ impl Shell { home_dir.join(".bashrc"), ] } + Shell::Ksh => { + // On Ksh it's standard POSIX `.profile` for login shells, and `.kshrc` for non-login. + vec![home_dir.join(".profile"), home_dir.join(".kshrc")] + } Shell::Zsh => { // On Zsh, we only need to update `.zshenv`. This file is sourced for both login and // non-login shells. However, we match rustup's logic for determining _which_ @@ -174,7 +182,7 @@ impl Shell { pub fn prepend_path(self, path: &Path) -> Option { match self { Shell::Nushell => None, - Shell::Bash | Shell::Zsh => Some(format!( + Shell::Bash | Shell::Zsh | Shell::Ksh => Some(format!( "export PATH=\"{}:$PATH\"", backslash_escape(&path.simplified_display().to_string()), )), @@ -208,6 +216,7 @@ impl std::fmt::Display for Shell { Shell::Zsh => write!(f, "Zsh"), Shell::Nushell => write!(f, "Nushell"), Shell::Csh => write!(f, "Csh"), + Shell::Ksh => write!(f, "Ksh"), } } } @@ -220,6 +229,7 @@ fn parse_shell_from_path(path: &Path) -> Option { "zsh" => Some(Shell::Zsh), "fish" => Some(Shell::Fish), "csh" => Some(Shell::Csh), + "ksh" => Some(Shell::Ksh), "powershell" | "powershell_ise" => Some(Shell::Powershell), _ => None, } diff --git a/crates/uv-virtualenv/src/activator/activate b/crates/uv-virtualenv/src/activator/activate index b026a2157..b6a128332 100644 --- a/crates/uv-virtualenv/src/activator/activate +++ b/crates/uv-virtualenv/src/activator/activate @@ -22,10 +22,18 @@ # This file must be used with "source bin/activate" *from bash* # you cannot run it directly - -if [ "${BASH_SOURCE-}" = "$0" ]; then - echo "You must source this script: \$ source $0" >&2 - exit 33 +# Get script path (only used if environment is relocatable). +if [ -n "${BASH_VERSION:+x}" ] ; then + SCRIPT_PATH="${BASH_SOURCE[0]}" + if [ "$SCRIPT_PATH" = "$0" ]; then + # Only bash has a reasonably robust check for source'dness. + echo "You must source this script: \$ source $0" >&2 + exit 33 + fi +elif [ -n "${ZSH_VERSION:+x}" ] ; then + SCRIPT_PATH="${(%):-%x}" +elif [ -n "${KSH_VERSION:+x}" ] ; then + SCRIPT_PATH="${.sh.file}" fi deactivate () { diff --git a/crates/uv-virtualenv/src/virtualenv.rs b/crates/uv-virtualenv/src/virtualenv.rs index fbcf3688d..08d09b619 100644 --- a/crates/uv-virtualenv/src/virtualenv.rs +++ b/crates/uv-virtualenv/src/virtualenv.rs @@ -298,7 +298,7 @@ pub(crate) fn create( (true, "activate") => { // Extremely verbose, but should cover all major POSIX shells, // as well as platforms where `readlink` does not implement `-f`. - r#"'"$(dirname -- "$(CDPATH= cd -- "$(dirname -- ${BASH_SOURCE[0]:-${(%):-%x}})" && echo "$PWD")")"'"# + r#"'"$(dirname -- "$(CDPATH= cd -- "$(dirname -- "$SCRIPT_PATH")" > /dev/null && echo "$PWD")")"'"# } (true, "activate.bat") => r"%~dp0..", (true, "activate.fish") => { diff --git a/crates/uv/src/commands/venv.rs b/crates/uv/src/commands/venv.rs index 47eb6fbe1..f5e1786f1 100644 --- a/crates/uv/src/commands/venv.rs +++ b/crates/uv/src/commands/venv.rs @@ -331,7 +331,7 @@ async fn venv_impl( // Determine the appropriate activation command. let activation = match Shell::from_env() { None => None, - Some(Shell::Bash | Shell::Zsh) => Some(format!( + Some(Shell::Bash | Shell::Zsh | Shell::Ksh) => Some(format!( "source {}", shlex_posix(venv.scripts().join("activate")) )), diff --git a/crates/uv/tests/venv.rs b/crates/uv/tests/venv.rs index fef4d8200..762af358b 100644 --- a/crates/uv/tests/venv.rs +++ b/crates/uv/tests/venv.rs @@ -794,7 +794,7 @@ fn verify_pyvenv_cfg_relocatable() { let activate_sh = scripts.child("activate"); activate_sh.assert(predicates::path::is_file()); - activate_sh.assert(predicates::str::contains(r#"VIRTUAL_ENV=''"$(dirname -- "$(CDPATH= cd -- "$(dirname -- ${BASH_SOURCE[0]:-${(%):-%x}})" && echo "$PWD")")"''"#)); + activate_sh.assert(predicates::str::contains(r#"VIRTUAL_ENV=''"$(dirname -- "$(CDPATH= cd -- "$(dirname -- "$SCRIPT_PATH")" > /dev/null && echo "$PWD")")"''"#)); let activate_bat = scripts.child("activate.bat"); activate_bat.assert(predicates::path::is_file());