make everything pass shellcheck #150

New Issue

ungleich-gitea · 2021-11-20T15:20:16Z

ungleich-gitea commented

2021-11-20 15:20:16 +00:00

Created by: 4nd3r

even if we exclude SC2154 (something is referenced but not assigned), then others should be dealt with in some point. yes, that's lot of work :>

thoughts on this?

$ find . -type f | while read -r l; do file "$l" | grep -Fq 'POSIX shell script' && shellcheck -f gcc "$l"; done | sed -nr 's/.+:[0-9]+:\s(.+)$/\1/p' | sort | uniq -c | sort -nr
    221 note: Double quote to prevent globbing and word splitting. [SC2086]
    196 warning: __object is referenced but not assigned. [SC2154]
    172 warning: __object_id is referenced but not assigned. [SC2154]
     41 warning: __global is referenced but not assigned. [SC2154]
     40 warning: __type is referenced but not assigned. [SC2154]
     30 warning: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined. [SC2166]
     17 warning: __explorer is referenced but not assigned. [SC2154]
     17 error: Iterating over ls output is fragile. Use globs. [SC2045]
     14 warning: __object_name is referenced but not assigned. [SC2154]
     13 warning: Prefer [ p ] || [ q ] as [ p -o q ] is not well defined. [SC2166]
     12 warning: Quote this to prevent word splitting. [SC2046]
     11 warning: __messages_out is referenced but not assigned. [SC2154]
     11 warning: Declare and assign separately to avoid masking return values. [SC2155]
     11 note: Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead. [SC2002]
     10 warning: __target_host is referenced but not assigned. [SC2154]
      9 note: Useless echo? Instead of 'echo $(cmd)', just use 'cmd'. [SC2005]
      9 note: $/${} is unnecessary on arithmetic variables. [SC2004]
      7 note: Expressions don't expand in single quotes, use double quotes for that. [SC2016]
      6 warning: In POSIX sh, 'local' is undefined. [SC2039]
      5 warning: Remove surrounding $() to avoid executing output. [SC2091]
      5 warning: __remote_exec is referenced but not assigned. [SC2154]
      5 warning: Quotes/backslashes in this variable will not be respected. [SC2090]
      5 warning: In POSIX sh, echo flags are undefined. [SC2039]
      5 note: Use find instead of ls to better handle non-alphanumeric filenames. [SC2012]
      4 warning: __remote_copy is referenced but not assigned. [SC2154]
      4 note: read without -r will mangle backslashes. [SC2162]
      4 note: Not following: /etc/openwrt_release was not specified as input (see shellcheck -x). [SC1091]
      3 warning: Use cd ... || exit in case cd fails. [SC2164]
      3 warning: __target_host is referenced but not assigned (did you mean 'my_target_host'?). [SC2154]
      3 warning: require is referenced but not assigned. [SC2154]
      3 warning: Quotes/backslashes will be treated literally. Use an array. [SC2089]
      3 warning: options appears unused. Verify it or export it. [SC2034]
      3 warning: __messages_in is referenced but not assigned. [SC2154]
      3 warning: For loops over find output are fragile. Use find -exec or a while read loop. [SC2044]
      3 warning: __cdist_object_marker is referenced but not assigned. [SC2154]
      3 note: Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'. [SC2116]
      3 note: echo won't expand escape sequences. Consider printf. [SC2028]
      3 error: Scripts are case sensitive. Use 'done', not 'DONE'. [SC1081]
      3 error: Argument mixes string and array. Use * or separate argument. [SC2145]
      2 warning: __target_host is referenced but not assigned (did you mean 'target_host'?). [SC2154]
      2 warning: state is referenced but not assigned. [SC2154]
      2 warning: state appears unused. Verify it or export it. [SC2034]
      2 warning: set_attributes appears unused. Verify it or export it. [SC2034]
      2 warning: ruby appears unused. Verify it or export it. [SC2034]
      2 warning: output is referenced but not assigned. [SC2154]
      2 warning: name appears unused. Verify it or export it. [SC2034]
      2 warning: gemsetname appears unused. Verify it or export it. [SC2034]
      2 warning: deleteJail references arguments, but none are ever passed. [SC2120]
      2 warning: __debug is referenced but not assigned. [SC2154]
      2 warning: cur_ip appears unused. Verify it or export it. [SC2034]
      2 warning: cksum appears unused. Verify it or export it. [SC2034]
      2 note: Use grep -q instead of comparing output with [ -n .. ]. [SC2143]
      2 note: Use deleteJail "$@" if function's $1 should mean script's $1. [SC2119]
      2 note: The mentioned parser error was in this case expression. [SC1009]
      2 note: Don't use [] around ranges in tr, it replaces literal square brackets. [SC2021]
      2 error: Unexpected keyword/token. Fix any mentioned problems and try again. [SC1072]
      2 error: -f doesn't work with globs. Use a for loop. [SC2144]
      2 error: Couldn't parse this case item. [SC1073]
      1 warning: __verbose is referenced but not assigned. [SC2154]
      1 warning: uri is referenced but not assigned. [SC2154]
      1 warning: type is referenced but not assigned (for output from commands, use "$(type ...)" ). [SC2154]
      1 warning: __type_explorer is referenced but not assigned. [SC2154]
      1 warning: type appears unused. Verify it or export it. [SC2034]
      1 warning: target appears unused. Verify it or export it. [SC2034]
      1 warning: supported_add_properties appears unused. Verify it or export it. [SC2034]
      1 warning: source appears unused. Verify it or export it. [SC2034]
      1 warning: service_id is referenced but not assigned. [SC2154]
      1 warning: runs is referenced but not assigned. [SC2154]
      1 warning: repo_name is referenced but not assigned. [SC2154]
      1 warning: relative_script appears unused. Verify it or export it. [SC2034]
      1 warning: prefix appears unused. Verify it or export it. [SC2034]
      1 warning: Possible misspelling: UNAME may not be assigned, but uname is. [SC2153]
      1 warning: path appears unused. Verify it or export it. [SC2034]
      1 warning: __object_id is referenced but not assigned (did you mean 'object_id'?). [SC2154]
      1 warning: name is referenced but not assigned. [SC2154]
      1 warning: my_target_host is referenced but not assigned. [SC2154]
      1 warning: In POSIX sh, LINENO is undefined. [SC2039]
      1 warning: In POSIX sh, 'caller' is undefined. [SC2039]
      1 warning: format appears unused. Verify it or export it. [SC2034]
      1 warning: forcedarch is referenced but not assigned. [SC2154]
      1 warning: file appears unused. Verify it or export it. [SC2034]
      1 warning: fetch_command appears unused. Verify it or export it. [SC2034]
      1 warning: extenstion is referenced but not assigned (did you mean 'extension'?). [SC2154]
      1 warning: Don't use variables in the printf format string. Use printf "..%s.." "$foo". [SC2059]
      1 warning: distribution is referenced but not assigned. [SC2154]
      1 warning: default appears unused. Verify it or export it. [SC2034]
      1 warning: component is referenced but not assigned. [SC2154]
      1 warning: chroot appears unused. Verify it or export it. [SC2034]
      1 note: require was modified in a subshell. That change might be lost. [SC2031]
      1 note: Not following: /lib/lsb/init-functions was not specified as input (see shellcheck -x). [SC1091]
      1 note: Not following: /etc/lsb-release was not specified as input (see shellcheck -x). [SC1091]
      1 note: Not following: /etc/default/consul was not specified as input (see shellcheck -x). [SC1091]
      1 note: Modification of require is local (to subshell caused by (..) group). [SC2030]
      1 note: Consider using grep -c instead of grep|wc. [SC2126]
      1 error: The order of the 2>&1 and the redirect matters. The 2>&1 has to be last. [SC2069]
      1 error: Parsing stopped here. Is this keyword correctly matched up? [SC1089]

