Re: fetching packs and storing them as packs

From: Linus Torvalds <torvalds@osdl.org>
Date: Oct 27, 2006, 20:22 UTC
Message-ID: <Pine.LNX.4.64.0610271310450.3849@g5.osdl.org>
In-Reply-To: <4540CA0C.6030300@tromer.org>

On Thu, 26 Oct 2006, Eran Tromer wrote:

Show 7 quoted lines

>
> This creates a race condition w.r.t. "git repack -a -d", similar to the
> existing race condition between "git fetch --keep" and
> "git repack -a -d". There's a point in time where the new pack is stored
> but not yet referenced, and if "git repack -a -d" runs at that point it
> will eradicate the pack. When the heads are finally updated, you get a
> corrupted repository.

(I note that there's a whole thread on this, but I was off doing other things, so I probably missed part of it)

We really should _never_ create a pack in-place with the final name.

The way to fix the race is to simply not create the patch as

	.git/objects/packed/pack-xyz.{pack|idx}

in the first place, but simply "mv" them into place later. If you do that after you've written the temporary pointer to it, there is no race (the temporary pointer may not be usable, of course, but that's a separate issue).

That said, I think some of the "git repack -d" logic is also unnecessarily fragile. In particular, it shouldn't just do

	existing=$(find . -type f \( -name '*.pack' -o -name '*.idx' \))

like it does to generate the "existing" list, it should probably only ever remove a pack-file and index file _pair_, ie it should do something like

	existing=$(find . -type f -name '*.pack')

and then do

	for pack in $existing
	do
		index="$(basename $pack).idx"
		if [ -f $index ] && [ "$pack"!= "$newpack" ]
		then
			rm -f "$pack" "$index"
		fi
	done

etc, exactly so that it would never remove anything that is getting indexed or is otherwise half-way done, regardless of any other issues.

Hmm?