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

On Thu, Dec 24, 2015 at 04:35:33PM +0700, Duy Nguyen wrote:

Sorry if I was confusing; most of the examples in my previous message are about the SIGPIPE thing. I was having trouble triggering the message in practice, even for externals (because the error message goes to the pager, too!).

Thanks, this is the part I was missing.

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.

-- >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(-)