Samba and ShellCheck

[Martin Schwenke]

On Mon, 29 Aug 2022 09:01:05 +0200, Andreas Schneider <asn at samba.org>
wrote:

> On Tuesday, 23 August 2022 10:02:43 CEST Martin Schwenke via samba-technical 
> wrote:

> > I use an "excellent" convention for local variables in /bin/sh scripts,
> > specially designed to reduce readability.  ;-)  I prefix any local
> > variable with '_'.  However, it makes no semantic difference, it just
> > acts as a hint.  
> 
> Either we convert everything to this convention or we disable the shellcheck 
> warning in those scripts. What do you prefer?

I wouldn't want to force that convention on anyone.  Out of those 2
choices, I would say disable the warning.

> ./script/check-shell-scripts.sh $(pwd) warning >shell.log 2>&1
> 
> 
> Example to disable the warning:
> 
> 
>  ctdb/tests/run_tests.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ctdb/tests/run_tests.sh b/ctdb/tests/run_tests.sh
> index ff431f8831f..12e495e3dc0 100755
> --- a/ctdb/tests/run_tests.sh
> +++ b/ctdb/tests/run_tests.sh
> @@ -1,4 +1,5 @@
>  #!/usr/bin/env bash
> +# shellcheck disable=SC3043
>  
>  usage() {
>      cat <<EOF

It looks like check-shell-scripts.sh is wrong.  It should not force
POSIX shell via --shell=sh.  shellcheck passes on
ctdb/tests/run_tests.sh with no options.  It is a bash script and
shellcheck can figure that out.  There are other shell scripts in ctdb/
historically named with a .sh suffix but which shellcheck can't figure
out the type of, and they would be a problem.

Note that in ctdb/ we already have ctdb/tests/UNIT/shellcheck/, which
contains tests to run shellcheck (with some exclusions) on all scripts
that are known to pass.  Try:

  ./ctdb/tests/run_tests.sh ./ctdb/tests/UNIT/shellcheck

I have WIP patches that add coverage for more scripts.

I think ctdb/ is relatively under control.  We have been improving
shellcheck compliance for quite a few years (since 2016, apparently).
:-)

peace & happiness,
martin