Re: [PATCH v4 12/16] files-backend: make reflog iterator go through per-worktree reflog

From: Richard Maw <richard.maw@gmail.com>
Date: Aug 24, 2017, 14:13 UTC
Message-ID: <20170824141348.GC2021@mimir>
In-Reply-To: <20170823123704.16518-13-pclouds@gmail.com>

On Wed, Aug 23, 2017 at 07:37:00PM +0700, Nguyễn Thái Ngọc Duy wrote:

Show 22 quoted lines

> refs/bisect is unfortunately per-worktree, so we need to look in
> per-worktree logs/refs/bisect in addition to per-repo logs/refs. The
> current iterator only goes through per-repo logs/refs.
> 
> Use merge iterator to walk two ref stores at the same time and pick
> per-worktree refs from the right iterator.
> 
> PS. Note the unsorted order of for_each_reflog in the test. This is
> supposed to be OK, for now. If we enforce order on for_each_reflog()
> then some more work will be required.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  refs/files-backend.c          | 59 +++++++++++++++++++++++++++++++++----------
>  t/t1407-worktree-ref-store.sh | 30 ++++++++++++++++++++++
>  2 files changed, 75 insertions(+), 14 deletions(-)
> 
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 5cca55510b..d4d22882ef 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -2055,23 +2046,63 @@ static struct ref_iterator_vtable files_reflog_iterator_vtable = {

<snip>

Show 19 quoted lines

> +static struct ref_iterator *files_reflog_iterator_begin(struct ref_store *ref_store)
> +{
> +	struct files_ref_store *refs =
> +		files_downcast(ref_store, REF_STORE_READ,
> +			       "reflog_iterator_begin");
> +
> +	if (!strcmp(refs->gitdir, refs->gitcommondir)) {
> +		return reflog_iterator_begin(ref_store, refs->gitcommondir);
> +	} else {
> +		return merge_ref_iterator_begin(
> +			reflog_iterator_begin(ref_store, refs->gitdir),
> +			reflog_iterator_begin(ref_store, refs->gitcommondir),
> +			reflog_iterator_select, refs);
> +	}
> +}
> +
>  /*
>   * If update is a direct update of head_ref (the reference pointed to
>   * by HEAD), then add an extra REF_LOG_ONLY update for HEAD.

Whilst trying to use ref backends to implement ref namespaces one of the issues I had was that namespaced bisect refs weren't being found.

This turned out to be for the same reason their reflogs weren't found, that by only iterating through the commondir refs it was missing the gitdir ones and that it only worked normally because of a special case in loose_fill_ref_dir

        /*
         * Manually add refs/bisect, which, being per-worktree, might
         * not appear in the directory listing for refs/ in the main
         * repo.
         */
        if (!strcmp(dirname, "refs/")) {
                int pos = search_ref_dir(dir, "refs/bisect/", 12);

                if (pos < 0) {
                        struct ref_entry *child_entry = create_dir_entry(
                                        dir->cache, "refs/bisect/", 12, 1);
                        add_entry_to_dir(dir, child_entry);
                }
        }

If files_ref_iterator_begin was made to use a merged or overlay ref iterator too then this special case could be removed and bisecting in a namespaced workspace would work.

I've yet to work out whether namespaced bisect refs makes sense, but that's a problem for the next time I have time to work on it :).