From fd12c88da532fe5f1f37c8f73690ff4501ac6886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85ke?= Date: Thu, 4 Nov 2021 08:31:17 +0100 Subject: [PATCH] Refactor/shellcheck bin (#3019) * Shellcheck and update mycroft-config * Shellcheck and update mycroft-pip * Shellcheck and update mycroft-cli-client * Shellcheck and update mycroft-help * Shellcheck and update mycroft-listen * Shellcheck and update mycroft-mic-test * Shellcheck and update mycroft-msk * Shellcheck and update mycroft-msm * Shellcheck and update mycroft-say-to * Shellcheck and update mycroft-skill-testrunner * Shellcheck and update mycroft-speak * Shellcheck and update mycroft-start * Shellcheck and update mycroft-stop * Add shellcheck step to github actions This runs most of the shellcheck tests. The excludes are: - SC1091: Avoids errors when shellcheck can't find sourced file - SC2034: Unused variables, for example colors that aren't used yet - SC2012: use of ls, from what I can see in our case this is fine (wc -l) The version is locked to latest master as of November 2 2021 --- .github/workflows/unit_test.yml | 11 ++++++++ bin/mycroft-cli-client | 2 +- bin/mycroft-config | 46 ++++++++++++++++----------------- bin/mycroft-help | 2 +- bin/mycroft-listen | 2 +- bin/mycroft-mic-test | 4 +-- bin/mycroft-msk | 2 +- bin/mycroft-msm | 2 +- bin/mycroft-pip | 2 +- bin/mycroft-say-to | 4 +-- bin/mycroft-skill-testrunner | 26 +++++++++---------- bin/mycroft-speak | 4 +-- bin/mycroft-start | 4 +-- bin/mycroft-stop | 4 +-- 14 files changed, 62 insertions(+), 53 deletions(-) diff --git a/.github/workflows/unit_test.yml b/.github/workflows/unit_test.yml index 858f23c0f89b..c35e6d828064 100644 --- a/.github/workflows/unit_test.yml +++ b/.github/workflows/unit_test.yml @@ -66,3 +66,14 @@ jobs: run: if [[ ${{ matrix.python-version }} == 3.9 ]]; then bash <(curl -s https://codecov.io/bash); fi env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} + shellcheck: + name: Shellcheck + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Shell check mycroft tools + uses: ludeeus/action-shellcheck@ceeca77f6538b97991ca2c2a2ebe1ab64e573b5e + env: + SHELLCHECK_OPTS: -x --exclude SC2034 --exclude SC2012 --exclude SC1091 + with: + scandir: ./bin diff --git a/bin/mycroft-cli-client b/bin/mycroft-cli-client index f40a316e3f61..a06e0351eb71 100755 --- a/bin/mycroft-cli-client +++ b/bin/mycroft-cli-client @@ -21,4 +21,4 @@ DIR="$( dirname "$SOURCE" )" source "$DIR/../venv-activate.sh" -q # Invoke the Command Line Interface -python -m mycroft.client.text $@ +python -m mycroft.client.text "$@" diff --git a/bin/mycroft-config b/bin/mycroft-config index 200c7ebeb57f..57f81db5ccd7 100755 --- a/bin/mycroft-config +++ b/bin/mycroft-config @@ -15,7 +15,7 @@ # limitations under the License. SOURCE="${BASH_SOURCE[0]}" -cd -P "$( dirname "$SOURCE" )" +cd -P "$( dirname "$SOURCE" )" || exit DIR="$( pwd )" script=${0} script=${script##*/} @@ -49,7 +49,7 @@ function help() { VIEWER="nano --syntax=json --view" if [ -z "$EDITOR" ] ; then - if [ $( which sensible-editor ) ] ; then + if which sensible-editor > /dev/null ; then EDITOR="sensible-editor" else EDITOR="nano --syntax=json --tempfile" @@ -82,14 +82,14 @@ function validate_config_file() { return 0 fi - echo -n ${BLUE} + echo -n "${BLUE}" # Remove any comments (lines starting with # or //) found in the file and # Use jq to validate and output errors sed 's/^\s*[#\/].*$//g' "$1" | sed '/^$/d' | jq -e "." > /dev/null result=$? - echo -n ${RESET} + echo -n "${RESET}" #xxx echo "RESULT=$result for $1" return $result @@ -99,7 +99,7 @@ _conf_file="${XDG_CONFIG_HOME:-$HOME/.config}/mycroft/mycroft.conf" function name_to_path() { case ${1} in "system") _conf_file="/etc/mycroft/mycroft.conf" ;; - "user") _conf_file=$(readlink -f ${XDG_CONFIG_HOME:-$HOME/.config}/mycroft/mycroft.conf) ;; + "user") _conf_file=$(readlink -f "${XDG_CONFIG_HOME:-$HOME/.config}/mycroft/mycroft.conf") ;; "default") _conf_file="$DIR/../mycroft/configuration/mycroft.conf" ;; "remote") _conf_file="$HOME/.cache/mycroft/web_cache.json" ;; @@ -113,12 +113,12 @@ function name_to_path() { ################################################################ function edit_config() { - name_to_path $1 - validate_config_file $_conf_file + name_to_path "$1" + validate_config_file "$_conf_file" rc=$? if [ $rc -ne 0 ] ; then echo "${YELLOW}WARNING: ${RESET}Configuration file did not pass validation before edits." - read -p "Review errors above and press ENTER to continue with editing." + read -r -p "Review errors above and press ENTER to continue with editing." fi if [ -f "${_conf_file}" ] ; then @@ -128,7 +128,7 @@ function edit_config() { echo "}" >> "${TEMP}/mycroft.json" fi - while [ 1 ] ; do + while true ; do case $1 in system | user) # Allow user to edit @@ -150,19 +150,18 @@ function edit_config() { fi # file was changed, validate changes - validate_config_file $TEMP/mycroft.json - if [ $? -ne 0 ] ; then + if validate_config_file $TEMP/mycroft.json > /dev/null ; then + key="S" + else echo "${YELLOW}WARNING: ${RESET}Configuration file does not pass validation, see errors above." echo "Press X to abandon changes, S to force save, any other key to edit again." - read -N1 -s key - else - key="S" + read -r -N1 -s key fi case $key in [Ss]) echo "Saving..." - mv $TEMP/mycroft.json $_conf_file + mv $TEMP/mycroft.json "$_conf_file" signal_reload_config break ;; @@ -180,11 +179,11 @@ function signal_reload_config() { source "$DIR/../venv-activate.sh" -q # Post a messagebus notification to reload the config file - output=$(python -m mycroft.messagebus.send "configuration.updated" "{}") + python -m mycroft.messagebus.send "configuration.updated" "{}" > /dev/null } function show_config() { - name_to_path $1 + name_to_path "$1" # Use jq to display formatted nicely (after stripping out comments) sed 's/^\s*[#\/].*$//g' "${_conf_file}" | sed '/^$/d' | jq "." @@ -201,7 +200,7 @@ function get_config() { json_config=$( source "$DIR/../venv-activate.sh" -q && python -c "import json; from mycroft.configuration import Configuration; print(json.dumps(Configuration.get()))" ) # Read the given variable from the mix - echo ${json_config} | jq -r "${value}" + echo "${json_config}" | jq -r "${value}" } function set_config() { @@ -212,10 +211,9 @@ function set_config() { value=".${value}" fi - jq "${value} = \"$2\"" $_conf_file > "${TEMP}/~mycroft.conf" - if [ $? -eq 0 ] ; then + if jq "${value} = \"$2\"" "$_conf_file" > "${TEMP}/~mycroft.conf" ; then # Successful update, replace the config file - mv "${TEMP}/~mycroft.conf" $_conf_file + mv "${TEMP}/~mycroft.conf" "$_conf_file" signal_reload_config fi } @@ -223,16 +221,16 @@ function set_config() { _opt=$1 case ${_opt} in "edit") - edit_config $2 + edit_config "$2" ;; "reload") signal_reload_config ;; "show") - show_config $2 + show_config "$2" ;; "get") - get_config $2 + get_config "$2" ;; "set") set_config "$2" "$3" diff --git a/bin/mycroft-help b/bin/mycroft-help index 822e078a2a60..4dba2452df3c 100755 --- a/bin/mycroft-help +++ b/bin/mycroft-help @@ -15,7 +15,7 @@ # limitations under the License. SOURCE="${BASH_SOURCE[0]}" -cd -P "$( dirname "$SOURCE" )"/.. +cd -P "$( dirname "$SOURCE" )"/.. || exit DIR="$( pwd )" echo -e "\e[36mMycroft\e[0m is your open source voice assistant. Full source" diff --git a/bin/mycroft-listen b/bin/mycroft-listen index 95a93b642556..d4533eb602eb 100755 --- a/bin/mycroft-listen +++ b/bin/mycroft-listen @@ -15,7 +15,7 @@ # limitations under the License. SOURCE="${BASH_SOURCE[0]}" -cd -P "$( dirname "$SOURCE" )" +cd -P "$( dirname "$SOURCE" )" || exit DIR="$( pwd )" # Enter the Mycroft venv diff --git a/bin/mycroft-mic-test b/bin/mycroft-mic-test index 584d1380940e..676ee712d58f 100755 --- a/bin/mycroft-mic-test +++ b/bin/mycroft-mic-test @@ -15,7 +15,7 @@ # limitations under the License. SOURCE="${BASH_SOURCE[0]}" -cd -P "$( dirname "$SOURCE" )" +cd -P "$( dirname "$SOURCE" )" || exit DIR="$( pwd )" restart=0 @@ -32,7 +32,7 @@ fi # Launch the standard audiotest -"$DIR/../start-mycroft.sh" audiotest $@ +"$DIR/../start-mycroft.sh" audiotest "$@" if [ $restart -eq 1 ] diff --git a/bin/mycroft-msk b/bin/mycroft-msk index be0a719f1e56..99c1e2d8a1c0 100755 --- a/bin/mycroft-msk +++ b/bin/mycroft-msk @@ -21,4 +21,4 @@ DIR="$( dirname "$SOURCE" )" source "$DIR/../venv-activate.sh" -q # Invoke the Mycroft Skills Kit from within the venv -msk $@ +msk "$@" diff --git a/bin/mycroft-msm b/bin/mycroft-msm index ab580da800ba..4edd2c1b0efa 100755 --- a/bin/mycroft-msm +++ b/bin/mycroft-msm @@ -21,4 +21,4 @@ DIR="$( dirname "$SOURCE" )" source "$DIR/../venv-activate.sh" -q # Invoke the Mycroft Skills Manager (msm) within the venv -msm $@ +msm "$@" diff --git a/bin/mycroft-pip b/bin/mycroft-pip index a42b16b847a3..dffe8b9399c8 100755 --- a/bin/mycroft-pip +++ b/bin/mycroft-pip @@ -21,4 +21,4 @@ DIR="$( dirname "$SOURCE" )" source "$DIR/../venv-activate.sh" -q # Install pip packages within the Mycroft venv -pip $@ \ No newline at end of file +pip "$@" diff --git a/bin/mycroft-say-to b/bin/mycroft-say-to index 4ae597f3062e..0d86d3a13f8b 100755 --- a/bin/mycroft-say-to +++ b/bin/mycroft-say-to @@ -15,7 +15,7 @@ # limitations under the License. SOURCE="${BASH_SOURCE[0]}" -cd -P "$( dirname "$SOURCE" )" +cd -P "$( dirname "$SOURCE" )" || exit DIR="$( pwd )" # Enter the Mycroft venv @@ -25,5 +25,5 @@ source "$DIR/../venv-activate.sh" -q set -- "${1:-$(