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

Re: [PATCH] bundle: dup() output descriptor closer to point-of-use

From
Junio C Hamano <gitster@pobox.com>
Date
Nov 17, 2018, 07:05 UTC
Message-ID
<xmqqk1lci3p3.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<20181116094358.GA6054@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 12 quoted lines
> Actually, I realized there's an even simpler way to do this. Here it is.
>
> -- >8 --
> Subject: [PATCH] bundle: dup() output descriptor closer to point-of-use
>
> When writing a bundle to a file, the bundle code actually creates
> "your.bundle.lock" using our lockfile interface. We feed that output
> descriptor to a child git-pack-objects via run-command, which has the
> quirk that it closes the output descriptor in the parent.
>
> To avoid confusing the lockfile code (which still thinks the descriptor
> is valid), we dup() it, and operate on the duplicate.
Yes...
Show 11 quoted lines
> We can solve this by moving the dup() much closer to start_command(),
> shrinking the window in which we have the second descriptor open. It's
> easy to place this in such a way that no die() is possible. We could
> still die due to a signal in the exact wrong moment, but we already
> tolerate races there (e.g., a signal could come before we manage to put
> the file on the cleanup list in the first place).
>
> As a bonus, this shields create_bundle() itself from the duplicate-fd
> trick, and we can simplify its error handling (note that the lock
> rollback now happens unconditionally, but that's OK; it's a noop if we
> didn't open the lock in the first place).

... I found this way too clever for me, but by following the codeflow, it was easy to convince myself that this does the right thing. Almost magical ;-)

Will queue, with a Tested-by: with Dscho's name on it.
TAhanks, both.
Previous: Johannes SchindelinNext: Duy Nguyen
Message 17 of 20 in “bundle: fix issue when bundles would be empty”
  1. 0/1 bundle: fix issue when bundles would be emptyJohannes Schindelin via GitGitGadget, Nov 13, 2018
  2. 1/1 bundle: refuse to create empty bundleGaël Lhez via GitGitGadget, Nov 13, 2018
  3. Stefan BellerNov 13, 2018
  4. Gaël LhezNov 13, 2018
  5. 0/1 bundle: fix issue when bundles would be emptyJohannes Schindelin via GitGitGadget, Nov 14, 2018
  6. 1/1 bundle: cleanup lock files on errorGaël Lhez via GitGitGadget, Nov 14, 2018
  7. Martin ÅgrenNov 14, 2018
  8. Stefan BellerNov 14, 2018
  9. Jeff KingNov 15, 2018
  10. Johannes SchindelinNov 15, 2018
  11. Jeff KingNov 15, 2018
  12. Johannes SchindelinNov 15, 2018
  13. Jeff KingNov 15, 2018
  14. Johannes SchindelinNov 15, 2018
  15. bundle: dup() output descriptor closer to point-of-useJeff King, Nov 16, 2018
  16. Johannes SchindelinNov 16, 2018
  17. Junio C HamanoNov 17, 2018
  18. Duy NguyenNov 15, 2018
  19. Stefan BellerNov 13, 2018
  20. Johannes SchindelinNov 14, 2018

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.