From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mon, 22 Jun 2015 13:01:50 GMT
Subject: Re: [PATCH v6 17/19] fsck: Introduce `git fsck --quick`
Message-ID: <80dac4a49123c6e4ddc905c74d63d44e@www.dscho.org>
In-Reply-To: <xmqq1th4kdeq.fsf@gitster.dls.corp.google.com>

Hi Junio,

On 2015-06-21 22:35, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
>> On 2015-06-21 19:15, Junio C Hamano wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>>> That's brilliant.
>>>
>>> Just to make sure I am reading you correctly, you mean the current
>>> overall structure:
>>>
>>> [...]
>>
>> The way I read Michael's mail, he actually meant something different:
>> if all of the blob-related errors/warnings are switched to "ignore",
>> simply skip unpacking the blobs.
> 
> That is how I read his mail, too.
> 
> But because IIRC we do not check anything special with blob other
> than we can read it correctly, my description of "overall structure"
> stayed at a very high conceptual level.  The unpacking may happen at
> a much higher level in the code, i.e. it comes way before this part
> of the logic flow:
> 
>         if ("is bad_blob ignored?")
> 		;
> 	else if (! "is the blob loadable and well-formed?") {
> 
> in which case "is bad blobs ignored?" check may have to happen
> before we unpack the object.
> 
> And I do not suggest introducing yet another BAD_BLOB error class; I
> would have guessed that you already have an error class for objects
> that are not stored correctly (be it truncated loose object, checksum
> mismatch in the packed base object, or corrupt delta in pack).

Sadly, there is no BAD_BLOB class. The reason is that we actually perform no test on blobs, as you pointed out, except for the implicit one: read it as a blob object.

And reading them even only partially would still imply a lot of I/O, taking away much of the performance improvement I wanted to achieve here.

Further, please note that the `--quick` option *solely* impacts `git fsck`, not `git receive-pack`, because we actually really skipped everything except the connectivity test.

To allow this discussion to be resolved without further ado, I therefore renamed the `--quick` option to `--connectivity-only`, as even I realize that there is not much of a check left if not even author or committer lines are tested.

Ciao,
Dscho