Re: gc considered dangerous

[Robin Rosenberg <robin.rosenberg.lists@dewire.com>]

söndag 08 februari 2009 21:00:57 skrev Junio C Hamano:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > My preliminary guess is that this code in pack-write.c needs to use the 
> > lock file paradigm:
> >
> >         if (!index_name) {
> > 		[...]
> >         } else {
> >                 unlink(index_name);
> >                 fd = open(index_name, O_CREAT|O_EXCL|O_WRONLY, 0600);
> >         }
> 
> Whoa.  That particular code has been (and is still) correct.
> 
> When repacking we should pack into a temporary pack and idx file and then
> replace the real ones after both new pack and its idx are successfully
> written, and I thought that is how we've been doing this all the time.
> Maybe the caller has been broken at some point?  Sigh...
> 
I intend to test something like this (as of yet completely untested)

-- robin

commit 25a77c80efeb221d165db0bef66b4498aacfac96
Author: Robin Rosenberg <robin.rosenberg@dewire.com>
Date:   Sun Feb 8 20:59:30 2009 +0100

    Make repack more fail-safe
    
    If renaming an old pack fails try to restore halfway renames
    befor failing. The basis is the assumption that this occurs
    because a files is locked for reading on Windows.

diff --git a/git-repack.sh b/git-repack.sh
index 458a497..e816997 100755
--- a/git-repack.sh
+++ b/git-repack.sh
@@ -94,14 +94,25 @@ for name in $names ; do
 	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 &&
+	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 to old-$pack"
+			echo >&2 "Please aquire advice on how to recover from this"
+			echo >&2 "situation before you proceed."
+			false
+			)
+		)
+	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" &&
@@ -109,6 +120,8 @@ for name in $names ; do
 		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."
+		echo >&2 "Please aquire advice on how to recover from this situation"
+		echo >&2 "before you proceed."
 		exit 1
 	}
 	rm -f "$PACKDIR/old-pack-$name.pack" "$PACKDIR/old-pack-$name.idx"