*Created by: 4nd3r* even if we exclude [SC2154](https://github.com/koalaman/shellcheck/wiki/SC2154) (something is referenced but not assigned), then others should be dealt with in some point. yes, that's lot of work :> thoughts on this? ``` $ find . -type f | while read -r l; do file "$l" | grep -Fq 'POSIX shell script' && shellcheck -f gcc "$l"; done | sed -nr 's/.+:[0-9]+:\s(.+)$/\1/p' | sort | uniq -c | sort -nr 221 note: Double quote to prevent globbing and word splitting. [SC2086] 196 warning: __object is referenced but not assigned. [SC2154] 172 warning: __object_id is referenced but not assigned. [SC2154] 41 warning: __global is referenced but not assigned. [SC2154] 40 warning: __type is referenced but not assigned. [SC2154] 30 warning: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined. [SC2166] 17 warning: __explorer is referenced but not assigned. [SC2154] 17 error: Iterating over ls output is fragile. Use globs. [SC2045] 14 warning: __object_name is referenced but not assigned. [SC2154] 13 warning: Prefer [ p ] || [ q ] as [ p -o q ] is not well defined. [SC2166] 12 warning: Quote this to prevent word splitting. [SC2046] 11 warning: __messages_out is referenced but not assigned. [SC2154] 11 warning: Declare and assign separately to avoid masking return values. [SC2155] 11 note: Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead. [SC2002] 10 warning: __target_host is referenced but not assigned. [SC2154] 9 note: Useless echo? Instead of 'echo $(cmd)', just use 'cmd'. [SC2005] 9 note: $/${} is unnecessary on arithmetic variables. [SC2004] 7 note: Expressions don't expand in single quotes, use double quotes for that. [SC2016] 6 warning: In POSIX sh, 'local' is undefined. [SC2039] 5 warning: Remove surrounding $() to avoid executing output. [SC2091] 5 warning: __remote_exec is referenced but not assigned. [SC2154] 5 warning: Quotes/backslashes in this variable will not be respected. [SC2090] 5 warning: In POSIX sh, echo flags are undefined. [SC2039] 5 note: Use find instead of ls to better handle non-alphanumeric filenames. [SC2012] 4 warning: __remote_copy is referenced but not assigned. [SC2154] 4 note: read without -r will mangle backslashes. [SC2162] 4 note: Not following: /etc/openwrt_release was not specified as input (see shellcheck -x). [SC1091] 3 warning: Use cd ... || exit in case cd fails. [SC2164] 3 warning: __target_host is referenced but not assigned (did you mean 'my_target_host'?). [SC2154] 3 warning: require is referenced but not assigned. [SC2154] 3 warning: Quotes/backslashes will be treated literally. Use an array. [SC2089] 3 warning: options appears unused. Verify it or export it. [SC2034] 3 warning: __messages_in is referenced but not assigned. [SC2154] 3 warning: For loops over find output are fragile. Use find -exec or a while read loop. [SC2044] 3 warning: __cdist_object_marker is referenced but not assigned. [SC2154] 3 note: Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'. [SC2116] 3 note: echo won't expand escape sequences. Consider printf. [SC2028] 3 error: Scripts are case sensitive. Use 'done', not 'DONE'. [SC1081] 3 error: Argument mixes string and array. Use * or separate argument. [SC2145] 2 warning: __target_host is referenced but not assigned (did you mean 'target_host'?). [SC2154] 2 warning: state is referenced but not assigned. [SC2154] 2 warning: state appears unused. Verify it or export it. [SC2034] 2 warning: set_attributes appears unused. Verify it or export it. [SC2034] 2 warning: ruby appears unused. Verify it or export it. [SC2034] 2 warning: output is referenced but not assigned. [SC2154] 2 warning: name appears unused. Verify it or export it. [SC2034] 2 warning: gemsetname appears unused. Verify it or export it. [SC2034] 2 warning: deleteJail references arguments, but none are ever passed. [SC2120] 2 warning: __debug is referenced but not assigned. [SC2154] 2 warning: cur_ip appears unused. Verify it or export it. [SC2034] 2 warning: cksum appears unused. Verify it or export it. [SC2034] 2 note: Use grep -q instead of comparing output with [ -n .. ]. [SC2143] 2 note: Use deleteJail "$@" if function's $1 should mean script's $1. [SC2119] 2 note: The mentioned parser error was in this case expression. [SC1009] 2 note: Don't use [] around ranges in tr, it replaces literal square brackets. [SC2021] 2 error: Unexpected keyword/token. Fix any mentioned problems and try again. [SC1072] 2 error: -f doesn't work with globs. Use a for loop. [SC2144] 2 error: Couldn't parse this case item. [SC1073] 1 warning: __verbose is referenced but not assigned. [SC2154] 1 warning: uri is referenced but not assigned. [SC2154] 1 warning: type is referenced but not assigned (for output from commands, use "$(type ...)" ). [SC2154] 1 warning: __type_explorer is referenced but not assigned. [SC2154] 1 warning: type appears unused. Verify it or export it. [SC2034] 1 warning: target appears unused. Verify it or export it. [SC2034] 1 warning: supported_add_properties appears unused. Verify it or export it. [SC2034] 1 warning: source appears unused. Verify it or export it. [SC2034] 1 warning: service_id is referenced but not assigned. [SC2154] 1 warning: runs is referenced but not assigned. [SC2154] 1 warning: repo_name is referenced but not assigned. [SC2154] 1 warning: relative_script appears unused. Verify it or export it. [SC2034] 1 warning: prefix appears unused. Verify it or export it. [SC2034] 1 warning: Possible misspelling: UNAME may not be assigned, but uname is. [SC2153] 1 warning: path appears unused. Verify it or export it. [SC2034] 1 warning: __object_id is referenced but not assigned (did you mean 'object_id'?). [SC2154] 1 warning: name is referenced but not assigned. [SC2154] 1 warning: my_target_host is referenced but not assigned. [SC2154] 1 warning: In POSIX sh, LINENO is undefined. [SC2039] 1 warning: In POSIX sh, 'caller' is undefined. [SC2039] 1 warning: format appears unused. Verify it or export it. [SC2034] 1 warning: forcedarch is referenced but not assigned. [SC2154] 1 warning: file appears unused. Verify it or export it. [SC2034] 1 warning: fetch_command appears unused. Verify it or export it. [SC2034] 1 warning: extenstion is referenced but not assigned (did you mean 'extension'?). [SC2154] 1 warning: Don't use variables in the printf format string. Use printf "..%s.." "$foo". [SC2059] 1 warning: distribution is referenced but not assigned. [SC2154] 1 warning: default appears unused. Verify it or export it. [SC2034] 1 warning: component is referenced but not assigned. [SC2154] 1 warning: chroot appears unused. Verify it or export it. [SC2034] 1 note: require was modified in a subshell. That change might be lost. [SC2031] 1 note: Not following: /lib/lsb/init-functions was not specified as input (see shellcheck -x). [SC1091] 1 note: Not following: /etc/lsb-release was not specified as input (see shellcheck -x). [SC1091] 1 note: Not following: /etc/default/consul was not specified as input (see shellcheck -x). [SC1091] 1 note: Modification of require is local (to subshell caused by (..) group). [SC2030] 1 note: Consider using grep -c instead of grep|wc. [SC2126] 1 error: The order of the 2>&1 and the redirect matters. The 2>&1 has to be last. [SC2069] 1 error: Parsing stopped here. Is this keyword correctly matched up? [SC1089] ```

ungleich-gitea closed this issue

2021-11-20 15:20:16 +00:00

ungleich-gitea commented

2021-11-20 15:43:39 +00:00

Created by: asteven

Nice work everyone. To be honest I expect some breakage. But we can just fix that along the way.
I do think we should eat this dog food ourself for quite some time before releasing.

*Created by: asteven* Nice work everyone. To be honest I expect some breakage. But we can just fix that along the way. I do think we should eat this dog food ourself for quite some time before releasing.

ungleich-gitea commented

2021-11-20 15:43:40 +00:00

Created by: darko-poljak

@asteven @thriqon @tom-ee @telmich Good news!
If I am right, #712 and #713 should be the last PRs regarding this issue :)

*Created by: darko-poljak* @asteven @thriqon @tom-ee @telmich Good news! If I am right, #712 and #713 should be the last PRs regarding this issue :)

ungleich-gitea commented

2021-11-20 15:43:41 +00:00

Created by: darko-poljak

To coordinate, after resolving https://github.com/ungleich/cdist/pull/708 I will create PR
for another fixes I am preparing, https://github.com/darko-poljak/cdist/tree/shellcheck-rest.
After this one, I think all shellcheck problems will be resolved.
Except SC2154 (var is referenced but not assigned, https://github.com/koalaman/shellcheck/wiki/SC2154) for all __<something> which we will ignore, since such variables are cdist's environment variables.

*Created by: darko-poljak* To coordinate, after resolving https://github.com/ungleich/cdist/pull/708 I will create PR for another fixes I am preparing, https://github.com/darko-poljak/cdist/tree/shellcheck-rest. After this one, I think all shellcheck problems will be resolved. Except SC2154 (var is referenced but not assigned, https://github.com/koalaman/shellcheck/wiki/SC2154) for all `__<something>` which we will ignore, since such variables are cdist's environment variables.

ungleich-gitea commented

2021-11-20 15:43:43 +00:00

Created by: darko-poljak

We can add Makefile target for shellcheck manifest and gencode-*.
Although we won't be able to check code-* scripts.

*Created by: darko-poljak* We can add Makefile target for shellcheck manifest and gencode-\*. Although we won't be able to check code-\* scripts.

ungleich-gitea commented

2021-11-20 15:43:44 +00:00

Created by: 4nd3r

because apt is specific for debian-like GNU/Linux distros which have GNU tools by default

then we should add # shellcheck disable=<code> before that line.

*Created by: 4nd3r* > because apt is specific for debian-like GNU/Linux distros which have GNU tools by default then we should add `# shellcheck disable=<code>` before that line.

ungleich-gitea commented

2021-11-20 15:43:45 +00:00

Created by: darko-poljak

For specific things, like apt types, which uses GNU version of find I think we do not need to port it to portable POSIX shell code, because apt is specific for debian-like GNU/Linux distros which have GNU tools by default.

*Created by: darko-poljak* For specific things, like apt types, which uses GNU version of find I think we do not need to port it to portable POSIX shell code, because apt is specific for debian-like GNU/Linux distros which have GNU tools by default.

ungleich-gitea commented

2021-11-20 15:43:47 +00:00

Created by: darko-poljak

@asteven For mktemp, POSIX does not specify mktemp utility.
Also, -p is not defined for FreeBSD mktemp. TMPDIR env var is used for this, '/tmp' as default if not set.
So I suggest to use:
TMPDIR=/tmp mktemp...

*Created by: darko-poljak* @asteven For mktemp, POSIX does not specify mktemp utility. Also, -p is not defined for FreeBSD mktemp. TMPDIR env var is used for this, '/tmp' as default if not set. So I suggest to use: `TMPDIR=/tmp mktemp...`

ungleich-gitea commented

2021-11-20 15:43:48 +00:00

Created by: darko-poljak

We need to check code generation and fix as many as we notice.
e.g.: __staged_file/gencode-remote generates code that uses mktemp --tmpdir, should be mktemp -p instead.

*Created by: darko-poljak* We need to check code generation and fix as many as we notice. e.g.: __staged_file/gencode-remote generates code that uses mktemp --tmpdir, should be mktemp -p instead.

ungleich-gitea commented

2021-11-20 15:43:49 +00:00

Created by: darko-poljak

@asteven Can you also take a look if you have time?
Here https://github.com/ungleich/cdist/issues/518 will also be handled.

*Created by: darko-poljak* @asteven Can you also take a look if you have time? Here https://github.com/ungleich/cdist/issues/518 will also be handled.

ungleich-gitea commented

2021-11-20 15:43:50 +00:00

Created by: darko-poljak

Hm... shellcheck can not check code that would be generated by gencode unless code is checked after it is generated. Guess this should be fixed when someone reports it. Or we look gencode by gencode and see if we spot something. Ideas?

*Created by: darko-poljak* Hm... shellcheck can not check code that would be generated by gencode unless code is checked after it is generated. Guess this should be fixed when someone reports it. Or we look gencode by gencode and see if we spot something. Ideas?

ungleich-gitea commented

2021-11-20 15:43:52 +00:00

Created by: 4nd3r

woah, that's a big change! it'll take some time to scroll through it. will do.

*Created by: 4nd3r* woah, that's a big change! it'll take some time to scroll through it. will do.

ungleich-gitea commented

2021-11-20 15:43:53 +00:00

Created by: darko-poljak

Can you take a look at https://github.com/ungleich/cdist/compare/master...darko-poljak:shellcheck if you can spot some mistake prior to creating PR?

*Created by: darko-poljak* Can you take a look at https://github.com/ungleich/cdist/compare/master...darko-poljak:shellcheck if you can spot some mistake prior to creating PR?

ungleich-gitea commented

2021-11-20 15:43:54 +00:00

Created by: darko-poljak

@4nd3r I am starting to work on this. I will create a PR with first changes where we can further discuss this.

*Created by: darko-poljak* @4nd3r I am starting to work on this. I will create a PR with first changes where we can further discuss this.

ungleich-gitea referenced this issue

2021-11-20 15:51:52 +00:00

__jail --state absent should imply --stopped #278

Sign in to join this conversation.