Re: [1/5] Consider a blob to be parsed

From: Linus Torvalds <torvalds@osdl.org>
Date: Apr 28, 2005, 05:28 UTC
Message-ID: <Pine.LNX.4.58.0504272217240.18901@ppc970.osdl.org>
In-Reply-To: <Pine.LNX.4.21.0504280100360.30848-100000@iabervon.org>

On Thu, 28 Apr 2005, Daniel Barkalow wrote:

>
> We don't parse blobs at all, so any that we've got are as parsed as
> they're going to get. Don't make fsck-cache mark them.

NO NO NO!

This is WRONG, dammit. I fixed it once, you are re-introducing the same bug.

Daniel, the problem is that you parse them only when you SEE them, and that is totally different from having seen a REFERENCE to them. One says "I've seen this object", the other says "I _want_ to see this object". They are two totally different things.

You now mark all "blob" objects parsed regardless of whether you have actually seen the blob or not. Ie you mark a blob parsed just from having seen a _reference_ to it, and fsck can never know whether it actually really saw the object or not.

This is the commit that already fixed this bug once, and that you are now re-introducing:

	commit 4728b861ace127dc39c648f3bea64c3b86bbabc5
	tree 242227fc3c3a74d070ed36496e790335dd00c44a
	parent da6abf5d9c342a74dffbcc2015b9c27d7819a900
	author Linus Torvalds <torvalds@ppc970.osdl.org> Sun, 24 Apr 2005 14:10:55 -0700
	committer Linus Torvalds <torvalds@ppc970.osdl.org> Sun, 24 Apr 2005 14:10:55 -0700
	
	    fsck-cache: notice missing "blob" objects.
	    
	    We should _not_ mark a blob object "parsed" just because we
	    looked it up: it gets marked that way only once we've actually
	    seen it. Otherwise we can never notice a missing blob.

please think about it.

Try to make some test-cases for fsck. They are quite easy to make: copy a 
good directory, and 
 - remove one commit (in the middle)
 - remove at least on tree
 - remove at least one blob
 - corrupt a file obviously (make it not uncompress ok)
 - corrupt a file in a subtle way (make it uncompress ok and have the
   right signature, but be the wrong type - for example, make a "commit" 
   object that points to a "tree" object that actually is a "blob")

And you'll see how this "consider a blob parsed" totally destroys fsck's ability to notice that the blob doesn't even _exist_ any more (case 3 above).

"parsing" and "looking up" are two totally independent operations. They are independent for commits and trees, and they are independent for blobs.

To mark a blob parsed, you _need_ to have actually looked it up and verified that it exists and that the object header is valid (and if you're fsck, that the sha1 matches). You MUST NOT do it in "lookup_blob()".

		Linus