Skip to content

[Bug] Shellcheck reports issues for the repo shell scripts #1999

Closed
@eero-t

Description

@eero-t

Priority

Undecided

OS type

N/A

Hardware type

N/A

Installation method

  • Pull docker images from hub.docker.com
  • Build docker images from source
  • Other
  • N/A

Deploy method

  • Docker
  • Docker Compose
  • Kubernetes Helm Charts
  • Other
  • N/A

Running nodes

N/A

What's the version?

Latest Git.

Description

shellcheck tool (available in similarly named package in all major distros) reports multiple issues for the scripts. Its error and warning severity output should be fixed as most of them appear to be genuine functional issues.

(As that tool is nowadays standard way to verify that shell scripts are correctly done, having that output also makes project look unprofessional for outside contributors.)

Besides fixing the bugs, there should be CI check preventing them being re-introduced, and a shellcheck config that hides output from checks that project does not care about. If some item is not worth a fix, that line can have shellcheck disable directive to avoid it triggering CI. There could also be e.g. a Makefile targets to run shellcheck.

While info severity output is unlikely to indicate a functional issues in how scripts are currently used, fixing those might improve project security, so it would be good to eventually look into those too. That does not need to be done in context of this ticket though.

(style severity output is just for optimizing / simplifying script, so that can be ignored.)

Reproduce steps

Type and count of shellcheck reported error severity items:

$ find -name '*.sh' | xargs shellcheck | grep "(error)" | sed -e 's/^ *//' -e 's/--*^/--/' | sort | uniq -c | sort -nr
     28 ^-- SC2148 (error): Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
      3 ^-- SC2170 (error): Invalid number for -eq. Use = to compare as string (or use $var to expand as a variable).
      3 ^-- SC2170 (error): Invalid number for -eq. Use = to compare as string (or use $((expr)) to expand as an arithmetic expression).
      2 ^-- SC2096 (error): On most OS, shebangs can only specify a single parameter.
      1 ^-- SC1113 (error): Use #!, not just #, for the shebang.

Most common warning severity shellcheck output:

$ find -name '*.sh' | xargs shellcheck | grep "(warning)" | sed -e 's/^ *//' -e 's/--*^/--/' | sort | uniq -c | sort -nr
    216 ^-- SC2155 (warning): Declare and assign separately to avoid masking return values.
     88 ^-- SC2164 (warning): Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
     60 ^-- SC2154 (warning): https_proxy is referenced but not assigned.
     59 ^-- SC2154 (warning): http_proxy is referenced but not assigned.
     32 ^-- SC2164 (warning): Use 'popd ... || exit' or 'popd ... || return' in case popd fails.
     19 ^-- SC2154 (warning): ip_address is referenced but not assigned.
     14 ^-- SC2034 (warning): MOUNT_DIR appears unused. Verify use (or export if used externally).
     12 ^-- SC2164 (warning): Use 'pushd ... || exit' or 'pushd ... || return' in case pushd fails.
     11 ^-- SC2154 (warning): host_ip is referenced but not assigned.
      6 ^-- SC2216 (warning): Piping to 'rm', a command that doesn't read stdin. Wrong command or missing xargs?
      6 ^-- SC2088 (warning): Tilde does not expand in quotes. Use $HOME.
      5 ^-- SC2154 (warning): host_ip is referenced but not assigned (did you mean 'HOST_IP'?).
...

=> Scripts use many variables that they have not set/defined, and other functional issues.

Most common info severity shellcheck output:

$ find -name '*.sh' | xargs shellcheck | grep "(info)" | sed -e 's/^ *//' -e 's/--*^/--/' | sort | uniq -c | sort -nr
   2341 ^-- SC2086 (info): Double quote to prevent globbing and word splitting.
     47 ^-- SC1091 (info): Not following: activate was not specified as input (see shellcheck -x).
     36 ^-- SC1091 (info): Not following: set_env.sh was not specified as input (see shellcheck -x).
     12 ^-- SC1091 (info): Not following: ./set_env.sh was not specified as input (see shellcheck -x).
      6 ^-- SC1091 (info): Not following: ./docker_compose/intel/set_env.sh was not specified as input (see shellcheck -x).
      5 ^-- SC2269 (info): This variable is assigned to itself, so the assignment does nothing.
      5 ^-- SC1091 (info): Not following: stress_venv/bin/activate was not specified as input (see shellcheck -x).
      5 ^-- SC1091 (info): Not following: ./stress_venv/bin/activate was not specified as input (see shellcheck -x).
      4 ^-- SC2162 (info): read without -r will mangle backslashes.
      4 ^-- SC2035 (info): Use ./*glob* or -- *glob* so names with dashes won't become options.
...

=> Having shell scripts without *.sh extension complicates automating their linting in CI...

Metadata

Metadata

Labels

A1high proritybugSomething isn't working

Type

Projects

Status

Done

Relationships

None yet

Development

No branches or pull requests

Issue actions