[BUG] worktree local refs weirdness

[BUG] worktree local refs weirdness

[Phillip Wood]

When it is run in a worktree 'git for-each-ref' only seems to show refs 
under refs/rewritten if that directory also exists under $GIT_COMMON_DIR 
even though they are local to the worktree.

Initially I thought this was due to $GIT_COMMON_DIR pointing to a bare 
repo, but that is not the case. However while writing a test case which 
cloned to a bare repo I noticed it was listing a ref for the HEAD of a 
worktree in the repo I had cloned from, not the clone itself.

The script below reproduces both issues, I've copied the output below 
it. I'm not sure what's going on or the best way to debug it.

Best Wishes

Phillip

# setup initial repo
mkdir repo &&
cd repo &&
git init &&
echo a>a &&
git add a &&
git commit -ma &&
#add a worktree and set a ref under refs/rewritten
git worktree add ../worktree-1 &&
cd ../worktree-1 &&
echo 'worktree-1 adding refs/rewritten/a' &&
git update-ref refs/rewritten/a HEAD &&
git rev-parse --verify refs/rewritten/a &&
echo 'refs/rewritten/a exists but is not shown' &&
git for-each-ref &&
# add a ref under refs/rewritten to the main repo
cd ../repo &&
echo 'repo adding refs/rewritten/z' &&
git update-ref refs/rewritten/z HEAD &&
git for-each-ref &&
cd ../worktree-1 &&
echo 'worktree-1 now shows refs/rewritten/a' &&
git for-each-ref &&
cd .. &&
# create a bare clone with a worktree
git clone --bare --no-local repo bare &&
cd bare &&
git worktree add ../worktree-2 &&
cd ../worktree-2 &&
echo 'worktree-2 adding refs/rewritten/b' &&
echo 'why does this show refs/worktree-1/head' &&
echo 'but not refs/rewritten/b?' &&
git worktree list &&
git update-ref refs/rewritten/b HEAD &&
git rev-parse --verify refs/rewritten/b &&
git for-each-ref


Output
Initialized empty Git repository in 
/home/phil/src/git-utils/dev/git-rewrite/repo/.git/
[master (root-commit) dc2045a] a
  1 file changed, 1 insertion(+)
  create mode 100644 a
Preparing worktree (new branch 'worktree-1')
HEAD is now at dc2045a a
worktree-1 adding refs/rewritten/a
dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656
refs/rewritten/a exists but is not shown
dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit	refs/heads/master
dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit	refs/heads/worktree-1
repo adding refs/rewritten/z
dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit	refs/heads/master
dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit	refs/heads/worktree-1
dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit	refs/rewritten/z
worktree-1 now shows refs/rewritten/a
dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit	refs/heads/master
dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit	refs/heads/worktree-1
dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit	refs/rewritten/a
Cloning into bare repository 'bare'...
remote: Enumerating objects: 3, done.
remote: Counting objects: 100% (3/3), done.
remote: Total 3 (delta 0), reused 0 (delta 0)
Receiving objects: 100% (3/3), done.
Preparing worktree (new branch 'worktree-2')
HEAD is now at dc2045a a
worktree-2 adding refs/rewritten/b
why does this show refs/worktree-1/head
but not refs/rewritten/b?
/home/phil/src/git-utils/dev/git-rewrite/bare        (bare)
/home/phil/src/git-utils/dev/git-rewrite/worktree-2  dc2045a [worktree-2]
dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656
dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit	refs/heads/master
dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit	refs/heads/worktree-1
dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit	refs/heads/worktree-2

Re: [BUG] worktree local refs weirdness

[Phillip Wood]

On 06/03/2019 15:57, Phillip Wood wrote:
> When it is run in a worktree 'git for-each-ref' only seems to show refs 
> under refs/rewritten if that directory also exists under $GIT_COMMON_DIR 
> even though they are local to the worktree.
> 
> Initially I thought this was due to $GIT_COMMON_DIR pointing to a bare 
> repo, but that is not the case. However while writing a test case which 
> cloned to a bare repo I noticed it was listing a ref for the HEAD of a 
> worktree in the repo I had cloned from, not the clone itself.

Ignore that last paragraph, it's just showing the branch that got 
created when the worktree was created. The last part of the script is 
redundant, I've updated it below


# setup initial repo
mkdir repo &&
cd repo &&
git init &&
echo a>a &&
git add a &&
git commit -ma &&
#add a worktree and set a ref under refs/rewritten
git worktree add ../worktree-1 &&
cd ../worktree-1 &&
echo 'worktree-1 adding refs/rewritten/a' &&
git update-ref refs/rewritten/a HEAD &&
git rev-parse --verify refs/rewritten/a &&
echo 'refs/rewritten/a exists but is not shown' &&
git for-each-ref &&
# add a ref under refs/rewritten to the main repo
cd ../repo &&
echo 'repo adding refs/rewritten/z' &&
git update-ref refs/rewritten/z HEAD &&
git for-each-ref &&
cd ../worktree-1 &&
echo 'worktree-1 now shows refs/rewritten/a' &&
git for-each-ref

