Re: [PATCH] fsck: check skiplist for object in fsck_blob()

From: Jeff King <peff@peff.net>
Date: Jun 28, 2018, 17:30 UTC
Message-ID: <20180628173014.GB31766@sigill.intra.peff.net>
In-Reply-To: <xmqqr2kqyies.fsf@gitster-ct.c.googlers.com>

On Thu, Jun 28, 2018 at 09:39:39AM -0700, Junio C Hamano wrote:

Show 13 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > Yeah, this solution seems sensible. Given that we would never report any
> > error for that blob, there is no point in even looking at it. I wonder
> > if we ought to do the same for other types, too. Is there any point in
> > opening a tree that is in the skiplist?
> 
> Suppose the tree is listed there only because it has one entry for a
> subtree with leading 0 in its mode.  We do want to ignore that
> format violation, but we still want to learn the fact that the
> subtree it points at and its contents are connected and not
> dangling, so we do need to open it.  Is that open done in a separate
> phase?

To be honest, I'm not sure. There _is_ a separate phase for checking reachability, but I think there may be some dependencies between the phases. Once upon a time they were communicated by the existence of entries in obj_hash (blech!) but I think these days it happens using a a bit in object->flags.

There is at least one case of interest just in this phase, though: we have to read the surrounding tree to find out that a particular blob is a .gitmodules file. So if you skiplist'd a tree, that would also mean we fail to mark its .gitmodules (if any) as such. I'm not sure if that would be a bug or a feature, though.

-Peff