shellcheck

[Andrew Bartlett]

On Mon, 2018-01-29 at 13:21 +1100, Martin Schwenke wrote:
> On Mon, 29 Jan 2018 07:12:06 +1100, Andrew Bartlett via samba-technical
> <samba-technical at lists.samba.org> wrote:
> 
> > On Sun, 2018-01-28 at 18:55 +0100, Timur I. Bakeyev wrote:
> > > I was wondering myself what is the state of my patches? Do I need to do anything else to get them pushed into upstream?
> > > 
> > > Error you mentioned is a good example why we should eliminate bash-isms(or switch entirely to bash in the tests to avoid such problems).  
> > 
> > This one Andreas raised just before the release because this overall
> > test now fails for him on Fedora, but not Ubuntu. 
> > 
> > We either need to do that (use bash explicitly) or need to have a way
> > to ensure these don't come back. 
> > 
> > One approach would be to put ShellCheck (with an ignore pattern or
> > expected set of errors) into make test, alongside the python pep8
> > checks.  
> 
> I think that this is generally a good idea.  We did this to all the
> non-test scripts in CTDB a while ago.
> 
> However, it was a lot of work, so I'd like to preempt any idea that we
> mandate the use of ShellCheck.  

OK.

> It is a very blunt instrument that is
> constantly changing.  

Ouch.

> The maintainers sometimes add checks that are of
> limited value.  There was also a regression in version 0.4.5 (fixed in
> 0.4.7) but this is not yet in Debian (which is still at 0.4.6), so I
> see see one test failure when I run on my laptop.
> 
> I think the approach should be to use ShellCheck on scripts that are
> "known good" to ensure that we don't get any regressions.  I would love
> for CTDB's test scripts to pass ShellCheck but that would require more
> time than I'm willing to spend on it right now.

OK.  I'm mainly interested in asserting that we don't go backwards, a
bit like we do with the cc warnings -> errors setting.

> What I think ShellCheck desperately needs is a setting for the
> level of compliance to be checked.  That way you could use it for
> syntax checking and basic style, without having to check all of the
> esoteric style violations.  Even better would be to have the checks
> grouped in a way that you could exclude groups by name or by check
> wildcard.
> 
> We would also need to decide what to do when ShellCheck is not
> available.  Given that it is written in Haskell, it isn't the simplest
> thing in the world to build... or patch.  For the CTDB shellcheck
> test-suite we just print a message and pass the tests if ShellCheck
> isn't installed.  This is probably what happens with our autobuilds on
> sn-devel.

I was hoping that would just mean that test fails, which isn't a big
drama if you are clearly informed why.  The tests we have that auto-
skip if a dependency is missing have sometimes actually been disabled
long-term (ouch!). 

Andrew Bartlett
-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba