Re: [PATCH v3 1/5] t3701: redefine what is "bogus" output of a diff filter

[Jeff King <peff@peff.net>]

On Wed, Aug 31, 2022 at 10:26:17AM +0100, Phillip Wood wrote:

> > Is it that all it cares about is that the output has the same number
> > of lines as the input?  If so, I agree that it is much less realistic,
> > but it may not matter in practice.  Even an "exit 0" might do ;-)
> 
> Yes I think it only cares that there are the same number of lines which begs
> the question "why are we changing this test in the first place?". The commit
> message talks about the being unable to parse the hunk header but that's
> only true because we would be looking in the wrong place for it - the output
> does in fact contain a valid hunk header. With this change there is no hunk
> header in the filtered output at all. In practice if the number of lines
> differs it is normally because the filter messed with the diff header and
> removed some lines from it which is what the existing test simulates.
> 
> I'm struggling to understand the need for this change from the explanation
> in the commit message as it seems to me  to assume the line being deleted is
> the hunk header when in fact it is deleting the diff header.

FWIW, as the author of the original test, I'm also confused about why it
needs to be changed. The filter I wrote in the original test was just
"echo too-short". It was switched to "sed 1d" because the original did
not read the input at all, which racily caused Git to see SIGPIPE:

  https://lore.kernel.org/git/20200113170417.GK32750@szeder.dev/

Switching to "exit 0" would bring that problem back. But I think "sed q"
potentially does, too, because sed will quit without reading all of the
input. We really do want something like "sed 1d" that changes the number
of lines, but ensures that the filter reads to EOF.

-Peff