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

[RFC/PATCH] avoid SIGPIPE warnings for aliases

From
Jeff King <peff@peff.net>
Date
Jan 4, 2013, 12:47 UTC
Message-ID
<20130104124756.GA402@sigill.intra.peff.net>

When git executes an alias that specifies an external command, it will complain if the alias dies due to a signal. This is usually a good thing, as signal deaths are unexpected. However, SIGPIPE is not unexpected for many commands which produce a lot of output; it is intended that the user closing the pager would kill them them via SIGPIPE.

As a result, the user might see annoying messages in a scenario like this:

  $ cat ~/.gitconfig
  [alias]
  lgbase = log --some-options
  lg = !git lgbase --more-options
  lg2 = !git lgbase --other-options
  $ git lg -p
  [user hits 'q' to exit pager]
  error: git lgbase --more-options died of signal 13
  fatal: While expanding alias 'lg': 'git lgbase --more-options': Success

Many users won't see this, because we execute the external command with the shell, and a POSIX shell will silently rewrite the signal-death exit code into 128+signal, and we will treat it like a normal exit code. However, this does not always happen:

  1. If the sub-command does not have shell metacharacters,
     we will skip the shell and exec it directly, getting
     the signal code.
  2. Some shells do not do this rewriting; for example,
     setting SHELL_PATH set to zsh will reveal this problem.

This patch squelches the message, and let's git exit silently (with the same exit code that a shell would use in case of a signal).

The first line of the message comes from run-command's wait_or_whine. To silence that message, we remain quiet anytime we see SIGPIPE.

The second line comes from handle_alias itself. It calls die_errno whenever run_command returns a negative value. However, only -1 indicates a syscall error where errno has something useful (note that it says the useless "success" above). Instead, we treat negative returns from run_command (except for -1) as a normal code to be passed to exit.

Signed-off-by: Jeff King <peff@peff.net>
---
I have two reservations with this patch:
  1. We are ignoring SIGPIPE all the time. For an alias that is calling
     "log", that is fine. But if pack-objects dies on the server side,
     seeing that it died from SIGPIPE is useful data, and we are
     squelching that. Maybe callers of run-command should have to pass
     an "ignore SIGPIPE" flag?
  2. The die_errno in handle_alias is definitely wrong. Even if we want
     to print a message for signal death, showing errno is bogus unless
     the return value was -1. But is it the right thing to just pass the
     negative value straight to exit()? It works, but it is depending on
     the fact that (unsigned char)(ret & 0xff) behaves in a certain way
     (i.e., that we are on a twos-complement platform, and -13 becomes
     141). That is not strictly portable, but it is probably fine in
     practice. I'd worry more about exit() doing something weird on
     Windows.
 git.c         | 2 +-
 run-command.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/git.c b/git.c
index d33f9b3..07edb8a 100644
--- a/git.c
+++ b/git.c
@@ -175,7 +175,7 @@ static int handle_alias(int *argcp, const char ***argv)
 			alias_argv[argc] = NULL;
 
 			ret = run_command_v_opt(alias_argv, RUN_USING_SHELL);
-			if (ret >= 0)   /* normal exit */
+			if (ret != -1)  /* normal exit */
 				exit(ret);
 
 			die_errno("While expanding alias '%s': '%s'",
diff --git a/run-command.c b/run-command.c
index 757f263..01a4c9b 100644
--- a/run-command.c
+++ b/run-command.c
@@ -242,7 +242,7 @@ static int wait_or_whine(pid_t pid, const char *argv0)
 		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
-- 
1.8.1.rc1.16.g6d46841
Next: Johannes Sixt
Message 1 of 23 in “avoid SIGPIPE warnings for aliases”
  1. avoid SIGPIPE warnings for aliasesJeff King, Jan 4, 2013
  2. Johannes SixtJan 4, 2013
  3. Jeff KingJan 4, 2013
  4. Junio C HamanoJan 4, 2013
  5. Jeff KingJan 5, 2013
  6. run-command: encode signal death as a positive integerJeff King, Jan 5, 2013
  7. Johannes SixtJan 5, 2013
  8. Jonathan NiederJan 5, 2013
  9. Jeff KingJan 5, 2013
  10. Jonathan NiederJan 5, 2013
  11. Junio C HamanoJan 6, 2013
  12. Junio C HamanoJan 9, 2013
  13. Jeff KingJan 9, 2013
  14. Junio C HamanoJan 9, 2013
  15. Jonathan NiederJan 10, 2013
  16. Junio C HamanoJan 10, 2013
  17. Jeff KingJan 10, 2013
  18. Junio C HamanoJan 10, 2013
  19. Jeff KingJan 10, 2013
  20. Johannes SixtJan 10, 2013
  21. Junio C HamanoJan 10, 2013
  22. Jeff KingJan 10, 2013
  23. mimimimiJul 21, 2014

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.