[PATCH] update autogen-waf.sh

Martin Schwenke martin at meltin.net
Fri Aug 11 10:32:11 UTC 2017


On Fri, 11 Aug 2017 11:21:00 +0100, Rowland Penny via samba-technical
<samba-technical at lists.samba.org> wrote:

> On Fri, 11 Aug 2017 18:53:16 +1000
> Martin Schwenke <martin at meltin.net> wrote:
> 
> > Hi Rowland,
> > 
> > On Thu, 10 Aug 2017 14:21:34 +0100, Rowland Penny via
> > samba-technical <samba-technical at lists.samba.org> wrote:
> >   
> > > OK, here is the first one, this is basically just running the script
> > > through 'shellcheck' and fixing any errors found, it should not
> > > change how the script works.  
> > 
> > It all looks good.  Can I suggest a minor improvement in a few places?
> > 
> > Instead of doing:
> > 
> >   "$p"/Makefile
> > 
> > it is probably better to do:
> > 
> >   "$p/Makefile"
> > 
> > That way the quotes are around the whole string and future edits are
> > less likely to be fragile.
> > 
> > However, you need to be careful with examples like this:
> > 
> >   "$p"/include/config*.h*
> > 
> > If you double-quote the wildcards then they don't get expanded.  There
> > are a few answers to this, including:
> > 
> >   "$p/include/config"*.h*
> > 
> > Deciding where to end the quotes is quite subjective.  :-)
> > 
> > Personally, I tend to "over-punctuate" my shell scripts.  I would
> > write the first example as:
> > 
> >   "${p}/Makefile"  
> 
> I understand all this Martin and as I said earlier, where do you stop ?

Cool.  I'm just over-communicating given the usual time turnaround
for long distance email conversations...  :-)

> > While the braces aren't necessary in this case, I believe that they
> > make the variable expansion more obvious... especially in cases where
> > there are multiple expansions in a string.  The only place I don't use
> > the braces is when the variable expansion is the whole string, as in
> > "$p".  
> 
> I normally use them in every case, just in case ;-) but in this case,
> I am just following 'shellcheck' and fixing obvious errors.

Yeah, shellcheck often provides hints that the code could be better.

> I think I will start again, follow 'shellcheck' but add what I would
> normally do and also fix any errors. I will also take on board what you
> say about the ctdb tests and ignore them.

I think that's a good move.

That said, if you really do want to attack the CTDB tests then I think
the order could be:

1. Fix shellcheck hits in unit tests
2. Fix shellcheck hits in the (bash) integration tests
3. Convert bash to POSIX sh

However, it is a major undertaking...

TV time.  :-)

peace & happiness,
martin



More information about the samba-technical mailing list