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

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