Samba and ShellCheck

[Rowland Penny]

On Mon, 2022-08-29 at 21:47 +1000, Martin Schwenke via samba-technical
wrote:
> 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. 


+1 from me about that, from the shellcheck manpage:

 -s shell, –shell=shell
              Specify Bourne shell dialect.  Valid values are sh, bash,
dash and ksh.  The default is to use the file's shebang, or bash if the
target shell can't be determined.

I think you are forcing 'sh' to be used, even when it is a 'bash'
script.

Rowland

>  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
>