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

Re: t5570-git-daemon fails with SIGPIPE on OSX

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Mar 2, 2019, 21:21 UTC
Message-ID
<nycvar.QRO.7.76.6.1903022209110.45@tvgsbejvaqbjf.bet>
In-Reply-To
<20190301190042.GF30847@sigill.intra.peff.net>
Hi Peff,
On Fri, 1 Mar 2019, Jeff King wrote:
Show 14 quoted lines
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index b620fd54b4..4ba63d5ac6 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1556,7 +1556,9 @@ static int fetch_one(struct remote *remote, int argc, const char **argv, int pru
>  
>  	sigchain_push_common(unlock_pack_on_signal);
>  	atexit(unlock_pack);
> +	sigchain_push(SIGPIPE, SIG_IGN);
>  	exit_code = do_fetch(gtransport, &rs);
> +	sigchain_pop(SIGPIPE);
>  	refspec_clear(&rs);
>  	transport_disconnect(gtransport);
>  	gtransport = NULL;
That indeed does the job:
	https://git.visualstudio.com/git/_build/results?buildId=358
Show 16 quoted lines
> That said, I actually think it's kind of pointless for git-fetch to use
> SIGPIPE at all. The purpose of SIGPIPE is that you can write a naive
> program that spews output, and you'll get informed (forcefully) by the
> OS if the process consuming your output stops listening. That makes
> sense for programs like "git log", whose primary purpose is generating
> output.
> 
> But for git-fetch, our primary purpose is receiving data and writing it
> to disk. We should be checking all of our write()s already, and SIGPIPE
> is just confusing. The only "big" output we generate is the status table
> at the end. And even if that is going to a pipe that closes, I don't
> think we'd want to fail the whole command (we'd want to finalize any
> writes for what we just fetched, clean up after ourselves, etc).
> 
> So I'd actually be fine with just declaring that certain commands (like
> fetch) just ignore SIGPIPE entirely.

That's a bigger change than I'm comfortable with, so I'd like to go with tge diff you gave above.

Do you want to turn these two patches into a proper patch series? Otherwise I can take care of it, probably this Monday or Tuesday.

Ciao, Dscho

Previous: Jeff KingNext: Jeff King
Message 12 of 21 in “t5570-git-daemon fails with SIGPIPE on OSX”
  1. SZEDER GáborAug 6, 2018
  2. SZEDER GáborAug 6, 2018
  3. Johannes SchindelinFeb 8, 2019
  4. SZEDER GáborFeb 8, 2019
  5. Jeff KingAug 14, 2018
  6. Jeff KingAug 14, 2018
  7. Johannes SchindelinFeb 8, 2019
  8. Johannes SchindelinFeb 8, 2019
  9. Jeff KingFeb 8, 2019
  10. Johannes SchindelinMar 1, 2019
  11. Jeff KingMar 1, 2019
  12. Johannes SchindelinMar 2, 2019
  13. Jeff KingMar 3, 2019
  14. 1/2 fetch: avoid calling write_or_die()Jeff King, Mar 3, 2019
  15. Duy NguyenMar 4, 2019
  16. Jeff KingMar 5, 2019
  17. 2/2 fetch: ignore SIGPIPE during network operationJeff King, Mar 3, 2019
  18. Junio C HamanoMar 4, 2019
  19. Jeff KingMar 5, 2019
  20. Junio C HamanoMar 3, 2019
  21. Johannes SchindelinMar 3, 2019

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.