git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Dec 29, 2015, 21:34 UTC
Message-ID
<xmqqlh8dgcr2.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20151229081222.GB9254@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 6 quoted lines
> 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.
Show 64 quoted lines
> -- >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
Previous: Jeff KingNext: Nguyễn Thái Ngọc Duy
Message 34 of 39 in “Fwd: Git clone fails during pre-commit hook due to GIT_WORK_TREE=. (regression 2.5 -> 2.6)”
  1. Anthony SottileNov 24, 2015
  2. Stefan BellerNov 24, 2015
  3. Duy NguyenNov 25, 2015
  4. Duy NguyenNov 30, 2015
  5. Junio C HamanoNov 30, 2015
  6. Duy NguyenDec 1, 2015
  7. Junio C HamanoDec 2, 2015
  8. 1/2 git.c: make it clear save_env() is for alias handling onlyNguyễn Thái Ngọc Duy, Dec 3, 2015
  9. 2/2 setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when ..Nguyễn Thái Ngọc Duy, Dec 3, 2015
  10. Junio C HamanoDec 4, 2015
  11. Duy NguyenDec 5, 2015
  12. 3/2 git.c: make sure we do not leak GIT_* to alias scriptsNguyễn Thái Ngọc Duy, Dec 5, 2015
  13. Junio C HamanoDec 7, 2015
  14. Duy NguyenDec 8, 2015
  15. Jeff KingDec 8, 2015
  16. Junio C HamanoDec 8, 2015
  17. Duy NguyenDec 5, 2015
  18. Junio C HamanoDec 7, 2015
  19. 0/3 nd/clear-gitenv-upon-use-of-aliasNguyễn Thái Ngọc Duy, Dec 20, 2015
  20. 1/3 git.c: make it clear save_env() is for alias handling onlyNguyễn Thái Ngọc Duy, Dec 20, 2015
  21. 2/3 setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE when ..Nguyễn Thái Ngọc Duy, Dec 20, 2015
  22. 3/3 git.c: make sure we do not leak GIT_* to alias scriptsNguyễn Thái Ngọc Duy, Dec 20, 2015
  23. Junio C HamanoDec 21, 2015
  24. Duy NguyenDec 22, 2015
  25. Duy NguyenDec 22, 2015
  26. Junio C HamanoDec 22, 2015
  27. Jeff KingDec 23, 2015
  28. Duy NguyenDec 23, 2015
  29. Eric SunshineDec 23, 2015
  30. Johannes SixtDec 23, 2015
  31. Jeff KingDec 23, 2015
  32. Duy NguyenDec 24, 2015
  33. Jeff KingDec 29, 2015
  34. Junio C HamanoDec 29, 2015
  35. Revert "setup: set env $GIT_WORK_TREE when work tree is set, like $GIT_DIR"Nguyễn Thái Ngọc Duy, Dec 21, 2015
  36. Junio C HamanoDec 21, 2015
  37. Junio C HamanoDec 21, 2015
  38. Duy NguyenDec 22, 2015
  39. Junio C HamanoDec 22, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.