On Wed, Nov 06, 2019 at 03:59:07PM -0500, Jeff King wrote: > On Wed, Nov 06, 2019 at 08:48:05PM +0100, Johannes Schindelin wrote: > > > Now, I think the two factors that trigger this bug over here are: > > > > - I had all the objects locally already, as I had pushed from a topic > > branch to `master` [*1*]. > > > > - My worktree's `.git/objects` is connected to an alternate that is > > connected to the current Git repository (yes, it is circular, long > > story...) and has refs pointing to commits its alternate that have > > been gc'ed away. > > I think this second one is the crux of the issue. Your alternate is a > corrupt repository, and I don't think that's something we ought to be > worried about supporting in general. Thinking on this a bit more, the whole thing is a bit subtle. Imagine what would happen in the fetch or push code paths from before my patches. We'd tell the other side "hey, I have object X" when in fact we don't. So we'd end up missing some objects from the transfer. Before my patches, we'd have done a full connectivity check, with no regard to the alternate, and complained. After my patches, we make the assumption that the alternate isn't corrupt, and trust its refs. So there's an opportunity for corruption in the alternate to spread to the child repository. We're actually saved somewhat by the current behavior where rev-list bails on the broken refs, rather than accepting them at face value. But it wouldn't protect us from deeper corruptions in the alternate. I have trouble getting too worked up about that, though. If your alternate is corrupt, this is only one of many ways that the corruption can spread to your repository. However, it would make sense to me that if we can cheaply notice a corruption in the alternate, that we should avoid it spreading. And noticing that the object pointed to by a ref is missing is reasonably cheap (in fact, it's done by most ref iteration; for-each-ref explicitly uses FILTER_REFS_INCLUDE_BROKEN). So I think the right direction is probably to teach for-each-ref an "--omit-broken" option, and use that for enumerating the alternate refs. That would let us not only notice this corruption, but we'd "route around" it by avoiding advertising the broken alternate tip in the first place. And it would fix your problem, too. In your case it sounds like you're not working with any of the corrupted objects at all; rather it's just an unrelated corruption that's causing rev-list to bail. -Peff
