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

[Junio C Hamano <gitster@pobox.com>]

David Aguilar <davvid@gmail.com> writes:

>> diff --git a/refs.c b/refs.c
>> index 9562325..b9b0244 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -1150,3 +1150,57 @@ int resolve_gitlink_ref(const char *path, const char *refname,
>>  {
>>  	return the_refs_backend->resolve_gitlink_ref(path, refname, sha1);
>>  }
>> +
>> +int head_ref(each_ref_fn fn, void *cb_data)
>> +{
>> +	return the_refs_backend->head_ref(fn, cb_data);
>> +}
>
> My only comment is that it seems like having a single static global
> the_refs_backend seems like it should be avoided.
>
> It seems like the intention was to keep the existing interface
> as-is, which explains why this is using globals, but it seems
> like the refs backend should be part of some "application
> context" struct on the stack or allocated during main().  It can
> then be passed around in the API so that we do not need to have
> a global.

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.

The refs and the object database go hand in hand (i.e. the former
gives the anchoring points to keep objects in the latter alive), so
an instance of "repository" would hold "the_refs_backend", "objects[]",
and possibly "the_index" for that repository.

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.