Shellcheck scripts in root folder (#3090)

* Fix Shellcheck: Add -r flag to read

read will mangle backslashes without it. These are not expected
characters so not explicitly necessary, but also won't hurt.

* Fix Shellcheck SC2086: Double quote variables

* Fix Shellcheck SC2004: dollar sign unnecessary on arithmetic vars

* Fix Shellcheck SC2129: use curly braces to >> file

* Disable shellcheck on sourced venv/bin/activate

* Fix Shellcheck SC1004

Simplify sed-expression to be a single line only removing the need for
the offending / and linefeed

* Fix Shellcheck SC2046 when finding repo root

Splits the check into to files to be able to quote the expressions
in a good way.

* Fix Shellcheck SC2086 Unquoted apt packagaes

This converts the variable APT_PACKAGE_LIST from a string to an array.
This is a safer way to handle arguments according to BashFAQ
(http://mywiki.wooledge.org/BashFAQ/050)

* Fix Shellcheck SC2230 by changing which to command -v

* CICD: Activate shellcheck job for dev_setup.sh

* Shellcheck start-mycroft.sh

This fixes the following shellcheck issues:
- https://www.shellcheck.net/wiki/SC2164 -- Use 'cd ... || exit' or 'cd ... |...
- https://www.shellcheck.net/wiki/SC2206 -- Quote to prevent word splitting/g...
- https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...
- https://www.shellcheck.net/wiki/SC2124 -- Assigning an array to a string! A...

The command is added to the CICD run of shellcheck

* Shellcheck stop-mycroft.sh

Fixes the following issues:
- https://www.shellcheck.net/wiki/SC2164 -- Use 'cd ... || exit' or 'cd ... |...
- https://www.shellcheck.net/wiki/SC2086 -- Double quote to prevent globbing ...

The file is now automatically checked by the CICD job

* Shellcheck venv-activate.sh

Fix the following shellcheck issues:
- https://www.shellcheck.net/wiki/SC2068 -- Double quote array expansions to ...
- https://www.shellcheck.net/wiki/SC2145 -- Argument mixes string and array. ...
- https://www.shellcheck.net/wiki/SC2128 -- Expanding an array without an ind...
- https://www.shellcheck.net/wiki/SC2139 -- This expands when defined, not wh...
- https://www.shellcheck.net/wiki/SC2155 -- Declare and assign separately to ...

The help for invalid arg now works when the faulty argument isn't the
first argument

Co-authored-by: Kris Gesling <kris.gesling@mycroft.ai>

.github/workflows/unit_test.yml

@@ -76,5 +76,5 @@ jobs:
       env:
         SHELLCHECK_OPTS: -x
       with:
-        ignore_names: 'dev_setup.sh start-mycroft.sh stop-mycroft.sh venv-activate.sh run_test_suite.sh'
+        ignore_names: 'run_test_suite.sh'
 #        ignore_paths: doc mycroft test requirements

dev_setup.sh

@@ -22,7 +22,8 @@ export LANGUAGE=en
 # exit on any error
 set -Ee
 
-cd $(dirname $0)
+ROOT_DIRNAME=$(dirname "$0")
+cd "$ROOT_DIRNAME"
 TOP=$(pwd -L)
 
 function clean_mycroft_files() {
@@ -35,7 +36,7 @@ generated within the mycroft-core directory.
 
 Do you wish to continue? (y/n)'
     while true; do
-        read -N1 -s key
+        read -rN1 -s key
         case $key in
         [Yy])
             sudo rm -rf /var/log/mycroft
@@ -146,7 +147,7 @@ function get_YN() {
     # Loop until the user hits the Y or the N key
     echo -e -n "Choice [${CYAN}Y${RESET}/${CYAN}N${RESET}]: "
     while true; do
-        read -N1 -s key
+        read -rN1 -s key
         case $key in
         [Yy])
             return 0
@@ -271,9 +272,11 @@ Would you like this to be added to your PATH in the .profile?'
         if [[ ! -f ~/.profile_mycroft ]] ; then
             # Only add the following to the .profile if .profile_mycroft
             # doesn't exist, indicating this script has not been run before
-            echo '' >> ~/.profile
+            {
-            echo '# include Mycroft commands' >> ~/.profile
+                echo ''
-            echo 'source ~/.profile_mycroft' >> ~/.profile
+                echo '# include Mycroft commands'
+                echo 'source ~/.profile_mycroft'
+            } >> ~/.profile
         fi
 
         echo "
@@ -295,9 +298,9 @@ fi" > ~/.profile_mycroft
         echo 'This script will create that folder for you.  This requires sudo'
         echo 'permission and might ask you for a password...'
         setup_user=$USER
-        setup_group=$(id -gn $USER)
+        setup_group=$(id -gn "$USER")
         $SUDO mkdir -p /opt/mycroft/skills
-        $SUDO chown -R ${setup_user}:${setup_group} /opt/mycroft
+        $SUDO chown -R "${setup_user}":"${setup_group}" /opt/mycroft
         echo 'Created!'
     fi
     if [[ ! -d skills ]] ; then
@@ -325,7 +328,7 @@ If unsure answer yes.
 fi
 
 function os_is() {
-    [[ $(grep "^ID=" /etc/os-release | awk -F'=' '/^ID/ {print $2}' | sed 's/\"//g') == $1 ]]
+    [[ $(grep "^ID=" /etc/os-release | awk -F'=' '/^ID/ {print $2}' | sed 's/\"//g') == "$1" ]]
 }
 
 function os_is_like() {
@@ -345,11 +348,11 @@ function redhat_common_install() {
 }
 
 function debian_install() {
-    APT_PACKAGE_LIST="git python3 python3-dev python3-setuptools libtool \
+    APT_PACKAGE_LIST=(git python3 python3-dev python3-setuptools libtool \
         libffi-dev libssl-dev autoconf automake bison swig libglib2.0-dev \
         portaudio19-dev mpg123 screen flac curl libicu-dev pkg-config \
         libjpeg-dev libfann-dev build-essential jq pulseaudio \
-        pulseaudio-utils"
+        pulseaudio-utils)
 
     if dpkg -V libjack-jackd2-0 > /dev/null 2>&1 && [[ -z ${CI} ]] ; then
         echo "
@@ -357,10 +360,10 @@ We have detected that your computer has the libjack-jackd2-0 package installed.
 Mycroft requires a conflicting package, and will likely uninstall this package.
 On some systems, this can cause other programs to be marked for removal.
 Please review the following package changes carefully."
-        read -p "Press enter to continue"
+        read -rp "Press enter to continue"
-        $SUDO apt-get install $APT_PACKAGE_LIST
+        $SUDO apt-get install "${APT_PACKAGE_LIST[@]}"
     else
-        $SUDO apt-get install -y $APT_PACKAGE_LIST
+        $SUDO apt-get install -y "${APT_PACKAGE_LIST[@]}"
     fi
 }
 
@@ -451,7 +454,7 @@ function install_deps() {
 ${YELLOW}Make sure to manually install:$BLUE git python3 python-setuptools python-venv pygobject libtool libffi libjpg openssl autoconf bison swig glib2.0 portaudio19 mpg123 flac curl fann g++ jq\n$RESET"
 
         echo 'Warning: Failed to install all dependencies. Continue? y/N'
-        read -n1 continue
+        read -rn1 continue
         if [[ $continue != 'y' ]] ; then
             exit 1
         fi
@@ -499,7 +502,7 @@ else
     # first, look for a build of mimic in the folder
     has_mimic=''
     if [[ -f ${TOP}/mimic/bin/mimic ]] ; then
-        has_mimic=$(${TOP}/mimic/bin/mimic -lv | grep Voice) || true
+        has_mimic=$("${TOP}"/mimic/bin/mimic -lv | grep Voice) || true
     fi
 
     # in not, check the system path
@@ -526,6 +529,7 @@ if [[ ! -x ${VIRTUALENV_ROOT}/bin/activate ]] ; then
 fi
 
 # Start the virtual environment
+# shellcheck source=/dev/null
 source "${VIRTUALENV_ROOT}/bin/activate"
 cd "$TOP"
 
@@ -534,7 +538,7 @@ HOOK_FILE='./.git/hooks/pre-commit'
 if [[ -n $INSTALL_PRECOMMIT_HOOK ]] || grep -q 'MYCROFT DEV SETUP' $HOOK_FILE; then
     if [[ ! -f $HOOK_FILE ]] || grep -q 'MYCROFT DEV SETUP' $HOOK_FILE; then
         echo 'Installing PEP8 check as precommit-hook'
-        echo "#! $(which python)" > $HOOK_FILE
+        echo "#! $(command -v python)" > $HOOK_FILE
         echo '# MYCROFT DEV SETUP' >> $HOOK_FILE
         cat ./scripts/pre-commit >> $HOOK_FILE
         chmod +x $HOOK_FILE
@@ -552,17 +556,15 @@ if [[ ! -f $VENV_PATH_FILE ]] ; then
     echo "import sys; new=sys.path[sys.__plen:]; del sys.path[sys.__plen:]; p=getattr(sys,'__egginsert',0); sys.path[p:p]=new; sys.__egginsert = p+len(new)" >> "$VENV_PATH_FILE" || return 1
 fi
 
-if ! grep -q "$TOP" $VENV_PATH_FILE ; then
+if ! grep -q "$TOP" "$VENV_PATH_FILE" ; then
     echo 'Adding mycroft-core to virtualenv path'
-    sed -i.tmp '1 a\
+    sed -i.tmp "1 a$TOP" "$VENV_PATH_FILE"
-'"$TOP"'
-' "$VENV_PATH_FILE"
 fi
 
 # install required python modules
 if ! pip install -r requirements/requirements.txt ; then
     echo 'Warning: Failed to install required dependencies. Continue? y/N'
-    read -n1 continue
+    read -rn1 continue
     if [[ $continue != 'y' ]] ; then
         exit 1
     fi
@@ -573,7 +575,7 @@ if [[ ! $(pip install -r requirements/extra-audiobackend.txt) ||
 	! $(pip install -r requirements/extra-stt.txt) ||
 	! $(pip install -r requirements/extra-mark1.txt) ]] ; then
     echo 'Warning: Failed to install some optional dependencies. Continue? y/N'
-    read -n1 continue
+    read -rn1 continue
     if [[ $continue != 'y' ]] ; then
         exit 1
     fi
@@ -585,7 +587,7 @@ if ! pip install -r requirements/tests.txt ; then
 fi
 
 SYSMEM=$(free | awk '/^Mem:/ { print $2 }')
-MAXCORES=$(($SYSMEM / 2202010))
+MAXCORES=$((SYSMEM / 2202010))
 MINCORES=1
 CORES=$(nproc)
 

start-mycroft.sh

@@ -18,7 +18,7 @@ SOURCE="${BASH_SOURCE[0]}"
 
 script=${0}
 script=${script##*/}
-cd -P "$( dirname "$SOURCE" )"
+cd -P "$( dirname "$SOURCE" )" || exit 1 # Enter scripts folder or fail!
 DIR="$( pwd )"
 VIRTUALENV_ROOT=${VIRTUALENV_ROOT:-"${DIR}/.venv"}
 
@@ -80,7 +80,7 @@ function name-to-script-path() {
 function source-venv() {
     # Enter Python virtual environment, unless under Docker
     if [ ! -f "/.dockerenv" ] ; then
-        source ${VIRTUALENV_ROOT}/bin/activate
+        source "${VIRTUALENV_ROOT}/bin/activate"
     fi
 }
 
@@ -97,19 +97,19 @@ function init-once() {
 function launch-process() {
     init-once
 
-    name-to-script-path ${1}
+    name-to-script-path "${1}"
 
     # Launch process in foreground
     echo "Starting $1"
-    python3 -m ${_module} $_params
+    python3 -m ${_module} "$@"
 }
 
 function require-process() {
     # Launch process if not found
-    name-to-script-path ${1}
+    name-to-script-path "${1}"
     if ! pgrep -f "python3 (.*)-m ${_module}" > /dev/null ; then
         # Start required process
-        launch-background ${1}
+        launch-background "${1}"
     fi
 }
 
@@ -117,11 +117,11 @@ function launch-background() {
     init-once
 
     # Check if given module is running and start (or restart if running)
-    name-to-script-path ${1}
+    name-to-script-path "${1}"
     if pgrep -f "python3 (.*)-m ${_module}" > /dev/null ; then
         if ($_force_restart) ; then
             echo "Restarting: ${1}"
-            "${DIR}/stop-mycroft.sh" ${1}
+            "${DIR}/stop-mycroft.sh" "${1}"
         else
             # Already running, no need to restart
             return
@@ -138,7 +138,7 @@ function launch-background() {
     fi
 
     # Launch process in background, sending logs to standard location
-    python3 -m ${_module} $_params >> /var/log/mycroft/${1}.log 2>&1 &
+    python3 -m ${_module} "$@" >> "/var/log/mycroft/${1}.log" 2>&1 &
 }
 
 function launch-all() {
@@ -188,7 +188,7 @@ if [[ "${1}" == "restart" ]] || [[ "${_opt}" == "restart" ]] ; then
     fi
     shift
 fi
-_params=$@
+
 
 if [[ ! "${_opt}" == "cli" ]] ; then
     check-dependencies
@@ -200,16 +200,16 @@ case ${_opt} in
         ;;
 
     "bus")
-        launch-background ${_opt}
+        launch-background "${_opt}"
         ;;
     "audio")
-        launch-background ${_opt}
+        launch-background "${_opt}"
         ;;
     "skills")
-        launch-background ${_opt}
+        launch-background "${_opt}"
         ;;
     "voice")
-        launch-background ${_opt}
+        launch-background "${_opt}"
         ;;
 
     "debug")
@@ -220,7 +220,7 @@ case ${_opt} in
     "cli")
         require-process bus
         require-process skills
-        launch-process ${_opt}
+        launch-process "${_opt}"
         ;;
 
     # TODO: Restore support for Wifi Setup on a Picroft, etc.
@@ -243,19 +243,19 @@ case ${_opt} in
         source "$DIR/bin/mycroft-skill-testrunner" vktest "$@"
         ;;
     "audiotest")
-        launch-process ${_opt}
+        launch-process "${_opt}"
         ;;
     "wakewordtest")
