Shell script indentation

[Martin Schwenke]

Hi Andreas,

On Thu, 10 Feb 2022 20:27:30 +0100, Andreas Schneider via
samba-technical <samba-technical at lists.samba.org> wrote:

> our shell scripts are a mess and we should try to fix this. So the first 
> questions is what do we want to use for indentation.
> 
> Most of the Samba scripts use 4 spaces.
> CTDB uses tabs (tabwidth: 8).
> 
> We need to settle on one. Should we first collect arguments or directly start 
> a vote?
> 
> 4 spaces or tabs (tw: 8)

A prominent statement near the top of README.Coding.md says:

* Use 8 Space Tabs to Indent

This is in the context of C coding style.  However, I'd suggest that
we attempt to be consistent for languages that have no official coding
style.

When Amitay and I started trying to ensure that CTDB's C code met
Samba's coding guidelines, we also changed the default for shell
scripts... to be consistent.  We have spent a lot of time moving
scripts from 4 spaces to tabs.

Although a small percentage of overall Samba code is shell scripts:

  sh:           65132 (1.72%)

a much more significant percentage of CTDB is shell scripts:

  sh:           28004 (21.72%)

Most of that is test code:

  sh:           23160 (51.79%)

[Above data generated using David A. Wheeler's 'SLOCCount'.]

More than 40% of the shell script in Samba is in CTDB... and, yes, 1/2
of our test code is shell scripts.

Rather than focusing on style, I suggest that we attempt to use
ShellCheck to improve shell scripts.  I realise this is much harder
because it requires functional code changes rather than just formatting
changes.

Currently about 1/3 of CTDB's non-test scripts and a much smaller
percentage of test scripts are checked using ShellCheck (with some
exclusions to avoid churn on some trivial and too-hard things).

A few years ago I added the -S option to ShellCheck to filter the
minimum "error" level (e.g. error, warning, info, style) with the hope
of using ShellCheck in Samba CI testing, starting by switching this on
at error level.

CTDB does quite amazingly on errors:

  $ find -name "*.sh" | xargs shellcheck -s sh -S error
  $ find -name "*.bash" | xargs shellcheck -S error
  $ shellcheck -s sh -S error config/functions 
  $ shellcheck -s sh -S error tools/onnode 
  $ 

Not quite so well on warnings, but fixing most of those should only be a
few hours of work.  I say "most" because the scripts use a lot of
include files, with functions that set globals, so there are a lot
of SC2154 (var is referenced but not assigned) because ShellCheck can't
follow the "non-constant includes".

Sadly, I don't have time to fix the other warnings right now...  :-(

So, I'll vote for tabs and ShellCheck filtering at error level.  :-D

peace & happiness,
martin