Skip to content

Support zsh in call_host.sh #42

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Support zsh in call_host.sh #42

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b3f1d07
Make call_host.sh work with zsh
clelange Oct 29, 2025
6d7d4ec
Fix zsh check to work also when running bash within apptainer started…
clelange Oct 29, 2025
3650254
Update the bash current_funcname helper to return the caller name (FU…
clelange Oct 29, 2025
f93eba4
Add unique path function
clelange Oct 29, 2025
ad93699
Fix shellcheck error SC2163
clelange Oct 29, 2025
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
160 changes: 137 additions & 23 deletions call_host.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/bin/bash
# Print each command before executing it (for debugging)
Copy link
Contributor

@kpedro88 kpedro88 Nov 4, 2025

Choose a reason for hiding this comment

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

is this comment accurate / needed?

Copy link
Author

@clelange clelange Nov 6, 2025

Choose a reason for hiding this comment

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

Sorry, a leftover from debugging, will remove

# shellcheck disable=SC2155,SC2223

# check for configuration
Expand All @@ -8,15 +9,70 @@ if [ -f "$CALL_HOST_CONFIG" ]; then
source "$CALL_HOST_CONFIG"
fi

# zsh / bash compatibility helpers
is_zsh(){
# detect the current shell process name (portable ps usage)
if command -v ps >/dev/null 2>&1; then
# get last path component if ps returns full path
p="$(ps -p $$ -o comm= 2>/dev/null | awk -F/ '{print $NF}')"
case "$p" in
zsh) return 0 ;;
Copy link
Contributor

@kpedro88 kpedro88 Nov 4, 2025

Choose a reason for hiding this comment

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

because there is only a zsh case here, in the bash case, this will proceed through the fallback below before returning non-zero. I would prefer stopping here if bash is directly detected.

Copy link
Author

@clelange clelange Nov 6, 2025

Choose a reason for hiding this comment

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

OK, will add

bash) return 1 ;;

esac
fi

# fallback: check common names for $0 or $ZSH_NAME (login shells may have a leading dash)
case "$(basename -- "${ZSH_NAME:-$0}" 2>/dev/null)" in
zsh|-zsh) return 0 ;;
esac

return 1
}

if is_zsh; then
# export a function to the environment for child shells (zsh)
export_func(){
typeset -fx "$1" 2>/dev/null || true
}
# declare an associative array (zsh) - create a named array using eval so dynamic name works
declare_assoc(){
eval "typeset -A $1"
Copy link
Contributor

@kpedro88 kpedro88 Nov 4, 2025

Choose a reason for hiding this comment

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

check if eval necessary here

}
# get current function name in zsh (be tolerant if indices differ)
current_funcname(){
echo "${funcstack[1]:-${funcstack[0]:-}}"
}
else
# bash
export_func(){
[ -n "$1" ] || return
eval "export -f $1" 2>/dev/null || true
Copy link
Contributor

@kpedro88 kpedro88 Nov 4, 2025

Choose a reason for hiding this comment

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

eval is not necessary here. The shellcheck warning should be handled in either of the following ways:

  1. put # shellcheck disable=SC2163 above this line
  2. use export -f ${1?} as noted at https://www.shellcheck.net/wiki/SC2163

Copy link
Author

@clelange clelange Nov 6, 2025

Choose a reason for hiding this comment

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

Since there's already a check for the existence of ${1}$, I'll go with the former

}
declare_assoc(){
# create named associative array in bash
eval "declare -A $1"
Copy link
Contributor

@kpedro88 kpedro88 Nov 4, 2025

Choose a reason for hiding this comment

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

doesn't seem like eval should be necessary here either

Copy link
Author

@clelange clelange Nov 6, 2025

Choose a reason for hiding this comment

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

I think it's needed because declare expects a literal variable name and not a variable containing the name. Are you able to test this quickly? Otherwise, I can check next week.

Copy link
Contributor

@kpedro88 kpedro88 Nov 6, 2025

Choose a reason for hiding this comment

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

the following works fine for me:

TEST1=TEST2
declare -A $TEST1
TEST2[foo]=bar
echo "${TEST2[@]}"

output:

bar

Copy link
Author

@clelange clelange Nov 6, 2025

Choose a reason for hiding this comment

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

OK, thanks, I'll remove the eval. Just wanted to make sure I'm not breaking the bash part.

}
current_funcname(){
# return the caller function name if available (FUNCNAME[1]), otherwise fall back to FUNCNAME[0]
if [ -n "${FUNCNAME[1]:-}" ]; then
echo "${FUNCNAME[1]}"
else
echo "${FUNCNAME[0]:-}"
fi
}
fi

