Re: [PATCH 3/4] tests: drop here-doc check from internal chain-linter

From: Jeff King <peff@peff.net>
Date: Mar 29, 2023, 06:28 UTC
Message-ID: <20230329062843.GC1793752@coredump.intra.peff.net>
In-Reply-To: <CAPig+cSMYt81fcU8fRNhN9qZ=95jvRNv9YGhkM6PxgdMywWnCA@mail.gmail.com>

On Tue, Mar 28, 2023 at 11:07:17PM -0400, Eric Sunshine wrote:

Show 13 quoted lines

> > I just think chainlint.pl is doing a good enough job of catching it that
> > we can rely on it. I'll be curious if Eric has input there on whether it
> > can do even better, which would remove all of the caveats from the
> > commit message.
> 
> It was an intentional design choice, for the sake of simplicity, _not_
> to make chainlint.pl a shell syntax checker, despite the fact that it
> is genuinely parsing shell code. After all, the shell itself -- by
> which test code will ultimately be executed -- is more than capable of
> diagnosing syntax errors, so why burden chainlint.pl with all that
> extra baggage? Instead, chainlint.pl is focussed on detecting semantic
> problems -- such as broken &&-chain and missing `return 1` -- which
> are of importance to _this_ project.

Yeah, I'm not too surprised to hear any of that, and I almost wrote off chainlint.pl for this purpose (hence the hand-waving in my commit message). But after realizing it has to find here-docs anyway to ignore them, it seemed reasonable to bend it to my will. ;)

Thanks again for your patch.

> [1]: Why is that, by the way? Is `eval` not complaining about the
> unclosed here-doc? Is that something that can be fixed more generally?
> Are there other syntactic problems that go unnoticed?

The behavior varies from shell to shell. Historically, I suspect it was a deliberate decision to read until EOF, because it lets you stick arbitrary data at the end of a script, like:

  $ cat foo.sh
  my_prog <<\EOF
  1 2 3 4
  5 6 7 8
  [imagine kilobytes of data tables here]

without having to worry about terminating it. I think I've seen it with something like:

  {
    echo "uudecode <<\EOF | tar tf -"
    tar cf - Documentation | uuencode -
  } >foo.sh

which makes foo.sh a sort of self-extracting tarball. (As an aside, I was disappointed that I did not have uuencode installed by default on my system. How times have changed. ;) ).

These days bash will warn about it, but dash will not:

  $ bash foo.sh | wc -l
  foo.sh: line 129028: warning: here-document at line 1 delimited by end-of-file (wanted `EOF')
  901

  $ dash foo.sh | wc -l
  901

Bash still has a zero exit code, though, so aside from the stderr cruft (which is hidden unless you are digging in "-v" output), the tests would pass. I can't remember when bash started warning, though "git log -S" in its repo suggests it was bash 4.0 in 2009.

-Peff