Re: [PATCH 0/5] chainlint: improve robustness against "unusual" shell coding

[Jeff King <peff@peff.net>]

On Tue, Aug 07, 2018 at 04:21:30AM -0400, Eric Sunshine wrote:

> This series improves chainlint's robustness when faced with the sort of
> unusual shell coding in contrib/subtree/t7900 which triggered a
> false-positive, as reported by Jonathan[1]. Jonathan has already
> rewritten[2] that code to be cleaner and more easily understood (and,
> consequently, to avoid triggering the false-positive), thus the
> improvements in this series are not strictly necessary.
> 
> Nevertheless, it seems prudent to make chainlint more robust against
> such unusual coding as an aid to future less-experienced test writers,
> making it less likely for them to trigger a false-positive and waste
> time trying to decipher a non-existent problem (in their code).
> 
> In [3], I said that 'sed' couldn't "be coerced" into dealing with nested
> here-docs with arbitrary tag names (explaining why it recognized only a
> "blessed" set of hard-coded names). However, I put a bit of thought into
> it and figured out how to do it. Patch 1/5 is the result.
> 
> This applies atop 'master'.

I had two minor comments on the first patch. I'll admit my eyes glazed
over looking at the rest of them, and to make any kind of intelligent
review I'd need to spend an hour understanding how the sed script works.
Which frankly, I'm not sure is worth it. Given the empirical results
(both on the real code base and the new tests you add) and the low-risk
nature (it's linting our tests, after all, not code users run), I'd be
inclined to say it's not making anything worse, and probably making
things better. We can find out about any further short-comings in the
wild.

-Peff