On Thu, 26 Oct 2006, Eran Tromer wrote:
quoted text
>
> 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?

		Linus
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html