[PATCH] receive-pack: allow deletion of corrupt refs

[PATCH] receive-pack: allow deletion of corrupt refs

[Johannes Schindelin]

Occasionally, in some setups (*cough* forks on repo.or.cz *cough*) some
refs go stale, e.g. when the forkee rebased and lost some objects needed
by the fork.  The quick & dirty way to deal with those refs is to delete
them and push them again.

However, git-push first would first fetch the current commit name for the
ref, would receive a null sha1 since the ref does not point to a valid
object, then tell receive-pack that it should delete the ref with this
commit name.  delete_ref() would be subsequently be called, and check that
resolve_ref() (which does _not_ check for validity of the object) returns
the same commit name.  Which would fail.

The proper fix is to avoid corrupting repositories, but in the meantime
this is a good fix in any case.

Incidentally, some instances of "cd .." in the test cases were fixed, so
that subsequent test cases run in t/trash/ irrespective of the outcome of
the previous test cases.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	I was bitten one time too many by a rebased git/mingw.git/.

	Will work on the "proper fix" I hinted at.

 receive-pack.c        |    4 ++++
 t/t5516-fetch-push.sh |   41 ++++++++++++++++++++++++++---------------
 2 files changed, 30 insertions(+), 15 deletions(-)

diff --git a/receive-pack.c b/receive-pack.c
index ed44b89..fba4cf8 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -200,6 +200,10 @@ static const char *update(struct command *cmd)
 	}
 
 	if (is_null_sha1(new_sha1)) {
+		if (!parse_object(old_sha1)) {
+			warning ("Allowing deletion of corrupt ref.");
+			old_sha1 = NULL;
+		}
 		if (delete_ref(name, old_sha1)) {
 			error("failed to delete %s", name);
 			return "failed to delete";
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index fd5f284..9d2dc33 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -274,9 +274,8 @@ test_expect_success 'push with HEAD nonexisting at remote' '
 test_expect_success 'push with dry-run' '
 
 	mk_test heads/master &&
-	cd testrepo &&
-	old_commit=$(git show-ref -s --verify refs/heads/master) &&
-	cd .. &&
+	(cd testrepo &&
+	 old_commit=$(git show-ref -s --verify refs/heads/master)) &&
 	git push --dry-run testrepo &&
 	check_push_result $old_commit heads/master
 '
@@ -284,28 +283,40 @@ test_expect_success 'push with dry-run' '
 test_expect_success 'push updates local refs' '
 
 	rm -rf parent child &&
-	mkdir parent && cd parent && git init &&
-		echo one >foo && git add foo && git commit -m one &&
-	cd .. &&
-	git clone parent child && cd child &&
+	mkdir parent &&
+	(cd parent && git init &&
+		echo one >foo && git add foo && git commit -m one) &&
+	git clone parent child &&
+	(cd child &&
 		echo two >foo && git commit -a -m two &&
 		git push &&
-	test $(git rev-parse master) = $(git rev-parse remotes/origin/master)
+	test $(git rev-parse master) = $(git rev-parse remotes/origin/master))
 
 '
 
 test_expect_success 'push does not update local refs on failure' '
 
 	rm -rf parent child &&
-	mkdir parent && cd parent && git init &&
+	mkdir parent &&
+	(cd parent && git init &&
 		echo one >foo && git add foo && git commit -m one &&
 		echo exit 1 >.git/hooks/pre-receive &&
-		chmod +x .git/hooks/pre-receive &&
-	cd .. &&
-	git clone parent child && cd child &&
-		echo two >foo && git commit -a -m two || exit 1
-		git push && exit 1
-	test $(git rev-parse master) != $(git rev-parse remotes/origin/master)
+		chmod +x .git/hooks/pre-receive) &&
+	git clone parent child &&
+	(cd child &&
+		echo two >foo && git commit -a -m two &&
+		! git push &&
+		test $(git rev-parse master) != \
+			$(git rev-parse remotes/origin/master))
+
+'
+
+test_expect_success 'allow deleting an invalid remote ref' '
+
+	pwd &&
+	rm -f testrepo/.git/objects/??/* &&
+	git push testrepo :refs/heads/master &&
+	(cd testrepo && ! git rev-parse --verify refs/heads/master)
 
 '
 
-- 
1.5.3.6.2065.gd47ac

Re: [PATCH] receive-pack: allow deletion of corrupt refs

[Johannes Schindelin]

Hi,

On Thu, 29 Nov 2007, Johannes Schindelin wrote:

> Incidentally, some instances of "cd .." in the test cases were fixed, so 
> that subsequent test cases run in t/trash/ irrespective of the outcome 
> of the previous test cases.

Just to clarify: I was quite surprised that my test case did not succeed, 
and eventually found out that $(pwd) was t/trash/child/child after the 
last test.

Ciao,
Dscho

Re: [PATCH] receive-pack: allow deletion of corrupt refs

[Petr Baudis]

  Hi,

On Thu, Nov 29, 2007 at 01:02:53AM +0000, Johannes Schindelin wrote:
> Occasionally, in some setups (*cough* forks on repo.or.cz *cough*) some
> refs go stale, e.g. when the forkee rebased and lost some objects needed
> by the fork.  The quick & dirty way to deal with those refs is to delete
> them and push them again.

  repo.or.cz now has this patch. I'm also repacking the kernel
repository which is in a pretty awful state, I'm hoping that will fix
some of the heavy performance problems repo.or.cz is hitting currently.

-- 
				Petr "Pasky" Baudis
We don't know who it was that discovered water, but we're pretty sure
that it wasn't a fish.		-- Marshall McLuhan