Output
Initialized empty Git repository in /tmp/x/repo/.git/
[master (root-commit) d60b5d6] a
  1 file changed, 1 insertion(+)
  create mode 100644 a
Preparing worktree (new branch 'worktree-1')
HEAD is now at d60b5d6 a
worktree-1 adding refs/rewritten/a
d60b5d6532fe5ed6e3bc3a0233c6de9d86092d13
refs/rewritten/a exists but is not shown
d60b5d6532fe5ed6e3bc3a0233c6de9d86092d13 commit	refs/heads/master
d60b5d6532fe5ed6e3bc3a0233c6de9d86092d13 commit	refs/heads/worktree-1
repo adding refs/rewritten/z
d60b5d6532fe5ed6e3bc3a0233c6de9d86092d13 commit	refs/heads/master
d60b5d6532fe5ed6e3bc3a0233c6de9d86092d13 commit	refs/heads/worktree-1
d60b5d6532fe5ed6e3bc3a0233c6de9d86092d13 commit	refs/rewritten/z
worktree-1 now shows refs/rewritten/a
d60b5d6532fe5ed6e3bc3a0233c6de9d86092d13 commit	refs/heads/master
d60b5d6532fe5ed6e3bc3a0233c6de9d86092d13 commit	refs/heads/worktree-1
d60b5d6532fe5ed6e3bc3a0233c6de9d86092d13 commit	refs/rewritten/a

Best Wishes

Phillip


> 
> The script below reproduces both issues, I've copied the output below 
> it. I'm not sure what's going on or the best way to debug it.
> 
> Best Wishes
> 
> Phillip
> 
> # setup initial repo
> mkdir repo &&
> cd repo &&
> git init &&
> echo a>a &&
> git add a &&
> git commit -ma &&
> #add a worktree and set a ref under refs/rewritten
> git worktree add ../worktree-1 &&
> cd ../worktree-1 &&
> echo 'worktree-1 adding refs/rewritten/a' &&
> git update-ref refs/rewritten/a HEAD &&
> git rev-parse --verify refs/rewritten/a &&
> echo 'refs/rewritten/a exists but is not shown' &&
> git for-each-ref &&
> # add a ref under refs/rewritten to the main repo
> cd ../repo &&
> echo 'repo adding refs/rewritten/z' &&
> git update-ref refs/rewritten/z HEAD &&
> git for-each-ref &&
> cd ../worktree-1 &&
> echo 'worktree-1 now shows refs/rewritten/a' &&
> git for-each-ref &&
> cd .. &&
> # create a bare clone with a worktree
> git clone --bare --no-local repo bare &&
> cd bare &&
> git worktree add ../worktree-2 &&
> cd ../worktree-2 &&
> echo 'worktree-2 adding refs/rewritten/b' &&
> echo 'why does this show refs/worktree-1/head' &&
> echo 'but not refs/rewritten/b?' &&
> git worktree list &&
> git update-ref refs/rewritten/b HEAD &&
> git rev-parse --verify refs/rewritten/b &&
> git for-each-ref
> 
> 
> Output
> Initialized empty Git repository in 
> /home/phil/src/git-utils/dev/git-rewrite/repo/.git/
> [master (root-commit) dc2045a] a
>   1 file changed, 1 insertion(+)
>   create mode 100644 a
> Preparing worktree (new branch 'worktree-1')
> HEAD is now at dc2045a a
> worktree-1 adding refs/rewritten/a
> dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656
> refs/rewritten/a exists but is not shown
> dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit    refs/heads/master
> dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit    refs/heads/worktree-1
> repo adding refs/rewritten/z
> dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit    refs/heads/master
> dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit    refs/heads/worktree-1
> dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit    refs/rewritten/z
> worktree-1 now shows refs/rewritten/a
> dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit    refs/heads/master
> dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit    refs/heads/worktree-1
> dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit    refs/rewritten/a
> Cloning into bare repository 'bare'...
> remote: Enumerating objects: 3, done.
> remote: Counting objects: 100% (3/3), done.
> remote: Total 3 (delta 0), reused 0 (delta 0)
> Receiving objects: 100% (3/3), done.
> Preparing worktree (new branch 'worktree-2')
> HEAD is now at dc2045a a
> worktree-2 adding refs/rewritten/b
> why does this show refs/worktree-1/head
> but not refs/rewritten/b?
> /home/phil/src/git-utils/dev/git-rewrite/bare        (bare)
> /home/phil/src/git-utils/dev/git-rewrite/worktree-2  dc2045a [worktree-2]
> dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656
> dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit    refs/heads/master
> dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit    refs/heads/worktree-1
> dc2045a36f38ba4e9bacd8cd86ba74f58a1d4656 commit    refs/heads/worktree-2
> 

