Re: [PATCH v2 0/3] nd/clear-gitenv-upon-use-of-alias

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> The outer git wrapper doesn't start the pager, so its stderr still gets
> seen by the user. But the _inner_ git-log does start the pager, and then
> dies of SIGPIPE.
>
> So yeah, I think we want something like this on top of
> nd/clear-gitenv-upon-use-of-alias.

That makes sense to me.

> -- >8 --
> Subject: [PATCH] run-command: don't warn on SIGPIPE deaths
>
> When git executes a sub-command, we print a warning if the
> command dies due to a signal, but make an exception for
> "uninteresting" cases like SIGINT and SIGQUIT (since the
> user presumably just hit ^C).
>
> We should make a similar exception for SIGPIPE, because it's
> an expected and uninteresting return in most cases; it
> generally means the user quit the pager before git had
> finished generating all output.  This used to be very hard
> to trigger in practice, because:
>
>   1. We only complain if we see a real SIGPIPE death, not
>      the shell-induced 141 exit code. This means that
>      anything we run via the shell does not trigger the
>      warning, which includes most non-trivial aliases.
>
>   2. The common case for SIGPIPE is the user quitting the
>      pager before git has finished generating all output.
>      But if the user triggers a pager with "-p", we redirect
>      the git wrapper's stderr to that pager, too.  Since the
>      pager is dead, it means that the message goes nowhere.
>
>   3. You can see it if you run your own pager, like
>      "git foo | head". But that only happens if "foo" is a
>      non-builtin (so it doesn't work with "log", for
>      example).
>
> However, it may become more common after 86d26f2, which
> teaches alias to re-exec builtins rather than running them
> in the same process. This case doesn't trigger (1), as we
> don't need a shell to run a git command. It doesn't trigger
> (2), because the pager is not started by the original git,
> but by the inner re-exec of git. And it doesn't trigger (3),
> because builtins are treated more like non-builtins in this
> case.
>
> Given how flaky this message already is (e.g., you cannot
> even know whether you will see it, as git optimizes out some
> shell invocations behind the scenes based on the contents of
> the command!), and that it is unlikely to ever provide
> useful information, let's suppress it for all cases of
> SIGPIPE.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  run-command.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/run-command.c b/run-command.c
> index 13fa452..694a6ff 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -245,7 +245,7 @@ static int wait_or_whine(pid_t pid, const char *argv0, int in_signal)
>  		error("waitpid is confused (%s)", argv0);
>  	} else if (WIFSIGNALED(status)) {
>  		code = WTERMSIG(status);
> -		if (code != SIGINT && code != SIGQUIT)
> +		if (code != SIGINT && code != SIGQUIT && code != SIGPIPE)
>  			error("%s died of signal %d", argv0, code);
>  		/*
>  		 * This return value is chosen so that code & 0xff