Re: [PATCH v3 01/10] built-in add -i/-p: treat SIGPIPE as EOF

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 15, 2020 at 10:32:59AM -0800, Junio C Hamano wrote:

> >>   test_config interactive.diffFilter "cat >/dev/null ; echo too-short" &&
> >> 
> >> made the test reliable, with over 2000 --stress repetitions, and that
> >> with only a single "y" on 'git add's stdin.
> >
> > Yeah, I agree the test should be changed. What you wrote above was my
> > first thought, too, but I think "sed 1d" is actually a more realistic
> > test (and is shorter and one fewer process).
> 
> I am not sure what we are aiming for.  Are we making sure the
> command behaves well in the hands of end users, who may write a
> script that consumes only early parts of the input that is needed
> for its use and stops reading, or are we just aiming to claim "all
> our tests pass"?  I was hoping that we would be doing the former,
> and I would understand if the suggestion were "sed 1q" for that
> exact reason.
> 
> IOW, shouldn't we be fixing the part that drives the external
> process, so that the test "passes" even with such a "broken" filter?

The original motivation for this test (and the code that fixes it) was
diff-so-fancy, which read all of the input but didn't have a 1:1 line
correspondence in the output (IIRC it condensed some particular lines,
like rename from/to into a single line).

And I think most sane filters would end up reading all of the content.
Or a misconfiguration would cause them to read nothing at all.

So something like "sed 1d" is more representative of a real filter. If
we want to test SIGPIPE, then the current one that reads _nothing_ is
the most torturous. But "sed 1q" is neither realistic (if that's what
we're going for) nor the hardest thing we can throw at the code (if
that's what we want).

> > I've been tempted to say that we should just ignore SIGPIPE everywhere,
> > and convert even copious-output programs like git-log to just check for
> > errors (they could probably even just check ferror(stdout) for each
> > commit we output, if we didn't want to touch every printf call).
> 
> Yeah, I share that temptation.

Hmm. My recollection was that you were more of a fan of SIGPIPE than I
am. But if you agree, then maybe the time has come for action. :)

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux