Subject: Re: [PATCH 03/16] refs: add methods for the ref iterators

From: Michael Haggerty <[email-masked]>
Date: Tue, 5 Jan 2016 14:43:40 +0100

On 01/04/2016 08:01 PM, Junio C Hamano wrote:
> David Aguilar <[email-masked]> writes:
> [...]
>> My only comment is that it seems like having a single static global
>> the_refs_backend seems like it should be avoided.
>> [...]
> 
> I think the ship is sailing in a different direction.  The API to
> deal with refs, possibly in multiple repositories, is that you use a
> single ref backend, and that backend is expected to handle refs in
> submodule repositories.  I.e. refs.c::for_each_ref_in_submodule()
> calls into the backend implementation of the same method.  So from
> that point of view, you cannot say "the top level repository uses
> LMDB backend but this and that submodule refs are looked up by
> consulting files backend".
> 
> If we want to go the opposite direction, so that we can keep more
> than one in-core representations of "repository" (what you called
> "application context") and optionally have different refs backend
> handling different repositories, we most likely would want to
> rethink the part of the refs API that special cases the submodule
> refs from within the current repository.  Their implementation
> should be excised from the API set, and instead made to be a set
> of thin wrapper functions that explicitly refer to a repository
> instance that represents the submodule.

I definitely think that the "one and only one refs backend per process"
model is not a good long-term approach. For example, submodules are
relatively independent but are nevertheless sometimes accessed from a
single process. I don't think insisting that they all use the same
references backend is tenable over the long term.

But I haven't insisted on this flexibility yet because I don't think it
will be much extra work in total to retrofit it later. What we mostly
want to avoid is the need to rewrite all call sites more than once. The
current patch series hasn't had to rewrite callers at all, so we still
have a rewrite in reserve :-)

Actually, maybe we will never have to rewrite all callers. I rather
think that we will retain one simplified API for dealing with "*this*
repository's references" and a second for dealing with other repos' refs.

By the way, I think the next step towards the ability to work with
multiple backends at the same time would be to store a pointer to the
refs backend inside the ref_cache instance (possibly renaming the
latter) and allowing them both to be looked up by submodule name as is
already done by get_ref_cache(). I think this change could also be done
without rewriting callers.

> [...]
> The caching of already read refs is a responsiblity of each ref
> backend in the current codebase.  I am not sure if that is a good
> placement, or a single implementation of the caching layer should
> instead sit on top of any and ll ref backends.

I still dream about having compoundable reference backends, in which
case, ultimately, a "CacheReferenceStore" instance would optionally wrap
on top of an arbitrary other "ReferenceStore" instance (so to speak) to
give it caching functionality.

Michael

-- 
Michael Haggerty
[email-masked]