Re: [RFC PATCH] vreportf: ensure sensible ordering of normal and error output

[Eric Sunshine <sunshine@sunshineco.com>]

On Tue, Nov 30, 2021 at 2:21 AM Jeff King <peff@peff.net> wrote:
> On Mon, Nov 29, 2021 at 11:39:46PM -0500, Eric Sunshine wrote:
> > Rather than attempting to address this issue on a case by case basis,
> > address it by making vreportf() -- which is the heart of error-reporting
> > functions die(), error(), warn(), etc. -- flush stdout before emitting
> > the error message to stderr.
>
> I left some thoughts on whether this flush is safe elsewhere in the
> thread. But for this particular case, two things occur to me:
>
>   - shouldn't status messages like this go to stderr anyway? I know some
>     people follow the "unless it is an error, it should not to go
>     stderr" philosophy. But I think in general our approach in Git is
>     more "if it is the main output of the program, it goes to stdout; if
>     it is chatter or progress for the user, it goes to stderr".

I considered this as well and agree that it would be a nicer localized
fix, but...

(1) I don't think the practice is documented anywhere, so people --
including me when I wrote builtin/worktree.c -- might not know about
it. Indeed, we don't seem to be entirely consistent about doing it
this way. Randomly picking submodule-helper.c, for instance, I see
status-like messages going to stdout:

    printf(_("Entering '%s'\n"), displaypath);
    printf(_("Synchronizing submodule url for '%s'\n"), ...);

    if (...)
        format = _("Cleared directory '%s'\n");
    else
        format = _("Could not remove submodule work tree '%s'\n");
    printf(format, displaypath);

(2) With git-worktree being four or five years old, for
backward-compatibility concerns, I worry that "that ship has sailed",
where 'that' is the freedom to relocate those status-like messages
from stdout to stderr. I don't want to break tooling which exists
around git-worktree.

I'd be happy to be wrong on the second point -- indeed, git-worktree
is still marked "experimental" in the man-page, but that may not mean
anything this late in the game -- and submit a patch which places
git-worktree's status-like messages on stderr instead of stdout.
Thoughts?

>   - the reason it works consistently on glibc is that stdout to a
>     terminal is line buffered by default, so the "preparing" line is
>     flushed immediately. If that isn't the case on Windows, should we
>     consider calling setlinebuf() preemptively when isatty(1)?

I'll let the Windows experts chime in on this (Dscho?). For all I
know, that might introduce a bad performance regression on that
platform under whatever terminal-like or pseudo-tty-like emulation
they are using.