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

Jeff King [off-list ref] writes:

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...

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.