From: Phillip Wood <phillip.wood123@gmail.com>
Date: Thu, 07 Mar 2019 14:45:01 GMT
Subject: Re: [PATCH 3/3] Make sure refs/rewritten/ is per-worktree
Message-ID: <1e0fe5a0-decb-ccd3-19ee-5fe2e5148fb1@gmail.com>
In-Reply-To: <20190307122917.12811-4-pclouds@gmail.com>

Hi Duy

Thanks for working on this I've had a quick look through and they seem 
to make sense to me although I've not really looked at the refs code before.

On 07/03/2019 12:29, Nguyễn Thái Ngọc Duy wrote:
> a9be29c981 (sequencer: make refs generated by the `label` command
> worktree-local, 2018-04-25) adds refs/rewritten/ as per-worktree
> reference space. Unfortunately (my bad) there are a couple places that
> need update to make sure it's really per-worktree.
> 
>   - add_per_worktree_entries_to_dir() is updated to make sure ref listing
>     look at per-worktree refs/rewritten/ instead of per-repo one [1]
> 
>   - common_list[] is updated so that git_path() returns the correct
>     location. This includes "rev-parse --git-path".
> 
> This mess is created by me. I started trying to fix it with the
> introduction of refs/worktree, where all refs will be per-worktree
> without special treatments. Unfortunate refs/rewritten came before
> refs/worktree so this is all we can do.
> 
> This also fixes logs/refs/worktree not being per-worktree.
> 
> [1] note that ref listing still works sometimes. For example, if you
>      have .git/worktrees/foo/refs/rewritten/bar AND the directory
>      .git/worktrees/refs/rewritten, 

should that be .git/refs/rewritten? (and below)

Best Wishes

Phillip

refs/rewritten/bar will show up.
>      add_per_worktree_entries_to_dir() is only needed when the directory
>      .git/worktrees/refs/rewritten is missing.
> 
> Reported-by: Phillip Wood <phillip.wood123@gmail.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>   path.c                   |  3 +++
>   refs/files-backend.c     |  4 ++--
>   t/t1415-worktree-refs.sh | 35 +++++++++++++++++++++++++++++++++++
>   3 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/path.c b/path.c
> index 03ab712839..25e97b8c3f 100644
> --- a/path.c
> +++ b/path.c
> @@ -115,10 +115,13 @@ static struct common_dir common_list[] = {
>   	{ 1, 1, 0, "logs" },
>   	{ 1, 1, 1, "logs/HEAD" },
>   	{ 0, 1, 1, "logs/refs/bisect" },
> +	{ 0, 1, 1, "logs/refs/rewritten" },
> +	{ 0, 1, 1, "logs/refs/worktree" },
>   	{ 0, 1, 0, "lost-found" },
>   	{ 0, 1, 0, "objects" },
>   	{ 0, 1, 0, "refs" },
>   	{ 0, 1, 1, "refs/bisect" },
> +	{ 0, 1, 1, "refs/rewritten" },
>   	{ 0, 1, 1, "refs/worktree" },
>   	{ 0, 1, 0, "remotes" },
>   	{ 0, 1, 0, "worktrees" },
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 3d0e06edcd..5848f32ef8 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -215,13 +215,13 @@ static void files_ref_path(struct files_ref_store *refs,
>   }
>   
>   /*
> - * Manually add refs/bisect and refs/worktree, which, being
> + * Manually add refs/bisect, refs/rewritten and refs/worktree, which, being
>    * per-worktree, might not appear in the directory listing for
>    * refs/ in the main repo.
>    */
>   static void add_per_worktree_entries_to_dir(struct ref_dir *dir, const char *dirname)
>   {
> -	const char *prefixes[] = { "refs/bisect/", "refs/worktree/" };
> +	const char *prefixes[] = { "refs/bisect/", "refs/worktree/", "refs/rewritten/" };
>   	int ip;
>   
>   	if (strcmp(dirname, "refs/"))
> diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
> index b664e51250..bb2c7572a3 100755
> --- a/t/t1415-worktree-refs.sh
> +++ b/t/t1415-worktree-refs.sh
> @@ -76,4 +76,39 @@ test_expect_success 'reflog of worktrees/xx/HEAD' '
>   	test_cmp expected actual.wt2
>   '
>   
> +test_expect_success 'for-each-ref from main repo' '
> +	mkdir fer1 &&
> +	git -C fer1 init repo &&
> +	test_commit -C fer1/repo initial &&
> +	git -C fer1/repo worktree add ../second &&
> +	git -C fer1/repo update-ref refs/bisect/main HEAD &&
> +	git -C fer1/repo update-ref refs/rewritten/main HEAD &&
> +	git -C fer1/repo update-ref refs/worktree/main HEAD &&
> +	git -C fer1/repo for-each-ref --format="%(refname)" | grep main >actual &&
> +	cat >expected <<-\EOF &&
> +	refs/bisect/main
> +	refs/rewritten/main
> +	refs/worktree/main
> +	EOF
> +	test_cmp expected actual
> +'
> +
> +test_expect_success 'for-each-ref from linked repo' '
> +	mkdir fer2 &&
> +	git -C fer2 init repo &&
> +	test_commit -C fer2/repo initial &&
> +	git -C fer2/repo worktree add ../second &&
> +	git -C fer2/second update-ref refs/bisect/second HEAD &&
> +	git -C fer2/second update-ref refs/rewritten/second HEAD &&
> +	git -C fer2/second update-ref refs/worktree/second HEAD &&
> +	git -C fer2/second for-each-ref --format="%(refname)" | grep second >actual &&
> +	cat >expected <<-\EOF &&
> +	refs/bisect/second
> +	refs/heads/second
> +	refs/rewritten/second
> +	refs/worktree/second
> +	EOF
> +	test_cmp expected actual
> +'
> +
>   test_done
>