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

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Jan 5, 2016, 13:43 UTC
Message-ID: <568BC88C.6000703@alum.mit.edu>
In-Reply-To: <xmqqziwlnp7d.fsf@gitster.mtv.corp.google.com>

On 01/04/2016 08:01 PM, Junio C Hamano wrote:

Show 24 quoted lines

> David Aguilar <davvid@gmail.com> 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.

Show 5 quoted lines

> [...]
> 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
mhagger@alum.mit.edu