QA: Add shellcheck and fix/ignore all issues.

This was inspired by and built upon Dan's PR here: pimoroni/grow-python#37

Co-authored-by: Dan Webb <dan.webb@damacus.io>

.github/workflows/qa.yml

@@ -33,3 +33,7 @@ jobs:
       - name: Run Code Checks
         run: |
           make check
+
+      - name: Run Bash Code Checks
+        run: |
+          make shellcheck

Makefile

@@ -30,11 +30,14 @@ uninstall:
 
 dev-deps:
 	python3 -m pip install -r requirements-dev.txt
-	sudo apt install dos2unix
+	sudo apt install dos2unix shellcheck
 
 check:
 	@bash check.sh
 
+shellcheck:
+	shellcheck *.sh
+
 qa:
 	tox -e qa
 

check.sh

@@ -3,9 +3,9 @@
 # This script handles some basic QA checks on the source
 
 NOPOST=$1
-LIBRARY_NAME=`hatch project metadata name`
-LIBRARY_VERSION=`hatch version | awk -F "." '{print $1"."$2"."$3}'`
-POST_VERSION=`hatch version | awk -F "." '{print substr($4,0,length($4))}'`
+LIBRARY_NAME=$(hatch project metadata name)
+LIBRARY_VERSION=$(hatch version | awk -F "." '{print $1"."$2"."$3}')
+POST_VERSION=$(hatch version | awk -F "." '{print substr($4,0,length($4))}')
 TERM=${TERM:="xterm-256color"}
 
 success() {
@@ -29,7 +29,7 @@ while [[ $# -gt 0 ]]; do
 		;;
 	*)
 		if [[ $1 == -* ]]; then
-			printf "Unrecognised option: $1\n";
+			printf "Unrecognised option: %s\n" "$1";
 			exit 1
 		fi
 		POSITIONAL_ARGS+=("$1")
@@ -40,8 +40,7 @@ done
 inform "Checking $LIBRARY_NAME $LIBRARY_VERSION\n"
 
 inform "Checking for trailing whitespace..."
-grep -IUrn --color "[[:blank:]]$" --exclude-dir=dist --exclude-dir=.tox --exclude-dir=.git --exclude=PKG-INFO
-if [[ $? -eq 0 ]]; then
+if grep -IUrn --color "[[:blank:]]$" --exclude-dir=dist --exclude-dir=.tox --exclude-dir=.git --exclude=PKG-INFO; then
     warning "Trailing whitespace found!"
     exit 1
 else
@@ -50,8 +49,7 @@ fi
 printf "\n"
 
 inform "Checking for DOS line-endings..."
-grep -lIUrn --color $'\r' --exclude-dir=dist --exclude-dir=.tox --exclude-dir=.git --exclude=Makefile
-if [[ $? -eq 0 ]]; then
+if grep -lIUrn --color $'\r' --exclude-dir=dist --exclude-dir=.tox --exclude-dir=.git --exclude=Makefile; then
     warning "DOS line-endings found!"
     exit 1
 else
@@ -60,8 +58,7 @@ fi
 printf "\n"
 
 inform "Checking CHANGELOG.md..."
-cat CHANGELOG.md | grep ^${LIBRARY_VERSION} > /dev/null 2>&1
-if [[ $? -eq 1 ]]; then
+if ! grep "^${LIBRARY_VERSION}" CHANGELOG.md > /dev/null 2>&1; then
     warning "Changes missing for version ${LIBRARY_VERSION}! Please update CHANGELOG.md."
     exit 1
 else
@@ -70,8 +67,7 @@ fi
 printf "\n"
 
 inform "Checking for git tag ${LIBRARY_VERSION}..."
-git tag -l | grep -E "${LIBRARY_VERSION}$"
-if [[ $? -eq 1 ]]; then
+if ! git tag -l | grep -E "${LIBRARY_VERSION}$"; then
     warning "Missing git tag for version ${LIBRARY_VERSION}"
 fi
 printf "\n"

install.sh

@@ -1,14 +1,13 @@
 #!/bin/bash
-LIBRARY_NAME=`grep -m 1 name pyproject.toml | awk -F" = " '{print substr($2,2,length($2)-2)}'`
+LIBRARY_NAME=$(grep -m 1 name pyproject.toml | awk -F" = " '{print substr($2,2,length($2)-2)}')
 CONFIG_FILE=config.txt
 CONFIG_DIR="/boot/firmware"
-DATESTAMP=`date "+%Y-%m-%d-%H-%M-%S"`
+DATESTAMP=$(date "+%Y-%m-%d-%H-%M-%S")
 CONFIG_BACKUP=false
 APT_HAS_UPDATED=false
 RESOURCES_TOP_DIR="$HOME/Pimoroni"
 VENV_BASH_SNIPPET="$RESOURCES_TOP_DIR/auto_venv.sh"
 VENV_DIR="$HOME/.virtualenvs/pimoroni"
-WD=`pwd`
 USAGE="./install.sh (--unstable)"
 POSITIONAL_ARGS=()
 FORCE=false
@@ -18,7 +17,7 @@ CMD_ERRORS=false
 
 
 user_check() {
-	if [ $(id -u) -eq 0 ]; then
+	if [ "$(id -u)" -eq 0 ]; then
 		fatal "Script should not be run as root. Try './install.sh'\n"
 	fi
 }
@@ -36,15 +35,6 @@ confirm() {
 	fi
 }
 
-prompt() {
-	read -r -p "$1 [y/N] " response < /dev/tty
-	if [[ $response =~ ^(yes|y|Y)$ ]]; then
-		true
-	else
-		false
-	fi
-}
-
 success() {
 	echo -e "$(tput setaf 2)$1$(tput sgr0)"
 }
@@ -79,10 +69,10 @@ find_config() {
 
 venv_bash_snippet() {
 	inform "Checking for $VENV_BASH_SNIPPET\n"
-	if [ ! -f $VENV_BASH_SNIPPET ]; then
+	if [ ! -f "$VENV_BASH_SNIPPET" ]; then
 		inform "Creating $VENV_BASH_SNIPPET\n"
-		mkdir -p $RESOURCES_TOP_DIR
-		cat << EOF > $VENV_BASH_SNIPPET
+		mkdir -p "$RESOURCES_TOP_DIR"
+		cat << EOF > "$VENV_BASH_SNIPPET"
 # Add "source $VENV_BASH_SNIPPET" to your ~/.bashrc to activate
 # the Pimoroni virtual environment automagically!
 VENV_DIR="$VENV_DIR"
@@ -98,21 +88,23 @@ EOF
 }
 
 venv_check() {
-	PYTHON_BIN=`which $PYTHON`
+	PYTHON_BIN=$(which "$PYTHON")
 	if [[ $VIRTUAL_ENV == "" ]] || [[ $PYTHON_BIN != $VIRTUAL_ENV* ]]; then
 		printf "This script should be run in a virtual Python environment.\n"
 		if confirm "Would you like us to create and/or use a default one?"; then
 			printf "\n"
-			if [ ! -f $VENV_DIR/bin/activate ]; then
+			if [ ! -f "$VENV_DIR/bin/activate" ]; then
 				inform "Creating a new virtual Python environment in $VENV_DIR, please wait...\n"
-				mkdir -p $VENV_DIR
-				/usr/bin/python3 -m venv $VENV_DIR --system-site-packages
+				mkdir -p "$VENV_DIR"
+				/usr/bin/python3 -m venv "$VENV_DIR" --system-site-packages
 				venv_bash_snippet
-				source $VENV_DIR/bin/activate
+				# shellcheck disable=SC1091
+				source "$VENV_DIR/bin/activate"
 			else
 				inform "Activating existing virtual Python environment in $VENV_DIR\n"
-				printf "source $VENV_DIR/bin/activate\n"
-				source $VENV_DIR/bin/activate
+				printf "source \"%s/bin/activate\"\n" "$VENV_DIR"
+				# shellcheck disable=SC1091
+				source "$VENV_DIR/bin/activate"
 			fi
 		else
 			printf "\n"
@@ -134,40 +126,40 @@ function do_config_backup {
 		CONFIG_BACKUP=true
 		FILENAME="config.preinstall-$LIBRARY_NAME-$DATESTAMP.txt"
 		inform "Backing up $CONFIG_DIR/$CONFIG_FILE to $CONFIG_DIR/$FILENAME\n"
-		sudo cp $CONFIG_DIR/$CONFIG_FILE $CONFIG_DIR/$FILENAME
-		mkdir -p $RESOURCES_TOP_DIR/config-backups/
-		cp $CONFIG_DIR/$CONFIG_FILE $RESOURCES_TOP_DIR/config-backups/$FILENAME
+		sudo cp "$CONFIG_DIR/$CONFIG_FILE $CONFIG_DIR/$FILENAME"
+		mkdir -p "$RESOURCES_TOP_DIR/config-backups/"
+		cp $CONFIG_DIR/$CONFIG_FILE "$RESOURCES_TOP_DIR/config-backups/$FILENAME"
 		if [ -f "$UNINSTALLER" ]; then
-			echo "cp $RESOURCES_TOP_DIR/config-backups/$FILENAME $CONFIG_DIR/$CONFIG_FILE" >> $UNINSTALLER
+			echo "cp $RESOURCES_TOP_DIR/config-backups/$FILENAME $CONFIG_DIR/$CONFIG_FILE" >> "$UNINSTALLER"
 		fi
 	fi
 }
 
 function apt_pkg_install {
-	PACKAGES=()
+	PACKAGES_NEEDED=()
 	PACKAGES_IN=("$@")
 	# Check the list of packages and only run update/install if we need to
 	for ((i = 0; i < ${#PACKAGES_IN[@]}; i++)); do
 		PACKAGE="${PACKAGES_IN[$i]}"
 		if [ "$PACKAGE" == "" ]; then continue; fi
-		printf "Checking for $PACKAGE\n"
-		dpkg -L $PACKAGE > /dev/null 2>&1
+		printf "Checking for %s\n" "$PACKAGE"
+		dpkg -L "$PACKAGE" > /dev/null 2>&1
 		if [ "$?" == "1" ]; then
-			PACKAGES+=("$PACKAGE")
+			PACKAGES_NEEDED+=("$PACKAGE")
 		fi
 	done
-	PACKAGES="${PACKAGES[@]}"
+	PACKAGES="${PACKAGES_NEEDED[*]}"
 	if ! [ "$PACKAGES" == "" ]; then
 		printf "\n"
 		inform "Installing missing packages: $PACKAGES"
 		if [ ! $APT_HAS_UPDATED ]; then
 			sudo apt update
 			APT_HAS_UPDATED=true
 		fi
-		sudo apt install -y $PACKAGES
+		sudo apt install -y "$PACKAGES"
 		check_for_error
 		if [ -f "$UNINSTALLER" ]; then
-			echo "apt uninstall -y $PACKAGES" >> $UNINSTALLER
+			echo "apt uninstall -y $PACKAGES" >> "$UNINSTALLER"
 		fi
 	fi
 }
@@ -196,33 +188,34 @@ while [[ $# -gt 0 ]]; do
 		;;
 	*)
 		if [[ $1 == -* ]]; then
-			printf "Unrecognised option: $1\n";
-			printf "Usage: $USAGE\n";
+			printf "Unrecognised option: %s\n" "$1";
+			printf "Usage: %s\n" "$USAGE";
 			exit 1
 		fi
 		POSITIONAL_ARGS+=("$1")
 		shift
 	esac
 done
 
-printf "Installing $LIBRARY_NAME...\n\n"
+printf "Installing %s...\n\n" "$LIBRARY_NAME"
 
 user_check
 venv_check
 
-if [ ! -f `which $PYTHON` ]; then
-	fatal "Python path $PYTHON not found!\n"
+if [ ! -f "$(which "$PYTHON")" ]; then
+	fatal "Python path %s not found!\n" "$PYTHON"
 fi
 
-PYTHON_VER=`$PYTHON --version`
+PYTHON_VER=$($PYTHON --version)
 
 inform "Checking Dependencies. Please wait..."
 
 # Install toml and try to read pyproject.toml into bash variables
 
 pip_pkg_install toml
 
-CONFIG_VARS=`$PYTHON - <<EOF
+CONFIG_VARS=$(
+	$PYTHON - <<EOF
 import toml
 config = toml.load("pyproject.toml")
 github_url = config['project']['urls']['GitHub']
@@ -237,32 +230,34 @@ APT_PACKAGES={apt_packages}
 SETUP_CMDS={commands}
 CONFIG_TXT={configtxt}
 """.format(**p))
-EOF`
+EOF
+)
 
+# shellcheck disable=SC2181 # Inlining the above command would be messy
 if [ $? -ne 0 ]; then
 	# This is bad, this should not happen in production!
 	fatal "Error parsing configuration...\n"
 fi
 
-eval $CONFIG_VARS
+eval "$CONFIG_VARS"
 
 RESOURCES_DIR=$RESOURCES_TOP_DIR/$LIBRARY_NAME
 UNINSTALLER=$RESOURCES_DIR/uninstall.sh
 
-RES_DIR_OWNER=`stat -c "%U" $RESOURCES_TOP_DIR`
+RES_DIR_OWNER=$(stat -c "%U" "$RESOURCES_TOP_DIR")
 
 # Previous install.sh scripts were run as root with sudo, which caused
 # the ~/Pimoroni dir to be created with root ownership. Try and fix it.
 if [[ "$RES_DIR_OWNER" == "root" ]]; then
 	warning "\n\nFixing $RESOURCES_TOP_DIR permissions!\n\n"
-	sudo chown -R $USER:$USER $RESOURCES_TOP_DIR
+	sudo chown -R "$USER:$USER" "$RESOURCES_TOP_DIR"
 fi
 
-mkdir -p $RESOURCES_DIR
+mkdir -p "$RESOURCES_DIR"
 
 # Create a stub uninstaller file, we'll try to add the inverse of every
 # install command run to here, though it's not complete.
-cat << EOF > $UNINSTALLER
+cat << EOF > "$UNINSTALLER"
 printf "It's recommended you run these steps manually.\n"
 printf "If you want to run the full script, open it in\n"
 printf "an editor and remove 'exit 1' from below.\n"
@@ -284,16 +279,15 @@ if $UNSTABLE; then
 	pip_pkg_install .
 else
 	inform "Installing stable library from pypi.\n"
-	pip_pkg_install $LIBRARY_NAME
+	pip_pkg_install "$LIBRARY_NAME"
 fi
 
+# shellcheck disable=SC2181 # One of two commands run, depending on --unstable flag
 if [ $? -eq 0 ]; then
 	success "Done!\n"
-	echo "$PYTHON -m pip uninstall $LIBRARY_NAME" >> $UNINSTALLER
+	echo "$PYTHON -m pip uninstall $LIBRARY_NAME" >> "$UNINSTALLER"
 fi
 
-cd $WD
-
 find_config
 
 # Run the setup commands from pyproject.toml / tool.pimoroni.commands
@@ -304,7 +298,7 @@ for ((i = 0; i < ${#SETUP_CMDS[@]}; i++)); do
 	if [[ "$CMD" == *"raspi-config"* ]] || [[ "$CMD" == *"$CONFIG_DIR/$CONFIG_FILE"* ]] || [[ "$CMD" == *"\$CONFIG_DIR/\$CONFIG_FILE"* ]]; then
 		do_config_backup
 	fi
-	eval $CMD
+	eval "$CMD"
 	check_for_error
 done
 
@@ -319,7 +313,7 @@ for ((i = 0; i < ${#CONFIG_TXT[@]}; i++)); do
 		inform "Adding $CONFIG_LINE to $CONFIG_DIR/$CONFIG_FILE"
 		sudo sed -i "s/^#$CONFIG_LINE/$CONFIG_LINE/" $CONFIG_DIR/$CONFIG_FILE
 		if ! grep -q "^$CONFIG_LINE" $CONFIG_DIR/$CONFIG_FILE; then
-			printf "$CONFIG_LINE\n" | sudo tee --append $CONFIG_DIR/$CONFIG_FILE
+			printf "%s \n" "$CONFIG_LINE" | sudo tee --append $CONFIG_DIR/$CONFIG_FILE
 		fi
 	fi
 done
@@ -331,8 +325,8 @@ printf "\n"
 if [ -d "examples" ]; then
 	if confirm "Would you like to copy examples to $RESOURCES_DIR?"; then
 		inform "Copying examples to $RESOURCES_DIR"
-		cp -r examples/ $RESOURCES_DIR
-		echo "rm -r $RESOURCES_DIR" >> $UNINSTALLER
+		cp -r examples/ "$RESOURCES_DIR"
+		echo "rm -r $RESOURCES_DIR" >> "$UNINSTALLER"
 		success "Done!"
 	fi
 fi
@@ -345,8 +339,7 @@ if confirm "Would you like to generate documentation?"; then
 	inform "Installing pdoc. Please wait..."
 	pip_pkg_install pdoc
 	inform "Generating documentation.\n"
-	$PYTHON -m pdoc $LIBRARY_NAME -o $RESOURCES_DIR/docs > /dev/null
-	if [ $? -eq 0 ]; then
+	if $PYTHON -m pdoc "$LIBRARY_NAME" -o "$RESOURCES_DIR/docs" > /dev/null; then
 		inform "Documentation saved to $RESOURCES_DIR/docs"
 		success "Done!"
 	else
@@ -360,13 +353,13 @@ if [ "$CMD_ERRORS" = true ]; then
 	warning "One or more setup commands appear to have failed."
 	printf "This might prevent things from working properly.\n"
 	printf "Make sure your OS is up to date and try re-running this installer.\n"
-	printf "If things still don't work, report this or find help at $GITHUB_URL.\n\n"
+	printf "If things still don't work, report this or find help at %s.\n\n" "$GITHUB_URL"
 else
 	success "\nAll done!"
 fi
 
 printf "If this is your first time installing you should reboot for hardware changes to take effect.\n"
-printf "Find uninstall steps in $UNINSTALLER\n\n"
+printf "Find uninstall steps in %s\n\n" "$UNINSTALLER"
 
 if [ "$CMD_ERRORS" = true ]; then
 	exit 1