Re: [PATCH 12/16] refs: store the main ref store inside the repository struct
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Apr 10, 2018, 14:02 UTC
- Message-ID
- <2c6214de-36b0-c67b-9a40-d43d28e780c4@alum.mit.edu>
- In-Reply-To
- <20180409224533.17764-13-sbeller@google.com>
On 04/10/2018 12:45 AM, Stefan Beller wrote:
Show 70 quoted lines
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> refs.c | 13 +++++--------
> refs.h | 4 +---
> repository.h | 3 +++
> 3 files changed, 9 insertions(+), 11 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index f58b9fb7df..b5be754a97 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1608,9 +1608,6 @@ static struct ref_store_hash_entry *alloc_ref_store_hash_entry(
> return entry;
> }
>
> -/* A pointer to the ref_store for the main repository: */
> -static struct ref_store *main_ref_store;
> -
> /* A hashmap of ref_stores, stored by submodule name: */
> static struct hashmap submodule_ref_stores;
>
> @@ -1652,13 +1649,13 @@ static struct ref_store *ref_store_init(const char *gitdir,
> return refs;
> }
>
> -struct ref_store *get_main_ref_store_the_repository(void)
> +struct ref_store *get_main_ref_store(struct repository *r)
> {
> - if (main_ref_store)
> - return main_ref_store;
> + if (r->main_ref_store)
> + return r->main_ref_store;
>
> - main_ref_store = ref_store_init(get_git_dir(), REF_STORE_ALL_CAPS);
> - return main_ref_store;
> + r->main_ref_store = ref_store_init(r->gitdir, REF_STORE_ALL_CAPS);
> + return r->main_ref_store;
> }
>
> /*
> diff --git a/refs.h b/refs.h
> index ab3d2bec2f..f5ab68c0ed 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -760,9 +760,7 @@ int reflog_expire(const char *refname, const struct object_id *oid,
>
> int ref_storage_backend_exists(const char *name);
>
> -#define get_main_ref_store(r) \
> - get_main_ref_store_##r()
> -struct ref_store *get_main_ref_store_the_repository(void);
> +struct ref_store *get_main_ref_store(struct repository *r);
> /*
> * Return the ref_store instance for the specified submodule. For the
> * main repository, use submodule==NULL; such a call cannot fail. For
> diff --git a/repository.h b/repository.h
> index 09df94a472..7d0710b273 100644
> --- a/repository.h
> +++ b/repository.h
> @@ -26,6 +26,9 @@ struct repository {
> */
> struct raw_object_store *objects;
>
> + /* The store in which the refs are held. */
> + struct ref_store *main_ref_store;
> +
> /*
> * Path to the repository's graft file.
> * Cannot be NULL after initialization.
> 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 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? So maybe calling things `repository.ref_store` and `get_ref_store(repository)` would be appropriate.
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").
Hope that helps, Michael