Re: [PATCH] commit: check result of resolve_ref_unsafe

From: Jeff King <peff@peff.net>
Date: Oct 19, 2017, 02:49 UTC
Message-ID: <20171019024919.ram3bzabt5fra2lc@sigill.intra.peff.net>
In-Reply-To: <xmqqo9p4j7eu.fsf@gitster.mtv.corp.google.com>

On Thu, Oct 19, 2017 at 09:41:29AM +0900, Junio C Hamano wrote:

Show 10 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > Tangential to your patch, I also wondered why we did not pass
> > RESOLVE_REF_READING to resolve_ref_unsafe(). I think the answer is that
> > for symref lookups, we normally don't pass it so that we can handle
> > dangling symrefs. Of course we _just_ wrote HEAD ourselves, so we'd
> > expect it to exist, so it shouldn't really matter either way.
> 
> If we do expect it to exist, shouldn't we checking with READING and
> catching a funny situation if it arises?

Maybe. In the normal case it would not matter. If we _do_ find it gone, I guess that is a sign that somebody is racing with us and changed HEAD after we made the commit.

TBH, I think the "most right" thing would be to actually capture the ref that HEAD points to when we actually make the commit, remember it, and then report it here (the whole function of this code is just to say "I made a commit on branch X"). But I don't know how much trouble it is worth going to for that. It's buried behind a ref_transaction, and I don't think builtin/commit.c ever sees the real name (though maybe it would be a good feature of the transaction to record the destinations of any symrefs we updated).

-Peff