Re: [PATCH] Make repack less likely to corrupt repository

onsdag 11 februari 2009 01:31:07 skrev Junio C Hamano:

Robin Rosenberg [off-list ref] writes:

quoted

onsdag 11 februari 2009 00:56:49 skrev Junio C Hamano:

quoted

We failed to honor what the end user wanted: to repack.  Why should we
exit 0 here?

A repack may or may not yield a better packed repo. In this case, not,
but for a different reason than failing to find better deltas. Given the
circumstances that is most likely to cause the "failure (repacking on
windows), this is "normal" behaviour and no reason to scare the user
with an error code.

Up to this point, I felt my earlier misconception corrected, but then ...

quoted

The unlink error might be enough.

... I think we should not even show unlink errors, if "this is not an
error, nothing to worry about" is the official stance about such failure;
otherwise the errors will scare people, *and* others then doubly complain
that even the command detects errors, the whole thing does *not* error
out.

Here is an amendment, the only change is that it outputs a more verbose
explanation. Remember the issue we are trying to fix is a critical bug, not
a cosmetic error. I'd rather hear people complain about the wording of error
messages, than lost repositories.

-- robin

From 3598881d6591e7c89b1a6a3c8da526f847382a35 Mon Sep 17 00:00:00 2001

From: Robin Rosenberg <redacted>
Date: Sun, 15 Feb 2009 17:05:49 +0100
Subject: [PATCH] Try to remove the old packs if we succeed. Exit success if rollback fails after
 failing to rename old packs.

---
 git-repack.sh |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/git-repack.sh b/git-repack.sh
index 0f13043..194af86 100755
--- a/git-repack.sh
+++ b/git-repack.sh

@@ -136,8 +136,15 @@ then
 		do
 			echo >&2 "WARNING:   old-$file -> $file"
 		done
+		exit 1
 	fi
-	exit 1
+	echo >&2 "INFO: We recovered from the repack error, but your repo"
+	echo >&2 "INFO: is probably suboptimally packed. You may try to repack"
+	echo >&2 "INFO: later. A common reason for repacking failure is that"
+	echo >&2 "INFO: a Windows program was locking one of the old pack files."
+	echo >&2 "INFO: To repack successfully you may have to close that program"
+	echo >&2 "INFO: before repacking."
+	exit
 fi
 
 # Now the ones with the same name are out of the way...

@@ -152,6 +159,15 @@ do
 	exit
 done
 
+# Remove the "old-" files
+for name in $names
+do
+	rm -f "$PACKDIR/old-pack-$name.idx"
+	rm -f "$PACKDIR/old-pack-$name.pack"
+done
+
+# End of pack replacement.
+
 if test "$remove_redundant" = t
 then
 	# We know $existing are all redundant.

-- 
1.6.1.285.g35d8b