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.

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

Jump to bottom

Use POSIX strict mode #304

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Use POSIX strict mode #304

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4f27e5a
Use POSIX strict mode
RichiH Jun 23, 2021
c154a99
In which the need to handle $VCSH_REPO_NAME explictly becomes clear
RichiH Jun 23, 2021
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
35 changes: 26 additions & 9 deletions vcsh.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,24 @@
# which is admittedly extremely unlikely to the point of being impossible,
# this software will most likely follow suit.

# This should always be the first line of code to facilitate debugging
# POSIX strict mode; set -eu would be the other way to write this
set -o errexit
set -o nounset
IFS=$(printf '\n\t')

# 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<N>-g<SHA>' 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
Expand All @@ -36,6 +44,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)
Expand Down Expand Up @@ -593,7 +602,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;;
Expand Down Expand Up @@ -682,7 +691,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
Expand All @@ -695,13 +704,19 @@ 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

# 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
Expand All @@ -727,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-}" ] \
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will only suceed by a somewhat odd side effect, -r tests specifically for files not directories so it will fail if the values is unset and the path is only to the directory. While the result is correct, the code logic is pretty opaque.

As the assorted comments here note, the active repo(s) name handling should probably be overhauled entirely.

&& . "$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
Expand Down