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

Jeff King [off-list ref] 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?

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

Jeff King [off-list ref] writes:

quoted

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