Re: [BUG] worktree local refs weirdness

[Duy Nguyen]

On Thu, Mar 7, 2019 at 4:38 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> On 06/03/2019 15:57, Phillip Wood wrote:
> > When it is run in a worktree 'git for-each-ref' only seems to show refs
> > under refs/rewritten if that directory also exists under $GIT_COMMON_DIR
> > even though they are local to the worktree.
> >
> > Initially I thought this was due to $GIT_COMMON_DIR pointing to a bare
> > repo, but that is not the case. However while writing a test case which
> > cloned to a bare repo I noticed it was listing a ref for the HEAD of a
> > worktree in the repo I had cloned from, not the clone itself.
>
> Ignore that last paragraph, it's just showing the branch that got
> created when the worktree was created. The last part of the script is
> redundant, I've updated it below

Thanks for the test script. I could reproduce it and I think I see the
problem (update-ref creates per-worktree refs even though
refs/rewritten should be per-repo). Working on it...
-- 
Duy

Re: [BUG] worktree local refs weirdness

[Duy Nguyen]

On Thu, Mar 7, 2019 at 4:46 PM Duy Nguyen <pclouds@gmail.com> wrote:
>
> On Thu, Mar 7, 2019 at 4:38 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
> >
> > On 06/03/2019 15:57, Phillip Wood wrote:
> > > When it is run in a worktree 'git for-each-ref' only seems to show refs
> > > under refs/rewritten if that directory also exists under $GIT_COMMON_DIR
> > > even though they are local to the worktree.
> > >
> > > Initially I thought this was due to $GIT_COMMON_DIR pointing to a bare
> > > repo, but that is not the case. However while writing a test case which
> > > cloned to a bare repo I noticed it was listing a ref for the HEAD of a
> > > worktree in the repo I had cloned from, not the clone itself.
> >
> > Ignore that last paragraph, it's just showing the branch that got
> > created when the worktree was created. The last part of the script is
> > redundant, I've updated it below
>
> Thanks for the test script. I could reproduce it and I think I see the
> problem (update-ref creates per-worktree refs even though
> refs/rewritten should be per-repo). Working on it...

My mistake. refs/rewritten is per-worktree, but the ref iteration code
has a bug. The fix should come soon.
-- 
Duy

[PATCH 0/3] Fix refs/rewritten not show up in for-each-ref

[Nguyễn Thái Ngọc Duy]

Apparently there is one corner case where refs/rewritten/* may not show
up in for-each-ref in multiple worktree setup. This is because the
per-worktree classification has to be done in three places (my bad!)
and Johannes only updated one.

This should fix it (and it also fixes refs/worktree/ not showing up
under the same condition because I'm an idiot who does not know how to
test).

Nguyễn Thái Ngọc Duy (3):
  files-backend.c: factor out per-worktree code in loose_fill_ref_dir()
  files-backend.c: reduce duplication in
    add_per_worktree_entries_to_dir()
  Make sure refs/rewritten/ is per-worktree

 path.c                   |  3 +++
 refs/files-backend.c     | 50 ++++++++++++++++++++++------------------
 t/t1415-worktree-refs.sh | 35 ++++++++++++++++++++++++++++
 3 files changed, 66 insertions(+), 22 deletions(-)

-- 
2.21.0.rc1.337.gdf7f8d0522

[PATCH 1/3] files-backend.c: factor out per-worktree code in loose_fill_ref_dir()

[Nguyễn Thái Ngọc Duy]

This is the first step for further cleaning up and extending this
function.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs/files-backend.c | 50 +++++++++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index ef053f716c..26417893c8 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -214,6 +214,33 @@ static void files_ref_path(struct files_ref_store *refs,
 	}
 }
 
+/*
+ * Manually add refs/bisect 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)
+{
+	int pos;
+
+	if (strcmp(dirname, "refs/"))
+		return;
+
+	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);
+	}
+
+	pos = search_ref_dir(dir, "refs/worktree/", 11);
+	if (pos < 0) {
+		struct ref_entry *child_entry =
+			create_dir_entry(dir->cache, "refs/worktree/", 11, 1);
+		add_entry_to_dir(dir, child_entry);
+	}
+}
+
 /*
  * Read the loose references from the namespace dirname into dir
  * (without recursing).  dirname must end with '/'.  dir must be the
@@ -297,28 +324,7 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
 	strbuf_release(&path);
 	closedir(d);
 
-	/*
-	 * Manually add refs/bisect and refs/worktree, 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);
-		}
-
-		pos = search_ref_dir(dir, "refs/worktree/", 11);
-
-		if (pos < 0) {
-			struct ref_entry *child_entry = create_dir_entry(
-					dir->cache, "refs/worktree/", 11, 1);
-			add_entry_to_dir(dir, child_entry);
-		}
-	}
+	add_per_worktree_entries_to_dir(dir, dirname);
 }
 
 static struct ref_cache *get_loose_ref_cache(struct files_ref_store *refs)
-- 
2.21.0.rc1.337.gdf7f8d0522

[PATCH 2/3] files-backend.c: reduce duplication in add_per_worktree_entries_to_dir()

[Nguyễn Thái Ngọc Duy]

This function is duplicated to handle refs/bisect/ and refs/worktree/
and a third prefix is coming. Time to clean up.

This also fixes incorrect "refs/worktrees/" length in this code. The
correct length is 14 not 11. The test in the next patch will also cover
this.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs/files-backend.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 26417893c8..3d0e06edcd 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -221,22 +221,22 @@ static void files_ref_path(struct files_ref_store *refs,
  */
 static void add_per_worktree_entries_to_dir(struct ref_dir *dir, const char *dirname)
 {
-	int pos;
+	const char *prefixes[] = { "refs/bisect/", "refs/worktree/" };
+	int ip;
 
 	if (strcmp(dirname, "refs/"))
 		return;
 
-	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);
-	}
+	for (ip = 0; ip < ARRAY_SIZE(prefixes); ip++) {
+		const char *prefix = prefixes[ip];
+		int prefix_len = strlen(prefix);
+		struct ref_entry *child_entry;
+		int pos;
 
-	pos = search_ref_dir(dir, "refs/worktree/", 11);
-	if (pos < 0) {
-		struct ref_entry *child_entry =
-			create_dir_entry(dir->cache, "refs/worktree/", 11, 1);
+		pos = search_ref_dir(dir, prefix, prefix_len);
+		if (pos >= 0)
+			continue;
+		child_entry = create_dir_entry(dir->cache, prefix, prefix_len, 1);
 		add_entry_to_dir(dir, child_entry);
 	}
 }