-        launch-process ${_opt}
+        launch-process "${_opt}"
         ;;
     "sdkdoc")
         source-venv
-        cd doc
+        cd doc || exit 1  # Exit if doc directory doesn't exist
-        make ${_params}
+        make "$@"
         cd ..
         ;;
     "enclosure")
-        launch-background ${_opt}
+        launch-background "${_opt}"
         ;;
 
     *)

stop-mycroft.sh

@@ -18,7 +18,7 @@ SOURCE="${BASH_SOURCE[0]}"
 
 script=${0}
 script=${script##*/}
-cd -P "$( dirname "$SOURCE" )"
+cd -P "$( dirname "$SOURCE" )" || exit 1 # quit if change of folder fails
 
 function help() {
     echo "${script}:  Mycroft service stopper"
@@ -49,16 +49,16 @@ function process-running() {
 }
 
 function end-process() {
-    if process-running $1 ; then
+    if process-running "$1" ; then
         # Find the process by name, only returning the oldest if it has children
         pid=$( pgrep -o -f "python3 (.*)-m mycroft.*${1}" )
         echo -n "Stopping $1 (${pid})..."
-        kill -SIGINT ${pid}
+        kill -SIGINT "${pid}"
 
         # Wait up to 5 seconds (50 * 0.1) for process to stop
         c=1
         while [ $c -le 50 ] ; do
-            if process-running $1 ; then
+            if process-running "$1" ; then
                 sleep 0.1
                 (( c++ ))
             else
@@ -66,11 +66,11 @@ function end-process() {
             fi
         done
 
-        if process-running $1 ; then
+        if process-running "$1" ; then
             echo "failed to stop."
             pid=$( pgrep -o -f "python3 (.*)-m mycroft.*${1}" )            
             echo -n "  Killing $1 (${pid})..."
-            kill -9 ${pid}
+            kill -9 "${pid}"
             echo "killed."
             result=120
         else

venv-activate.sh

@@ -45,21 +45,23 @@ function main() {
                ;;
 
             *)
-               echo "ERROR:  Unrecognized option: $@"
+               echo "ERROR:  Unrecognized option: $arg"
                return 1
                ;;
        esac
     done
 
-    if [[ "$0" == "$BASH_SOURCE" ]] ; then
+    if [[ "$0" == "${BASH_SOURCE[0]}" ]] ; then
         # Prevent running in script then exiting immediately
         echo "ERROR: Invoke with 'source venv-activate.sh' or '. venv-activate.sh'"
     else
-        local SRC_DIR="$( builtin cd "$( dirname "${BASH_SOURCE}" )" ; pwd -P )"
+        local SRC_DIR
-        source ${SRC_DIR}/.venv/bin/activate
+        SRC_DIR="$( builtin cd "$( dirname "${BASH_SOURCE[0]}" )" || exit 1; pwd -P )"
+        source "${SRC_DIR}/.venv/bin/activate"
         
         # Provide an easier to find "mycroft-" prefixed command.
         unalias mycroft-venv-activate 2>/dev/null
+        # shellcheck disable=SC2139 # The intention _is_ to resolve the variable at define time
         alias mycroft-venv-deactivate="deactivate && unalias mycroft-venv-deactivate 2>/dev/null && alias mycroft-venv-activate=\"source '${SRC_DIR}/venv-activate.sh'\""
         if [ $quiet -eq 0 ] ; then
             echo "Entering Mycroft virtual environment.  Run 'mycroft-venv-deactivate' to exit"
@@ -67,4 +69,4 @@ function main() {
     fi
 }
 
-main $@
+main "$@"