Re: [PATCH v2 2/9] refs: teach arbitrary repo support to iterators

The two steps so far seems to give the necessary information to code
paths that want them, so it is not wrong per-se, but this makes me
wonder a few things.

 - There may be multiple ref backends and iterators corresponding to
   them.  Is it reasonable to assume that there are backends that do
   not need "repo"?  Otherwise, shouldn't this be added to the base
   class "struct ref_iterator base"?

All backends need repos, but not all iterators need backends - there is
a merge_ref_iterator and a prefix_ref_iterator, for example.

 - The iterator_begin() and other functions have been taught to take
   the repository in addition to the ref_store in the previous step,
   but

   . Doesn't iterator iterate over a single ref_store?  Shouldn't it
     have a pointer to the ref_store it is iterating over?

No - as above, merge_ref_iterator, for example, does not iterate over a
ref_store but combines the results of 2 other iterators.

   . Doesn't a ref_store belong to a single repository?  Shouldn't
     it have a pointer to the repository it is part of?

   If the answers to both are 'yes', then we wouldn't need to add a
   repository pointer as a new parameter to functions that already
   took a ref store.

In other words, I am wondering if the right pieces of information
are stored in the right structure.

Thanks.

A ref_store does belong to a single repository. The reason why it
doesn't have a pointer to that repository is probably because struct
ref_store (00eebe351c ("refs: create a base class "ref_store" for
files_ref_store", 2016-09-09)) predates struct repository (359efeffc1
("repository: introduce the repository object", 2017-06-23)). I've been
avoiding introducing a pointer to the repository in struct ref_store to
avoid unnecessary coupling, but it is looking more and more necessary,
as you mention in your reply [1] to another patch about how this would
eliminate certain other "user" codepaths needing to know about the repo.
I'll take a look at introducing a pointer to the repo in struct
ref_store and report back with my findings.

[1] https://lore.kernel.org/git/xmqqh7e4iacw.fsf@gitster.g/ (local)