Subject: Re: Be more careful about updating refs

From: Charles Bailey <[email-masked]>
Date: Thu, 17 Jan 2008 09:15:58 +0000

On Wed, Jan 16, 2008 at 11:52:43AM -0800, Junio C Hamano wrote:
> 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.

I'm assuming that this original patch and the test update turned into
the following commit in master:

c3b0dec509fe136c5417422f31898b5a4e2d5e02 is first bad commit

I just thought I should warn you that this seems (git bisect tells me
so) to have caused a failure in t9301-fast-export.sh on my Mac OS X
10.4.11 machine although I haven't yet had the time to investigate
why.

Charles.