git-repack -a -d produces unusable packs with 1.4.0

git-repack -a -d produces unusable packs with 1.4.0

[Thomas Glanzmann]

Hello everyone,
what on earth is going wrong here (this is with 1.4.0):

        (node01) [~] git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
        Checking files out...)
        100% (19830/19830) done
        (node01) [~] cd linux-2.6/
        (node01) [~/linux-2.6] git repack -a -d
        Generating pack...
        Done counting 263175 objects.
        Deltifying 263175 objects.
        100% (263175/263175) done
        Writing 263175 objects.
        100% (263175/263175) done
        Total 263175, written 263175 (delta 206825), reused 262943 (delta 206593)
        Pack pack-ce49d2efd5af06ed6093049050b5ba41da8b683f created.
        mv: overwrite `.git/objects/pack/pack-ce49d2efd5af06ed6093049050b5ba41da8b683f.pack', overriding mode 0444? y
        mv: overwrite `.git/objects/pack/pack-ce49d2efd5af06ed6093049050b5ba41da8b683f.idx', overriding mode 0444? y
        fatal: packfile .git/objects/pack/pack-ce49d2efd5af06ed6093049050b5ba41da8b683f.pack does not match index.
        (node01) [~/linux-2.6] git repack -a -d
        Generating pack...
        fatal: expected sha1, got garbage:
        fatal: packfile .git/objects/pack/pack-ce49d2efd5af06ed6093049050b5ba41da8b683f.pack does not match index.

Oh and are there localized git mirrors like for example
git.de.kernel.org or is the git protocoll only available from the
primary site?

        Thomas

Re: git-repack -a -d produces unusable packs with 1.4.0

[Junio C Hamano]

Thomas Glanzmann <sithglan@stud.uni-erlangen.de> writes:

>         Pack pack-ce49d2efd5af06ed6093049050b5ba41da8b683f created.
>         mv: overwrite `.git/objects/pack/pack-ce49d2efd5af06ed6093049050b5ba41da8b683f.pack', overriding mode 0444? y
>         mv: overwrite `.git/objects/pack/pack-ce49d2efd5af06ed6093049050b5ba41da8b683f.idx', overriding mode 0444? y
>         fatal: packfile .git/objects/pack/pack-ce49d2efd5af06ed6093049050b5ba41da8b683f.pack does not match index.

I would understand if you answer 'y' to one but 'n' to the other
it would result in a situation with unmatching .pack and .idx
and you would see something like the above (by the way, the
"fatal" is coming from update-server-info that tries to read
from freshly moved packfiles); but the above is different, so it
does not explain the symptom.  I am worried and curious as to
what happened, since you are answering 'y' to both of them.
This would trigger immediately after a clone which creates pack
and idx unwritable; repack leaves the results writable unless
your umask is 0222, so "mv" would not even ask the silly
questions.

Did you happen to have .tmp-pack-ce49d2ef... at the project root
level after this failure?  If so which one (either .pack or .idx)?

If you had .tmp-pack-*.pack then .git/objects/pack/pack-ce49...pack
is from the old round and .git/objects/pack/pack-ce49...idx is
from the new one.  Moving .tmp-pack-* to .git/objects/pack/pack-*
would hopefully solve this problem.

Nevertheless, this _is_ a dangerous and grave bug, and thanks
for reporting it.

Maybe we would want to do something like this:

-- >8 --
git-repack: Be careful when updating the same pack as existing one.

After clone, packfiles are read-only by default and "mv" went
interactive asking if the user wants to replace it with a
repacked copy.  If one is successfully moved and the other is
not, the pack and its idx would become out-of-sync and corrupts
the repository.

Recovering is straightforward -- it is just the matter of
finding the remaining .tmp-pack-* and make sure they are both
moved -- but we should be extra careful not to do something so
alarming to the users.

---
diff --git a/git-repack.sh b/git-repack.sh
index eb75c8c..b58cf91 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -54,9 +54,21 @@ else
 	fi
 	mkdir -p "$PACKDIR" || exit
 
-	mv .tmp-pack-$name.pack "$PACKDIR/pack-$name.pack" &&
-	mv .tmp-pack-$name.idx  "$PACKDIR/pack-$name.idx" ||
-	exit
+	for sfx in pack idx
+	do
+		if test -f "$PACKDIR/pack-$name.$sfx"
+		then
+			mv -f "$PACKDIR/pack-$name.$sfx" "$PACKDIR/old-pack-$name.$sfx"
+		fi
+	done
+	mv -f .tmp-pack-$name.pack "$PACKDIR/pack-$name.pack" &&
+	mv -f .tmp-pack-$name.idx  "$PACKDIR/pack-$name.idx" || {
+		echo >&2 "Couldn't replace the existing pack with updated one."
+		echo >&2 "The original set of packs have been saved as"
+		echo >&2 "old-pack-$name.{pack,idx} in $PACKDIR."
+		exit 1
+	}
+	rm -f "$PACKDIR/old-pack-$name.pack" "$PACKDIR/old-pack-$name.idx" 
 fi
 
 if test "$remove_redundant" = t