Hi Michael,
On Tue, Apr 10, 2018 at 7:02 AM, Michael Haggerty <mhagger@xxxxxxxxxxxx> wrote:
> This also makes sense to me, as far as it goes. I have a few comments
> and questions:
>
> Why do you call the new member `main_ref_store`? Is there expected to be
> some other `ref_store` associated with a repository?
I'll rename it in a reroll.
>
> I think the origin of the name `main_ref_store` was to distinguish it
> from submodule ref stores. But presumably those will soon become the
> "main" ref stores for their respective submodule repository objects,
> right?
I hope so.
> So maybe calling things `repository.ref_store` and
> `get_ref_store(repository)` would be appropriate.
ok.
>
> There are some places in the reference code that only work with the main
> repository. The ones that I can think of are:
>
> * `ref_resolves_to_object()` depends on an object store.
>
> * `peel_object()` and `ref_iterator_peel()` also have to look up objects
> (in this case, tag objects).
>
> * Anything that calls `files_assert_main_repository()` or depends on
> `REF_STORE_MAIN` isn't implemented for other reference stores (usually,
> I think, these are functions that depend on the object store).
>
> Some of these things might be easy to generalize to non-main
> repositories, but I didn't bother because AFAIK currently only the main
> repository store is ever mutated.
>
> You can move a now-obsolete comment above the definition of `struct
> files_ref_store` if you haven't in some other patch (search for
> "libification").
ok, I'll have a look at that.
My plan was to remove the submodule accessors from the refs API, and
mandate the access via
repo_submodule_init(&submodule_repo, superproject_repo, path);
sub_ref_store = get_ref_store(submodule_repo);
instead of also having
sub_ref_store = get_submodule_ref_store(path);
as that would ease the refs API (and its internals potentially)
as well as avoiding errors with mixing up repositories. As the
construction of a submodule repository struct requires its
superproject repo, it helps avoiding pitfalls with nested
submodules IMO.
Thanks for the comments,
Stefan
>
> Hope that helps,
> Michael
