Re: [PATCH v3 2/4] refs: introduce get_worktree_ref_store()
- From
Michael Haggerty <mhagger@alum.mit.edu>
- Date
- Mar 20, 2017, 06:59 UTC
- Message-ID
- <b841a674-7b2b-420d-7faa-e5b836fb534e@alum.mit.edu>
- In-Reply-To
- <20170318100206.5980-3-pclouds@gmail.com>
On 03/18/2017 11:02 AM, Nguyễn Thái Ngọc Duy wrote:
Show 64 quoted lines
> files-backend at this point is still aware of the per-repo/worktree
> separation in refs, so it can handle a linked worktree.
>
> Some refs operations are known not working when current files-backend is
> used in a linked worktree (e.g. reflog). Tests will be written when
> refs_* functions start to be called with worktree backend to verify that
> they work as expected.
>
> Note: accessing a worktree of a submodule remains unaddressed. Perhaps
> after get_worktrees() can access submodule (or rather a new function
> get_submodule_worktrees(), that lists worktrees of a submodule), we can
> update this function to work with submodules as well.
> ---
> refs.c | 32 ++++++++++++++++++++++++++++++++
> refs.h | 2 ++
> 2 files changed, 34 insertions(+)
>
> diff --git a/refs.c b/refs.c
> index 77a39f8b17..6695140cfe 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -10,6 +10,7 @@
> #include "object.h"
> #include "tag.h"
> #include "submodule.h"
> +#include "worktree.h"
>
> /*
> * List of all available backends
> @@ -1593,6 +1594,37 @@ struct ref_store *get_submodule_ref_store(const char *submodule)
> return refs;
> }
>
> +struct ref_store *get_worktree_ref_store(const struct worktree *wt)
> +{
> + struct ref_store *refs;
> + unsigned int refs_all_capabilities =
> + REF_STORE_READ | REF_STORE_WRITE |
> + REF_STORE_ODB | REF_STORE_MAIN;
> +
> + if (wt->is_current)
> + return get_main_ref_store();
> +
> + /*
> + * We share the same hash map with submodules for
> + * now. submodule paths are always relative (to topdir) while
> + * worktree paths are always absolute. No chance of conflict.
> + */
> + refs = lookup_submodule_ref_store(wt->path);
> + if (refs)
> + return refs;
> +
> + if (wt->id)
> + refs = ref_store_init(git_common_path("worktrees/%s", wt->id),
> + refs_all_capabilities);
> + else
> + refs = ref_store_init(get_git_common_dir(),
> + refs_all_capabilities);
> +
> + if (refs)
> + register_submodule_ref_store(refs, wt->path);
> + return refs;
> +}
> +I guess I can hold my nose and accept storing worktree and submodule `ref_store`s together in a single hashmap, but not if the code continues to use "submodule" in the names, because that will be a continuing source of confusion to readers. For example,
* lookup_submodule_ref_store() * register_submodule_ref_store() * submodule_hash_entry * submodule_hash_cmp() * alloc_submodule_hash_entry() * submodule_ref_stores
Any docstrings would also have to be updated.
As a new naming scheme, maybe use s/submodule/other/, for example `lookup_other_ref_store()` etc.
> [...]
Michael