# validation
call_host_valid(){
VAR_TO_VALIDATE="$1"
# shellcheck disable=SC2076
if [[ ! " enable disable " =~ " ${!VAR_TO_VALIDATE} " ]]; then
echo "Warning: unsupported value ${!VAR_TO_VALIDATE} for $VAR_TO_VALIDATE; disabling"
# retrieve the value of the named variable in a way that works in bash and zsh
VARVAL="$(eval "printf '%s' \"\${$VAR_TO_VALIDATE}\"")"
# check allowed values in a POSIX-compatible way
Copy link
Contributor

@kpedro88 kpedro88 Nov 4, 2025

Choose a reason for hiding this comment

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

zsh really doesn't support =~?

Copy link
Contributor

@kpedro88 kpedro88 Nov 4, 2025

Choose a reason for hiding this comment

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

(the benefit of this construct is scaling: more allowed values can be added without repeating "$VARVAL" !=)

Copy link
Author

@clelange clelange Nov 6, 2025

Choose a reason for hiding this comment

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

It does support it, but it's a bit tricky because of differences in quoting and word-splitting, so this is a more robust implementation. One could switch to case or loop/grep to avoid repeating "$VARVAL" !=. What's your take?

Copy link
Contributor

@kpedro88 kpedro88 Nov 6, 2025

Choose a reason for hiding this comment

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

Case would be okay with me.

if [ "$VARVAL" != "enable" ] && [ "$VARVAL" != "disable" ]; then
echo "Warning: unsupported value $VARVAL for $VAR_TO_VALIDATE; disabling"
eval "export $VAR_TO_VALIDATE=disable"
fi
}
export_func call_host_valid

# default values
: ${CALL_HOST_STATUS:=enable}
Expand Down Expand Up @@ -46,18 +102,41 @@ if [ ! -d "$CALL_HOST_DIR" ]; then
echo "Warning: could not create specified dir CALL_HOST_DIR $CALL_HOST_DIR. disabling"
export CALL_HOST_STATUS=disable
fi
# ensure the pipe dir is bound
export APPTAINER_BIND=${APPTAINER_BIND}${APPTAINER_BIND:+,}${CALL_HOST_DIR}

# helper: add value to a PATH-like variable only if not already present
add_path_unique(){
# args: varname value [sep]
varname="$1"; val="$2"; sep="${3:-:}"
# retrieve current value portably
cur="$(eval "printf '%s' \"\${${varname}:-}\"")"
Copy link
Contributor

@kpedro88 kpedro88 Nov 4, 2025

Choose a reason for hiding this comment

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

maybe should be a function, since it's used repeatedly (also for $VAR_TO_VALIDATE above)

# if empty, set and export
if [ -z "$cur" ]; then
eval "export $varname=\"\$val\""
return
fi
# check for whole-element match using separators to avoid substrings
case "${sep}${cur}${sep}" in
*"${sep}${val}${sep}"*)
# already present
return
;;
esac
# append with separator
eval "export $varname=\"${cur}${sep}${val}\""
}

# ensure the pipe dir is bound (use comma separator for APPTAINER_BIND)
add_path_unique APPTAINER_BIND "$CALL_HOST_DIR" ","

