Re: [PATCH 4/4] gc: remove broken refs

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 25, 2015, 00:08 UTC
Message-ID: <xmqqr3lnuzqu.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20150924175758.GB15117@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 5 quoted lines

> For the same reasons as in my earlier responses, I think it's dangerous
> to remove broken refs (it makes a small corruption much worse). It seems
> reasonably sane to remove a dangling symref, though if we teach
> for_each_ref to gracefully skip them, then leaving them in place isn't a
> problem.

One thing I wondered was if we can reliably tell between a ref that wanted to be a real ref that records a broken object name and a ref that wanted to be a symbolic ref that points a bogus thing, and if we can't, should we worry about it too much. The former is more serious, as the history behind the commit it wanted to but failed to record is at risk of being pruned.

One case that is clearly safe is "ref: refs/heads/gone"; it is not likely to be the result of attempting to write a real object name gone bad by whatever filesystem corruption. On the other hand, an obviously problematic case is an empty file. We cannot tell if the "broken" ref used to anchor the tip of a real history (which is about to be lost with Dscho's patch 1/4) or was merely pointing at another ref (which will not harm the object database if ignored).

So the rule should be

    If resolve_ref_unsafe_1() says it is a symbolic ref, if
    check_ref_format() is OK with the ref it points at, and if that
    pointee is missing, then it is safe to skip.

All other funnies should trigger the safety.

The collection of "broken and can be removed" refs introduced by 3/4 may also have to take that into account, I think.