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

From
Eric Sunshine <hidden>
Date
2021-11-30 14:06:07
Reply to
Jeff King, 2021-11-30
On Tue, Nov 30, 2021 at 2:21 AM Jeff King [off-list ref] wrote:
On Mon, Nov 29, 2021 at 11:39:46PM -0500, Eric Sunshine wrote:
quoted
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.
Thread (10 messages) 10 messages, 3 authors, 2021-12-01
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help