On Thu, 26 Oct 2006, Eran Tromer wrote:
>
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html