Re: [PATCH v4] add-patch: response to unknown command

[Jeff King <peff@peff.net>]

On Wed, Apr 24, 2024 at 10:15:25PM -0400, Eric Sunshine wrote:

> > I got a test failure on Windows CI from this. The test_cmp output looks
> > like this:
> >
> >   -(1/1) Stage addition [y,n,q,a,d,e,p,?]? Unknown command 'W' (use '?' for help)
> >   -(1/1) Stage addition [y,n,q,a,d,e,p,?]?
> >   +(1/1) Stage addition [y,n,q,a,d,e,p,?]? (1/1) Stage addition [y,n,q,a,d,e,p,?]?
> >   +Unknown command 'W' (use '?' for help)
> >
> > which makes me suspect a race. Perhaps because the prompt is going to
> > stdout, but the "Unknown command" message goes to stderr? Maybe we
> > should keep stdout and stderr separate and check them independently.
> 
> That's very reminiscent of [1]. Although, unlike [1], the output
> presented to the user in this case is (I suppose) less likely to be
> messed up; only the combined captured output is probably affected. So,
> capturing stdout and stderr separately would indeed be a good idea.

Ah, yeah, that is almost certainly the same issue. I could not reproduce
it on my local system with --stress, so I think it may be unique to
Windows. What it looks like is that stderr is buffered when output to a
file (whereas on most systems it would still be line buffered at most).
But I didn't actually run it through a debugger to see.

So I _suspect_ that yet another possible solution here is to do an
explicit:

  setvbuf(stderr, NULL, _IOLBF, BUFSIZ);

in common-main. Though grepping for setvbuf() in our code base, it looks
like compat/winansi.c already does:

          if (fd == 2)
                setvbuf(stderr, NULL, _IONBF, BUFSIZ);

which would imply that output should happen even more immediately than
line-buffering. So maybe there is some other layer at work. There are
probably dragons, and definitely pursuing that line would involve some
experimentation.

The "if you want them in order, make sure they are on the same stream"
approach suggested elsewhere is by comparison much simpler. :)

-Peff