On Thu, Aug 26, 2021 at 1:23 AM Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote: > > Currently, ref iterators access the object store each time they advance > if and only if the boolean flag DO_FOR_EACH_INCLUDE_BROKEN is unset. > (The iterators access the object store because, if > DO_FOR_EACH_INCLUDE_BROKEN is unset, they need to attempt to resolve > each ref to determine that it is not broken.) > > Also, the object store accessed is always that of the_repository, making > it impossible to iterate over a submodule's refs without > DO_FOR_EACH_INCLUDE_BROKEN (unless add_submodule_odb() is used). > > As a first step in resolving both these problems, replace the > DO_FOR_EACH_INCLUDE_BROKEN flag with a struct repository pointer. This > commit is a mechanical conversion - whenever DO_FOR_EACH_INCLUDE_BROKEN > is set, a NULL repository (representing access to no object store) is > used instead, and whenever DO_FOR_EACH_INCLUDE_BROKEN is unset, a > non-NULL repository (representing access to that repository's object > store) is used instead. Right now, the locations in which > non-the_repository support needs to be added are marked with BUG() > statements - in a future patch, these will be replaced. (NEEDSWORK: in > this RFC patch set, this has not been done) from a design perspective, it would be nice if the ref backend wouldn't need to know about the object store. Can't this be hidden in the layer in refs.c that calls into the backends? If they have to know about the object store, have you considered passing the repository pointer in xxx_ref_store_create() ? Then there is no possibliity to mismatch the repository pointers and with the ref store. > - Making all ref stores not access the object store during their > _advance() callbacks, and making ref_iterator_advance() be responsible > for checking the object store - thus, simplifying the code in that the > logic of checking for the flag (current) or the pointer (after the > equivalent of this commit) is only in one place instead of in every > ref store's callback. However, the ref stores already make use of this > flag for another reason - for determining if refs are resolvable when > writing (search for "REF_STORE_ODB"). Thus, I decided to retain each I looked, but I couldn't figure out how this flag is used. -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Han-Wen Nienhuys Google Munich hanwen@xxxxxxxxxx
