On Thu, Jun 28, 2018 at 09:39:39AM -0700, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> 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