-- 
2.21.0.rc1.337.gdf7f8d0522

[PATCH 3/3] Make sure refs/rewritten/ is per-worktree

[Nguyễn Thái Ngọc Duy]

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, 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
-- 
2.21.0.rc1.337.gdf7f8d0522

Re: [PATCH 2/3] files-backend.c: reduce duplication in add_per_worktree_entries_to_dir()

[Phillip Wood]

Hi Duy

On 07/03/2019 12:29, Nguyễn Thái Ngọc Duy wrote:
> This function is duplicated to handle refs/bisect/ and refs/worktree/
> and a third prefix is coming. Time to clean up.
> 
> This also fixes incorrect "refs/worktrees/" length in this code. The
> correct length is 14 not 11. The test in the next patch will also cover
> this.

I ran strace this morning and saw

openat(AT_FDCWD, "/tmp/x/repo/.git/refs/worktr", 
O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or 
directory)

I was just about to email you about it but you've already fixed it!

Best Wishes

Phillip

> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>   refs/files-backend.c | 22 +++++++++++-----------
>   1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 26417893c8..3d0e06edcd 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -221,22 +221,22 @@ static void files_ref_path(struct files_ref_store *refs,
>    */
>   static void add_per_worktree_entries_to_dir(struct ref_dir *dir, const char *dirname)
>   {
> -	int pos;
> +	const char *prefixes[] = { "refs/bisect/", "refs/worktree/" };
> +	int ip;
>   
>   	if (strcmp(dirname, "refs/"))
>   		return;
>   
> -	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);
> -	}
> +	for (ip = 0; ip < ARRAY_SIZE(prefixes); ip++) {
> +		const char *prefix = prefixes[ip];
> +		int prefix_len = strlen(prefix);
> +		struct ref_entry *child_entry;
> +		int pos;
>   
> -	pos = search_ref_dir(dir, "refs/worktree/", 11);
> -	if (pos < 0) {
> -		struct ref_entry *child_entry =
> -			create_dir_entry(dir->cache, "refs/worktree/", 11, 1);
> +		pos = search_ref_dir(dir, prefix, prefix_len);
> +		if (pos >= 0)
> +			continue;
> +		child_entry = create_dir_entry(dir->cache, prefix, prefix_len, 1);
>   		add_entry_to_dir(dir, child_entry);
>   	}
>   }
> 

Re: [PATCH 3/3] Make sure refs/rewritten/ is per-worktree

[Phillip Wood]

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
> 

Re: [PATCH 3/3] Make sure refs/rewritten/ is per-worktree

[Duy Nguyen]

On Thu, Mar 7, 2019 at 9:45 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
> > [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)

Yes.
-- 
Duy