# enable/disable toggles
call_host_enable(){
export CALL_HOST_STATUS=enable
}
export -f call_host_enable
export_func call_host_enable
call_host_disable(){
export CALL_HOST_STATUS=disable
}
export -f call_host_disable
export_func call_host_disable
# single toggle for debug printouts
call_host_debug(){
if [ "$CALL_HOST_DEBUG" = "enable" ]; then
Expand All @@ -66,18 +145,19 @@ call_host_debug(){
export CALL_HOST_DEBUG=enable
fi
}
export -f call_host_debug
export_func call_host_debug
# helper for debug printouts
call_host_debug_print(){
if [ "$CALL_HOST_DEBUG" = "enable" ]; then
echo "$@"
fi
}
export -f call_host_debug_print
export_func call_host_debug_print

call_host_plugin_01(){
# provide htcondor-specific info in container
declare -A CONDOR_OS
# portable associative-array declaration
declare_assoc CONDOR_OS
CONDOR_OS[7]="SL7"
CONDOR_OS[8]="EL8"
CONDOR_OS[9]="EL9"
Expand All @@ -93,7 +173,7 @@ call_host_plugin_01(){
fi
fi
}
export -f call_host_plugin_01
export_func call_host_plugin_01

# concept based on https://stackoverflow.com/questions/32163955/how-to-run-shell-script-on-host-from-docker-container

Expand All @@ -113,7 +193,7 @@ listenhost(){
echo "$tmpexit" > "$3"
done
}
export -f listenhost
export_func listenhost

# creates randomly named pipe and prints the name
makepipe(){
Expand All @@ -122,7 +202,7 @@ makepipe(){
mkfifo "$PIPETMP"
echo "$PIPETMP"
}
export -f makepipe
export_func makepipe

# to be run on host before launching each apptainer session
startpipe(){
Expand All @@ -132,16 +212,19 @@ startpipe(){
# export pipes to apptainer
echo "export APPTAINERENV_HOSTPIPE=$HOSTPIPE; export APPTAINERENV_CONTPIPE=$CONTPIPE; export APPTAINERENV_EXITPIPE=$EXITPIPE"
}
export -f startpipe
export_func startpipe

# sends function to host, then listens for output, and provides exit code from function
call_host(){
# disable ctrl+c to prevent "Interrupted system call"
trap "" SIGINT
if [ "${FUNCNAME[0]}" = "call_host" ]; then

# determine caller function name in a portable way
CURFN="$(current_funcname)"
if [ "$CURFN" = "call_host" ] || [ -z "$CURFN" ]; then
FUNCTMP=
else
FUNCTMP="${FUNCNAME[0]}"
FUNCTMP="$CURFN"
fi

# extra environment settings; set every time because commands are executed on host in subshell
Expand All @@ -152,15 +235,32 @@ call_host(){
cat < "$CONTPIPE"
return "$(cat < "$EXITPIPE")"
}
export -f call_host
export_func call_host

# from https://stackoverflow.com/questions/1203583/how-do-i-rename-a-bash-function
copy_function() {
test -n "$(declare -f "$1")" || return
eval "${_/$1/$2}"
eval "export -f $2"
# portable retrieval of function source and re-definition under a new name
if is_zsh; then
# zsh: use `functions` to get the definition
if functions "$1" >/dev/null 2>&1; then
fnsrc="$(functions "$1" 2>/dev/null)"
else
return
fi
else
# bash: use declare -f
if declare -f "$1" >/dev/null 2>&1; then
fnsrc="$(declare -f "$1")"
else
return
fi
fi
Comment on lines +242 to +257
Copy link
Contributor

@kpedro88 kpedro88 Nov 4, 2025

Choose a reason for hiding this comment

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

this is fairly repetitive. suggestion:

  1. make get_function() functions for both bash and zsh (using declare -f and functions, resp.) in the "compatibility helpers" section at the top
  2. here, just call: 
    fnsrc="$(get_function "$1")"
if [ -z "$fnsrc" ]; then return; fi
    then the remaining lines below

# replace the function name in the source and eval it to create new function
fnnew="$(printf '%s\n' "$fnsrc" | sed "s/^$1\\b/$2/")"
eval "$fnnew"
export_func "$2"
}
export -f copy_function
export_func copy_function

if [ -z "$APPTAINER_ORIG" ]; then
export APPTAINER_ORIG=$(which apptainer)
Expand Down Expand Up @@ -198,11 +298,25 @@ apptainer(){
)
fi
}
export -f apptainer
export_func apptainer

# on host: get list of condor executables
if [ -z "$APPTAINER_CONTAINER" ]; then
export APPTAINERENV_HOSTFNS=$(compgen -c | grep '^condor_\|^eos')
# portable command list discovery:
if command -v compgen >/dev/null 2>&1; then
export APPTAINERENV_HOSTFNS=$(compgen -c | grep -E '^condor_|^eos' | tr '\n' ' ')
else
# fallback: scan PATH for matching executables (portable)
APPTAINERENV_HOSTFNS="$( ( IFS=:
for d in $PATH; do
[ -d "$d" ] || continue
for f in "$d"/condor_* "$d"/eos*; do
Copy link
Contributor

@kpedro88 kpedro88 Nov 4, 2025

Choose a reason for hiding this comment

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

would prefer to avoid repetition here (specifying condor and eos twice, since the list could grow in the future). put these in an array, then either create the grep expression (first case) or use a nested loop (second case)?

[ -x "$f" ] && basename "$f"
done
done ) | sort -u | tr '\n' ' ')"
export APPTAINERENV_HOSTFNS
fi

if [ -n "$CALL_HOST_USERFNS" ]; then
export APPTAINERENV_HOSTFNS="$APPTAINERENV_HOSTFNS $CALL_HOST_USERFNS"
fi
Expand Down