#2754 PR `closed`: Prepare for Issue #1040: add a fancy GitHub Action for 'shellcheck' for all pushes and PRs¶

Labels: enhancement, discuss / RFC, ReaR Project, no-pr-activity

thomasmerz opened issue at 2022-02-04 16:27:¶

Relax-and-Recover (ReaR) Pull Request Template¶

Pull Request Details:¶

Type: Enhancement
Impact: High
Reference to related issue (URL):
This is a preparation for future PRs from me to solve #1040
How was this pull request tested?
See GitHub Action in my forked repo for this PR:
https://github.com/thomasmerz/rear/runs/5069429307?check_suite_focus=true
Brief description of the changes in this pull request:
This PR introduces a GitHub Action that will shellcheck all scripts in dirs "test" and "usr" for committer's and also repo-owner's convenience. All warnings including and above "severity=warning" will be reported and made transparent.

pcahyna commented at 2022-02-04 16:44:¶

@antonvoznia has been working on something similar recently, but if you beat him to it, why not :-)

pcahyna commented at 2022-02-04 16:49:¶

That said, it looks quite primitive. Do you have an example of a run for a pull request in your repo (pull request from your branch to your branch)? I am afraid that any trivial pull request will produce hundreds if not thousands of warnings unrelated to the PR itself. I believe that what we need is to display only errors that are introduced by the PR, not everything.

thomasmerz commented at 2022-02-04 18:10:¶

That said, it looks quite primitive. Do you have an example of a run for a pull request in your repo (pull request from your branch to your branch)? I am afraid that any trivial pull request will produce hundreds if not thousands of warnings unrelated to the PR itself. I believe that what we need is to display only errors that are introduced by the PR, not everything.

