On Mon, Jan 27, 2020 at 11:53 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > > > From: Han-Wen Nienhuys <hanwen@xxxxxxxxxx> > > Subject: Re: [PATCH v2 3/5] Document how ref iterators and symrefs interact > > "Subject: refs: document how ...", perhaps? > > Also isn't it more like iterators and symrefs do not interact in any > unexpected way, is it? iterators while enumerating refs when they > see a symref, they do not dereference and give the underlying ref > the symref is pointing at---the underlying ref will be listed when > it comes its turn to be shown as an ordinary ref. I am not sure > what is there to single out and document... I mean, you can't return a zero OID and set REF_ISSYMLINK. Instead, the ref backend has to follow the symlink recursively until it finds a ref pointing to a OID. > > Change-Id: Ie3ee63c52254c000ef712986246ca28f312b4301 > > Signed-off-by: Han-Wen Nienhuys <hanwen@xxxxxxxxxx> > > --- > > refs/refs-internal.h | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/refs/refs-internal.h b/refs/refs-internal.h > > index ff2436c0fb..fc18b12340 100644 > > --- a/refs/refs-internal.h > > +++ b/refs/refs-internal.h > > @@ -269,6 +269,9 @@ int refs_rename_ref_available(struct ref_store *refs, > > * to the next entry, ref_iterator_advance() aborts the iteration, > > * frees the ref_iterator, and returns ITER_ERROR. > > * > > + * Ref iterators cannot return symref targets, so symbolic refs must be > > + * dereferenced during the iteration. > > What this says is not techincally incorrect. Iterators do not give > each_ref_fn what underlying ref a symref is pointing at. But it > also is misleading. If your callback wants to know what object each > ref is pointing at do not need to do anything extra when it sees a > symref, as name of the object pointed at by the underlying ref is > given to it. Only callbacks that wants to know the other ref, not > the value of the other ref, needs to dereference when called with a > symref. > > But I wonder if we need to even say this. Isn't it obvious from the > each_ref_fn API that nothing other than the refname, object id, and > what kind of ref it is, will be given to the user of the API, so it > would be natural for a caller that wants to do extra things it needs > to do for symrefs must act when it learns a ref is a symref? After > all, that is why the flags word is one of the parameters given to an > each_ref_fn. Maybe it is obvious to you, but it was not obvious to me, coming from JGit working on the RefDatabase. One could imagine that the caller needs to do something special to handle a symref, and returning a zero object ID is OK, and this is a natural assumption if you implement a ref backend, as dereferencing the symref to find out the final OID may do more work than the caller needs. -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
