Re: [PATCH v5 1/3] ls-refs: report unborn targets of symrefs

From: Jonathan Tan <jonathantanmy@google.com>
Date: Feb 2, 2021, 02:20 UTC
Message-ID: <20210202022024.1791749-1-jonathantanmy@google.com>
In-Reply-To: <xmqqtuqzs8m5.fsf@gitster.c.googlers.com>

> So a dangling symref, e.g. "refs/remotes/origin/HEAD -> trunk" when
> no "refs/remotes/origin/trunk" exists, is not reported to send_ref()
> in the same way as an unborn "HEAD"?

I've tried it, and yes, for_each_namespaced_ref() will not report it. Looking at the code, I think it is packed_ref_iterator_advance() which checks for broken objects and skips over them.

Show 10 quoted lines

> I would have expected that we'd
> report where it points at, and for that to work, you'd have to use
> not just the vanilla send_ref() as the callback, but something that
> knows how to do "are we expected to send unborn symrefs" logic, like
> send_possibly_unborn_head does.
> 
> That "changed to tolerate ... should work" worries me.
> 
> If "for_each_namespaced_ref(send_ref, &data)" will never call send_ref()
> with NULL (as in (void *)0) oid, then that would be OK,

If it called send_ref() with (void *)0 OID, it would segfault with the current code, which calls oid_to_hex() on the OID unconditionally.

Show 5 quoted lines

> but if it
> ends up calling with NULL somehow, it is responsible to ensure that
> data->symrefs is true and flag has REF_ISSYMREF set, or send_ref()
> would misbehave, (see the first part of your message, which I am
> responding to), no?

If it did, then yes.