Subject: Re: Be more careful about updating refs

From: Junio C Hamano <[email-masked]>
Date: Wed, 16 Jan 2008 11:52:43 -0800

Linus Torvalds <[email-masked]> writes:

> On Tue, 15 Jan 2008, Linus Torvalds wrote:
>> 
>> This makes write_ref_sha1() more careful: it actually checks the SHA1 of 
>> the ref it is updating, and refuses to update a ref with an object that it 
>> cannot find.
>
> Side note: this breaks some tests, because those tests do things like
>
> 	git update-ref refs/heads/master 1111111111111111111111111111111111111111 &&
> 		test 1111111111111111111111111111111111111111 = $(cat .git/refs/heads/master)
>
> ...
> (Pet peeve on mine: people fixing assert()'s by changing the source-code, 
> without ever asking themselves whether maybe the assert itself was the 
> bug).

The rules for the plumbing used to be that refs can point at
anything that get_sha1() accepts.  We did not even require it to
be parse_object() happy let alone it being parse_commit() kosher.

You changed the world order.  I agree that the world order was
changed in a good way, but saying that the original test did not
check the right thing or it was a bug is not quite fair.  At
worst, we can say that it was very sloppily written by assuming
that the commands involved in the particular test would not care
about corrupted repositories whose refs point at nonexistant
bogus objects.

I'll squash the following to your patch.

---
 t/t1400-update-ref.sh |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index a90824b..71ab2dd 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -7,12 +7,19 @@ test_description='Test git update-ref and basic ref logging'
 . ./test-lib.sh
 
 Z=0000000000000000000000000000000000000000
-A=1111111111111111111111111111111111111111
-B=2222222222222222222222222222222222222222
-C=3333333333333333333333333333333333333333
-D=4444444444444444444444444444444444444444
-E=5555555555555555555555555555555555555555
-F=6666666666666666666666666666666666666666
+
+test_expect_success setup '
+
+	for name in A B C D E F
+	do
+		test_tick &&
+		T=$(git write-tree) &&
+		sha1=$(echo $name | git commit-tree $T) &&
+		eval $name=$sha1
+	done
+
+'
+
 m=refs/heads/master
 n_dir=refs/heads/gu
 n=$n_dir/fixes