> 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? Thanks for taking a look. The answer requires additional context, so I'll answer this at the end of this email. > 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. I thought about that, but didn't want to make things worse - the effort in this patch set is, after all, to attempt to increase the dissociation between the ref stores and a certain object store (that is, the_repository's object store), and I thought that reintroducing an association (albeit to arbitrary object stores instead of a hardcoded object store) would be a step back. But this may be the way to go - the ref stores already have a gitdir field that we could replace with a struct repository field. > > - 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. I was thinking of files_ref_iterator_begin() setting a local variable required_flags. Somehow I thought that files_pack_refs() relied on files_ref_iterator_begin() setting that variable, but now I see that that's not true - both functions are independently checking that the underlying ref store supports ODB access, so I can remove ODB from files_ref_iterator_begin() if I want to. To go back to the question at the top, now I agree that hiding the ODB access in _advance() in the layer in refs.c is possible. The last part still accessing the ODB is files_pack_refs(), I think. Refactoring that is possible, but I'll leave that to another patch set. If you or anyone else has more questions or comments, please reply - and in the meantime, I'll update this patch set to move the ODB access in _advance() to the layer in refs.c.