You could merge this PR not before a fix+foxy shellchecked version exists, so that any following push/merge to master and every PR will only shellcheck (all) scripts in a specific path (tests/** and usr/**). So we won't get any warning that won't be related to the PR itself.
Details: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#example-including-paths
Or/and we can also trigger no action on pushing to HEAD or merging into HEAD branch.

See my next commit which didn't trigger an action for point 1.

So, do you agree to this (1) or do you think point 2 might also be reasonable?

jsmeix commented at 2022-02-16 10:18:¶

I won't find noticeable time for this pull request until ReaR 2.7 was released, cf.
https://github.com/rear/rear/issues/2751
so I set this pull request's milestone to ReaR v2.8 i.e. after ReaR 2.7 was released.

thomasmerz commented at 2022-02-18 16:50:¶

Output of my "local GH action" (act makes it possible):

17:49 $
🦎🖥  ✔ ~/temp/PRs/rear [issue_1040_shellcheck|✔]
17:49 $ act
[Shellcheck Lint/Shellcheck Lint] 🚀  Start image=ghcr.io/catthehacker/ubuntu:act-latest
[Shellcheck Lint/Shellcheck Lint]   🐳  docker pull image=ghcr.io/catthehacker/ubuntu:act-latest platform= username= forcePull=false
[Shellcheck Lint/Shellcheck Lint]   🐳  docker create image=ghcr.io/catthehacker/ubuntu:act-latest platform= entrypoint=["/usr/bin/tail" "-f" "/dev/null"] cmd=[]
[Shellcheck Lint/Shellcheck Lint]   🐳  docker run image=ghcr.io/catthehacker/ubuntu:act-latest platform= entrypoint=["/usr/bin/tail" "-f" "/dev/null"] cmd=[]
[Shellcheck Lint/Shellcheck Lint]   🐳  docker exec cmd=[mkdir -m 0777 -p /var/run/act] user=root workdir=
[Shellcheck Lint/Shellcheck Lint]   🐳  docker cp src=/home/thomas/temp/PRs/rear/. dst=/home/thomas/temp/PRs/rear
[Shellcheck Lint/Shellcheck Lint]   🐳  docker exec cmd=[mkdir -p /home/thomas/temp/PRs/rear] user= workdir=
[Shellcheck Lint/Shellcheck Lint] ⭐  Run actions/checkout@v2
[Shellcheck Lint/Shellcheck Lint]   ✅  Success - actions/checkout@v2
[Shellcheck Lint/Shellcheck Lint] ⭐  Run Download Shellcheck
[Shellcheck Lint/Shellcheck Lint]   🐳  docker exec cmd=[bash --noprofile --norc -e -o pipefail /home/thomas/temp/PRs/rear/workflow/1] user= workdir=
| shellcheck-stable/LICENSE.txt
| shellcheck-stable/README.txt
| shellcheck-stable/shellcheck
[Shellcheck Lint/Shellcheck Lint]   ✅  Success - Download Shellcheck
[Shellcheck Lint/Shellcheck Lint] ⭐  Run Check Shellcheck Version
[Shellcheck Lint/Shellcheck Lint]   🐳  docker exec cmd=[bash --noprofile --norc -e -o pipefail /home/thomas/temp/PRs/rear/workflow/2] user= workdir=
| ShellCheck - shell script analysis tool
| version: 0.8.0
| license: GNU General Public License, version 3
| website: https://www.shellcheck.net
[Shellcheck Lint/Shellcheck Lint]   ✅  Success - Check Shellcheck Version
[Shellcheck Lint/Shellcheck Lint] ⭐  Run Run Shellcheck
[Shellcheck Lint/Shellcheck Lint]   🐳  docker exec cmd=[bash --noprofile --norc -e -o pipefail /home/thomas/temp/PRs/rear/workflow/3] user= workdir=
| shellcheck'ing ./tests/setup1/run.sh
|
| In ./tests/setup1/run.sh line 65:
|   grep -v "dev eth0" $f > $RESULT_DIR/$(basename $f)
|                                             ^------------^ SC2046 (warning): Quote this to prevent word splitting.
|
| For more information:
|   https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...
| shellcheck'ing ./tests/setup1/tcase1.sh
| shellcheck'ing ./tests/setup1/tcase2.sh
| shellcheck'ing ./tests/setup1/tcase3.sh
| shellcheck'ing ./tests/setup1/tcase4.sh
| shellcheck'ing ./tests/setup1/tcase5.sh
| shellcheck'ing ./tests/setup1/tcase6.sh
| shellcheck'ing ./tests/setup1/tcase7.sh
|
| In ./tests/setup1/tcase7.sh line 3:
| CONFIG_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
| ^--------^ SC2034 (warning): CONFIG_DIR appears unused. Verify use (or export if used externally).
|
| For more information:
|   https://www.shellcheck.net/wiki/SC2034 -- CONFIG_DIR appears unused. Verify...
| shellcheck'ing ./tests/setup1/tcase8.sh
|
| In ./tests/setup1/tcase8.sh line 3:
| CONFIG_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
| ^--------^ SC2034 (warning): CONFIG_DIR appears unused. Verify use (or export if used externally).
|
| For more information:
|   https://www.shellcheck.net/wiki/SC2034 -- CONFIG_DIR appears unused. Verify...
| shellcheck'ing ./tests/setup1/verify.sh
| shellcheck'ing ./tests/setup2/tcase1.sh
| shellcheck'ing ./tests/setup2/tcase2.sh
| shellcheck'ing ./tests/setup2/tcase3.sh
| shellcheck'ing ./tests/setup2/tcase4.sh
| shellcheck'ing ./tests/setup2/tcase5.sh
| shellcheck'ing ./tests/setup2/tcase6.sh
| shellcheck'ing ./tests/setup2/tcase7.sh
|
| In ./tests/setup2/tcase7.sh line 3:
| CONFIG_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
| ^--------^ SC2034 (warning): CONFIG_DIR appears unused. Verify use (or export if used externally).
|
| For more information:
|   https://www.shellcheck.net/wiki/SC2034 -- CONFIG_DIR appears unused. Verify...
| shellcheck'ing ./tests/setup2/tcase8.sh
|
| In ./tests/setup2/tcase8.sh line 3:
| CONFIG_DIR="$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")"
| ^--------^ SC2034 (warning): CONFIG_DIR appears unused. Verify use (or export if used externally).
|
| For more information:
|   https://www.shellcheck.net/wiki/SC2034 -- CONFIG_DIR appears unused. Verify...
| shellcheck'ing ./usr/sbin/rear
|
| In ./usr/sbin/rear line 44:
| readonly INITIAL_BASH_FLAGS_AND_OPTIONS_COMMANDS="$( get_bash_flags_and_options_commands )"
|          ^-- SC2034 (warning): INITIAL_BASH_FLAGS_AND_OPTIONS_COMMANDS appears unused. Verify use (or export if used externally).
|          ^-- SC2155 (warning): Declare and assign separately to avoid masking return values.
|
|
| In ./usr/sbin/rear line 71:
| readonly START_SECONDS=$( date +%s )
|          ^-----------^ SC2155 (warning): Declare and assign separately to avoid masking return values.
|
|
| In ./usr/sbin/rear line 73:
| readonly START_DATE_TIME_NUMBER=$( date -d@$START_SECONDS +%Y%m%d%H%M%S )
|          ^--------------------^ SC2034 (warning): START_DATE_TIME_NUMBER appears unused. Verify use (or export if used externally).
|          ^--------------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.
|
|
| In ./usr/sbin/rear line 75:
| readonly START_DATE_TIME_STRING=$( date -d@$START_SECONDS +'%F %T' )
|          ^--------------------^ SC2155 (warning): Declare and assign separately to avoid masking return values.
|
|
| In ./usr/sbin/rear line 93:
| readonly SCRIPT_FILE="$( readlink -f $( type -p "$0" || echo "$0" ) )"
|          ^---------^ SC2155 (warning): Declare and assign separately to avoid masking return values.
|                                      ^----------------------------^ SC2046 (warning): Quote this to prevent word splitting.
|
|
| In ./usr/sbin/rear line 112:
| readonly WORKING_DIR="$( pwd )"
|          ^---------^ SC2034 (warning): WORKING_DIR appears unused. Verify use (or export if used externally).
|          ^---------^ SC2155 (warning): Declare and assign separately to avoid masking return values.
|
|
| In ./usr/sbin/rear line 120:
| DISKLAYOUT_FILE="$VAR_DIR/layout/disklayout.conf"
| ^-------------^ SC2034 (warning): DISKLAYOUT_FILE appears unused. Verify use (or export if used externally).
|
|
| In ./usr/sbin/rear line 124:
| readonly TARGET_FS_ROOT="/mnt/local"
|          ^------------^ SC2034 (warning): TARGET_FS_ROOT appears unused. Verify use (or export if used externally).
|
|
| In ./usr/sbin/rear line 232:
|             echo "$PROGNAME: unrecognized option '$option'"
|                                                   ^-----^ SC2154 (warning): option is referenced but not assigned.
|
|
| In ./usr/sbin/rear line 277:
|     v="-v"
|     ^-- SC2034 (warning): v appears unused. Verify use (or export if used externally).
|
|
| In ./usr/sbin/rear line 278:
|     verbose="--verbose"
|     ^-----^ SC2034 (warning): verbose appears unused. Verify use (or export if used externally).
|
|
| In ./usr/sbin/rear line 288:
| readonly DEBUG DEBUGSCRIPTS DEBUGSCRIPTS_ARGUMENT DEBUG_OUTPUT_DEV DISPENSABLE_OUTPUT_DEV
|                ^----------^ SC2034 (warning): DEBUGSCRIPTS appears unused. Verify use (or export if used externally).
|                                                   ^--------------^ SC2034 (warning): DEBUG_OUTPUT_DEV appears unused. Verify use (or export if used externally).
|                                                                    ^--------------------^ SC2034 (warning): DISPENSABLE_OUTPUT_DEV appears unused. Verify use (or export if used externally).
|
|
| In ./usr/sbin/rear line 289:
| readonly SIMULATE STEPBYSTEP VERBOSE
|                   ^--------^ SC2034 (warning): STEPBYSTEP appears unused. Verify use (or export if used externally).
|
|
| In ./usr/sbin/rear line 327:
| readonly DEFAULT_BASH_FLAGS_AND_OPTIONS_COMMANDS="$( get_bash_flags_and_options_commands )"
|          ^-- SC2034 (warning): DEFAULT_BASH_FLAGS_AND_OPTIONS_COMMANDS appears unused. Verify use (or export if used externally).
|          ^-- SC2155 (warning): Declare and assign separately to avoid masking return values.
|
|
| In ./usr/sbin/rear line 497:
|     DebugPrint "Command line options: $0 ${CMD_OPTS[@]}"
|                                          ^------------^ SC2145 (error): Argument mixes string and array. Use * or separate argument.
|
|
| In ./usr/sbin/rear line 504:
| test "$DEBUG" && KEEP_BUILD_DIR=1 || true
|                  ^------------^ SC2034 (warning): KEEP_BUILD_DIR appears unused. Verify use (or export if used externally).
|
|
| In ./usr/sbin/rear line 508:
| readonly RECOVERY_MODE
|          ^-----------^ SC2034 (warning): RECOVERY_MODE appears unused. Verify use (or export if used externally).
|
|
| In ./usr/sbin/rear line 570:
|                 else if test -r "$config_append_file_path.conf" ; then
|                 ^-- SC1075 (error): Use 'elif' instead of 'else if' (or put 'if' on new line if nesting).
|
|
| In ./usr/sbin/rear line 586:
|         else if test -r "$config_append_file_path.conf" ; then
|         ^-- SC1075 (error): Use 'elif' instead of 'else if' (or put 'if' on new line if nesting).
|
|
| In ./usr/sbin/rear line 597:
| readonly SHARE_DIR CONFIG_DIR VAR_DIR LOG_DIR KERNEL_VERSION
|                                               ^------------^ SC2034 (warning): KERNEL_VERSION appears unused. Verify use (or export if used externally).
|
|
| In ./usr/sbin/rear line 606:
| readonly VERSION_INFO="
|          ^----------^ SC2034 (warning): VERSION_INFO appears unused. Verify use (or export if used externally).
|          ^----------^ SC2155 (warning): Declare and assign separately to avoid masking return values.
|
|
| In ./usr/sbin/rear line 649:
| EXIT_FAIL_MESSAGE=0
| ^---------------^ SC2034 (warning): EXIT_FAIL_MESSAGE appears unused. Verify use (or export if used externally).
|
| For more information:
|   https://www.shellcheck.net/wiki/SC1075 -- Use 'elif' instead of 'else if' (...
|   https://www.shellcheck.net/wiki/SC2145 -- Argument mixes string and array. ...
|   https://www.shellcheck.net/wiki/SC2034 -- DEBUGSCRIPTS appears unused. Veri...
| shellcheck'ing ./usr/share/rear/lib/write-protect-functions.sh
|
| In ./usr/share/rear/lib/write-protect-functions.sh line 18:
|     local device="$( write_protected_candidate_device "$1" )"
|           ^----^ SC2155 (warning): Declare and assign separately to avoid masking return values.
|
|
| In ./usr/share/rear/lib/write-protect-functions.sh line 59:
|     local device="$(write_protected_candidate_device "$1")"
|           ^----^ SC2155 (warning): Declare and assign separately to avoid masking return values.
|
|
| In ./usr/share/rear/lib/write-protect-functions.sh line 91:
|     local device="$(write_protected_candidate_device "$1")"
|           ^----^ SC2155 (warning): Declare and assign separately to avoid masking return values.
|
|
| In ./usr/share/rear/lib/write-protect-functions.sh line 100:
|                 if [[ "$partition_label" == $write_protected_pattern ]]; then
|                                             ^----------------------^ SC2053 (warning): Quote the right-hand side of == in [[ ]] to prevent glob matching.
|
|
| In ./usr/share/rear/lib/write-protect-functions.sh line 112:
|     local device="$(write_protected_candidate_device "$1")"
|           ^----^ SC2155 (warning): Declare and assign separately to avoid masking return values.
|
| For more information:
|   https://www.shellcheck.net/wiki/SC2053 -- Quote the right-hand side of == i...
|   https://www.shellcheck.net/wiki/SC2155 -- Declare and assign separately to ...
| shellcheck'ing ./usr/share/rear/skel/default/bin/dhclient-script
|
| In ./usr/share/rear/skel/default/bin/dhclient-script line 9:
| LOGFACILITY="local7"
| ^---------^ SC2034 (warning): LOGFACILITY appears unused. Verify use (or export if used externally).
|
|
| In ./usr/share/rear/skel/default/bin/dhclient-script line 10:
| LOGLEVEL="notice"
| ^------^ SC2034 (warning): LOGLEVEL appears unused. Verify use (or export if used externally).
|
|
| In ./usr/share/rear/skel/default/bin/dhclient-script line 18:
| new_prefix="$(get_prefix ${new_ip_address} ${new_subnet_mask})"
|                                            ^----------------^ SC2154 (warning): new_subnet_mask is referenced but not assigned.
|
|
| In ./usr/share/rear/skel/default/bin/dhclient-script line 19:
| old_prefix="$(get_prefix ${old_ip_address} ${old_subnet_mask})"
| ^--------^ SC2034 (warning): old_prefix appears unused. Verify use (or export if used externally).
|                                            ^----------------^ SC2154 (warning): old_subnet_mask is referenced but not assigned.
|
|
| In ./usr/share/rear/skel/default/bin/dhclient-script line 20:
| alias_prefix="$(get_prefix ${alias_ip_address} ${alias_subnet_mask})"
|                                                ^------------------^ SC2154 (warning): alias_subnet_mask is referenced but not assigned.
|
|
| In ./usr/share/rear/skel/default/bin/dhclient-script line 22:
| case "${reason}" in
|       ^-------^ SC2154 (warning): reason is referenced but not assigned.
|
|
| In ./usr/share/rear/skel/default/bin/dhclient-script line 34:
|         if [ "${keep_old_ip}" = "yes" ]; then
|               ^------------^ SC2154 (warning): keep_old_ip is referenced but not assigned.
|
|
| In ./usr/share/rear/skel/default/bin/dhclient-script line 162:
|             ip -4 addr add ${new_ip_address}/${new_prefix} broadcast ${new_broadcast_address} dev ${interface}
|                                                                      ^----------------------^ SC2154 (warning): new_broadcast_address is referenced but not assigned.
|
| For more information:
|   https://www.shellcheck.net/wiki/SC2034 -- LOGFACILITY appears unused. Verif...
|   https://www.shellcheck.net/wiki/SC2154 -- alias_subnet_mask is referenced b...
| shellcheck'ing ./usr/share/rear/skel/default/bin/dhcpcd.sh
| shellcheck'ing ./usr/share/rear/skel/default/bin/ifup
| shellcheck'ing ./usr/share/rear/skel/default/bin/login
|
| In ./usr/share/rear/skel/default/bin/login line 45:
| cd $HOME
| ^------^ SC2164 (warning): Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
|
| Did you mean:
| cd $HOME || exit
|
| For more information:
|   https://www.shellcheck.net/wiki/SC2164 -- Use 'cd ... || exit' or 'cd ... |...
| shellcheck'ing ./usr/share/rear/skel/default/etc/scripts/boot
|
| In ./usr/share/rear/skel/default/etc/scripts/boot line 38:
| export HOSTNAME="$(cat /etc/HOSTNAME)" # set hostname in THIS shell
|        ^------^ SC2155 (warning): Declare and assign separately to avoid masking return values.
|
|
| In ./usr/share/rear/skel/default/etc/scripts/boot line 41:
| echo Hostname set to $(uname -n)
|                      ^---------^ SC2046 (warning): Quote this to prevent word splitting.
|
| For more information:
|   https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...
|   https://www.shellcheck.net/wiki/SC2155 -- Declare and assign separately to ...
| shellcheck'ing ./usr/share/rear/skel/default/etc/scripts/do-shutdown
| shellcheck'ing ./usr/share/rear/skel/default/etc/scripts/run-serial
|
| In ./usr/share/rear/skel/default/etc/scripts/run-serial line 7:
|     if $(grep -q ^s$nr: /etc/inittab) ; then
|        ^----------------------------^ SC2091 (warning): Remove surrounding $() to avoid executing output (or use eval if intentional).
|
| For more information:
|   https://www.shellcheck.net/wiki/SC2091 -- Remove surrounding $() to avoid e...
| shellcheck'ing ./usr/share/rear/skel/default/etc/scripts/run-sshd
| shellcheck'ing ./usr/share/rear/skel/default/etc/scripts/run-syslog
| shellcheck'ing ./usr/share/rear/skel/default/etc/scripts/system-setup
|
| In ./usr/share/rear/skel/default/etc/scripts/system-setup line 13:
| kernel_command_line=( $( cat /proc/cmdline ) )
|                       ^--------------------^ SC2207 (warning): Prefer mapfile or read -a to split command output (or quote to avoid splitting).
|
|
| In ./usr/share/rear/skel/default/etc/scripts/system-setup line 200:
|         select choice in "${choices[@]}" ; do
|         ^----^ SC2034 (warning): choice appears unused. Verify use (or export if used externally).
|
| For more information:
|   https://www.shellcheck.net/wiki/SC2034 -- choice appears unused. Verify use...
|   https://www.shellcheck.net/wiki/SC2207 -- Prefer mapfile or read -a to spli...
| shellcheck'ing ./usr/share/rear/skel/default/etc/scripts/system-status.sh
|
| In ./usr/share/rear/skel/default/etc/scripts/system-status.sh line 5:
| LF="$(echo)"
| ^-- SC2034 (warning): LF appears unused. Verify use (or export if used externally).
|
|
| In ./usr/share/rear/skel/default/etc/scripts/system-status.sh line 17:
| if test $(brctl show | wc -l) -gt 1 ; then
|         ^-------------------^ SC2046 (warning): Quote this to prevent word splitting.
|
| For more information:
|   https://www.shellcheck.net/wiki/SC2034 -- LF appears unused. Verify use (or...
|   https://www.shellcheck.net/wiki/SC2046 -- Quote this to prevent word splitt...
| shellcheck'ing ./usr/share/rear/skel/default/etc/scripts/unlock-opal-disks
|
| In ./usr/share/rear/skel/default/etc/scripts/unlock-opal-disks line 66:
|     IFS='\$' read -r _ method salt _ <<<"$OPAL_PBA_DEBUG_PASSWORD"
|         ^--^ SC2141 (warning): This IFS value contains a literal backslash. For tabs/linefeeds/escapes, use $'..', literal, or printf.
|
|
| In ./usr/share/rear/skel/default/etc/scripts/unlock-opal-disks line 186:
| devices=( $(opal_devices) )
|           ^-------------^ SC2207 (warning): Prefer mapfile or read -a to split command output (or quote to avoid splitting).
|
| For more information:
|   https://www.shellcheck.net/wiki/SC2141 -- This IFS value contains a literal...
|   https://www.shellcheck.net/wiki/SC2207 -- Prefer mapfile or read -a to spli...
| Shellcheck failed for one or more shellscript(s)
[Shellcheck Lint/Shellcheck Lint]   ❌  Failure - Run Shellcheck
Error: exit with `FAILURE`: 1
🦎🖥  ✔ ~/temp/PRs/rear [issue_1040_shellcheck|✔]
17:49 $

But PR #2756 will fix this 😉