[PATCH] Make repack less likely to corrupt repository

From: Robin Rosenberg <robin.rosenberg@dewire.com>
Date: Feb 9, 2009, 00:44 UTC
Message-ID: <1234140299-29785-1-git-send-email-robin.rosenberg@dewire.com>

Repack could easily leave a repo in bad state after a failed repack on Windows if a pack file that repack wanted to replace was locked by a reader. A second attempt at running repack would then destroy the repo by removing the pack file. This attempts to make repack leave the repo in a good state, although not optimal, in order to avoid disasters.

If renaming an old pack fails we will try to restore halfway renames before exiting repack.

For severe situations we encourage the user to seek advice.

Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
 git-repack.sh |   43 ++++++++++++++++++++++++++++++++-----------
 1 files changed, 32 insertions(+), 11 deletions(-)

Here is an attempt at fixing this It was tested by making sure the idx or pack file was opened by some process and verifying that repo was ok after git repack -a -d. That was a manual test. We should probably add an automatic one too of some sort but I submit this for reading and ad-hoc testing by interested parties.

-- robin

diff --git a/git-repack.sh b/git-repack.sh
index 458a497..6a7ba90 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -93,22 +93,43 @@ for name in $names ; do
 	chmod a-w "$PACKTMP-$name.pack"
 	chmod a-w "$PACKTMP-$name.idx"
 	mkdir -p "$PACKDIR" || 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 &&
+	ok=t
+	if test -f "$PACKDIR/pack-$name.pack"
+	then
+		mv -f "$PACKDIR/pack-$name.pack" \
+			"$PACKDIR/old-pack-$name.pack"
+	fi &&
+	if test -f "$PACKDIR/pack-$name.idx"
+	then
+		mv -f "$PACKDIR/pack-$name.idx" \
+			"$PACKDIR/old-pack-$name.idx" ||
+		(
+			mv -f "$PACKDIR/old-pack-$name.pack" \
+			"$PACKDIR/pack-$name.pack" || (
+				echo >&2 "Failed to restore after a failure to rename"\
+					"pack-$name{pack,idx} to old-$pack{pack,idx} in $PACKDIR"
+				echo >&2 "Please acquire advice on how to recover from this"\
+					"situation before you proceed."
+				exit 1
+			) || false
+		) || (
+			echo >&2 "Failed to replace the existing pack with updated one."
+			echo >&2 "We recovered from the situation, but cannot continue".
+			echo >&2 "repacking."
+			exit 0
+		)
+	fi &&
 	mv -f "$PACKTMP-$name.pack" "$PACKDIR/pack-$name.pack" &&
 	mv -f "$PACKTMP-$name.idx"  "$PACKDIR/pack-$name.idx" &&
 	test -f "$PACKDIR/pack-$name.pack" &&
 	test -f "$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."
+		if (test -f "$PACKDIR/old-pack-$name.pack" ||
+			test -f "$PACKDIR/old-pack-$name.idx")
+		then
+			echo >&2 "The original set of packs have been saved as"
+			echo >&2 "old-pack-$name.{pack,idx} in $PACKDIR."
+		fi
 		exit 1
 	}
 	rm -f "$PACKDIR/old-pack-$name.pack" "$PACKDIR/old-pack-$name.idx"

-- 
1.6.1.285.g35d8b