From 4f27e5acabbb416e771a6b00544ea0e3a99cdfd0 Mon Sep 17 00:00:00 2001 From: Richard Hartmann Date: Wed, 23 Jun 2021 10:54:22 +0200 Subject: [PATCH 1/2] Use POSIX strict mode Fixes https://github.com/RichiH/vcsh/issues/303 Signed-off-by: Richard Hartmann --- vcsh.in | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/vcsh.in b/vcsh.in index 5e35ec93..7a79b7eb 100755 --- a/vcsh.in +++ b/vcsh.in @@ -12,7 +12,13 @@ # which is admittedly extremely unlikely to the point of being impossible, # this software will most likely follow suit. +# POSIX strict mode; set -eu would be the other way to write this +set -o errexit +set -o nounset +IFS=$(printf '\n\t') + # This should always be the first line of code to facilitate debugging +VCSH_DEBUG=''; export VCSH_DEBUG [ -n "$VCSH_DEBUG" ] && set -vx @@ -36,6 +42,7 @@ fatal() { # options that will modify our behaviour. # Commands are handled at the end of this script. # shellcheck disable=SC2220 +VCSH_OPTION_CONFIG=''; export VCSH_OPTION_CONFIG while getopts c:dv flag; do case "$flag" in d) @@ -593,7 +600,7 @@ if [ ! "x$VCSH_GITIGNORE" = 'xexact' ] && [ ! "x$VCSH_GITIGNORE" = 'xnone' ] && fatal "'\$VCSH_GITIGNORE' must equal 'exact', 'none', or 'recursive'" 1 fi -VCSH_COMMAND=$1; export VCSH_COMMAND +VCSH_COMMAND=${1-}; export VCSH_COMMAND case $VCSH_COMMAND in clon|clo|cl) VCSH_COMMAND=clone;; @@ -682,7 +689,7 @@ elif [ x"$VCSH_COMMAND" = x'status' ]; then shift fi VCSH_REPO_NAME=$2; export VCSH_REPO_NAME -elif [ -n "$2" ]; then +elif [ -n "${2-}" ]; then VCSH_COMMAND='run'; export VCSH_COMMAND VCSH_REPO_NAME=$1; export VCSH_REPO_NAME GIT_DIR=$VCSH_REPO_D/$VCSH_REPO_NAME.git; export GIT_DIR @@ -695,7 +702,8 @@ elif [ -n "$VCSH_COMMAND" ]; then GIT_DIR=$VCSH_REPO_D/$VCSH_REPO_NAME.git; export GIT_DIR [ -d "$GIT_DIR" ] || { help; exit 1; } else - # $1 is empty + # $1 is empty. We exit 1 to discern from `vcsh help` and to help catch + # scripts which erroueously don't provide options help && exit 1 fi From c154a9992583b025433b058cd6e074f78356342a Mon Sep 17 00:00:00 2001 From: Richard Hartmann Date: Wed, 23 Jun 2021 12:40:55 +0200 Subject: [PATCH 2/2] In which the need to handle $VCSH_REPO_NAME explictly becomes clear `set -o nounset` made it clear that my flow control is really really bad and relies heavily on $VCSH_REPO_NAME transporting information both through its value and by being set or not. That will need to be cleaned up. NB: It might be an option to set it to empty as a global and leave it as that, but that seems icky and brittle. Signed-off-by: Richard Hartmann --- vcsh.in | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/vcsh.in b/vcsh.in index 7a79b7eb..ed0fe18c 100755 --- a/vcsh.in +++ b/vcsh.in @@ -17,17 +17,19 @@ set -o errexit set -o nounset IFS=$(printf '\n\t') -# This should always be the first line of code to facilitate debugging +# This block should always come as early as possible to facilitate debugging VCSH_DEBUG=''; export VCSH_DEBUG [ -n "$VCSH_DEBUG" ] && set -vx +# Initialize globals # If '.r-g' is appended to the version, you are seeing an unreleased # version built from the main branch HEAD. If "-standalone" is appended you are # using a version built explicitly for portability as a standalone script # rather than being installed to a specific system. VCSH_VERSION='@VERSION@@DEPLOYMENT@'; export VCSH_VERSION VCSH_SELF='@TRANSFORMED_PACKAGE_NAME@'; export VCSH_SELF +VCSH_VERBOSE=''; export VCSH_VERBOSE # Ensure all files created are accessible only to the current user. umask 0077 @@ -707,9 +709,14 @@ else help && exit 1 fi -# Did we receive a directory instead of a name? -# Mangle the input to fit normal operation. -if echo "$VCSH_REPO_NAME" | @GREP@ -q '/'; then +# Detect and handle if $VCSH_REPO_NAME is set to a directory instead of a repo. +# TODO: We rely on $VCSH_REPO_NAME being set which implicitly transports if it +# should be set, then then we ${-} that check away to make `setopt -o nounset` +# pass. That seems potentially brittle and against the intention of nounset; +# We may need to make it explicit if we should anticipate $VCSH_REPO_NAME being +# set here. At which point it would make sense to do the check for a directory +# earlier and move this all into a helper function. +if echo "${VCSH_REPO_NAME-}" | @GREP@ -q '/'; then GIT_DIR=$VCSH_REPO_NAME; export GIT_DIR VCSH_REPO_NAME=$(basename "$VCSH_REPO_NAME" .git); export VCSH_REPO_NAME fi @@ -735,11 +742,13 @@ VCSH_COMMAND=$(echo "$VCSH_COMMAND" | @SED@ 's/-/_/g'); export VCSH_COMMAND # Source repo-specific configuration file # shellcheck source=/dev/null -[ -r "$XDG_CONFIG_HOME/vcsh/config.d/$VCSH_REPO_NAME" ] \ +# TODO: And we rely on $VCSH_REPO_NAME implictly again +[ -r "$XDG_CONFIG_HOME/vcsh/config.d/${VCSH_REPO_NAME-}" ] \ && . "$XDG_CONFIG_HOME/vcsh/config.d/$VCSH_REPO_NAME" # source overlay functions -for overlay in "$VCSH_OVERLAY_D/$VCSH_COMMAND"* "$VCSH_OVERLAY_D/$VCSH_REPO_NAME.$VCSH_COMMAND"*; do +# TODO: And we rely on $VCSH_REPO_NAME implictly again +for overlay in "$VCSH_OVERLAY_D/$VCSH_COMMAND"* "$VCSH_OVERLAY_D/${VCSH_REPO_NAME-}.$VCSH_COMMAND"*; do [ -r "$overlay" ] || continue info "sourcing '$overlay'" # shellcheck source=/dev/null