[PATCH 0/8] fix per-worktree ref iteration in fsck/reflog expire

[PATCH 0/8] fix per-worktree ref iteration in fsck/reflog expire

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

Sorry it took me so long to revisit these bugs, even though the first
one was reported nearly a year ago. I guess I slept on it way longer
than I should have.

This series opens up the currrent worktree's ref space, allowing one
worktree to see refs from other worktrees. With this, it's possible to
have less confusing error reports (e.g. "heh.. what HEAD are you
talking about?"). And it's possible to peek one worktree from another,
e.g. now I could do

    git checkout main/HEAD
    make test -j8

in one terminal and go back to the original terminal and continue
hacking while tests are being run in the background.

With this issue out of the way, I could now continue to fix the
"forget to look at all HEADs/reflogs" problem in fsck and "reflog
expire". The fsck patches are mostly from Elijah with some code/test
adaptation from me.

I also take this opportunity to try to standardize a common/private
space in $GIT_DIR or ref hierarchy so we don't have to add more rules
in the future.

One heads up. One remaining problem with "refs and worktrees" is the
ability to completely separate ref space between worktrees (i.e.
refs/heads/master on worktree A is completely different than one in
worktree B). This is needed to make use worktrees in submodules. But
the way I see it, per-worktree refs may have to be moved back to
$GIT_COMMON_DIR/refs to be efficient.

This may be backward incompatible change. Haven't thought it through
yet (and didn't see it coming because I largely ignored refs/bisect,
which should have made me think about this much earlier)

Elijah Newren (2):
  fsck: Move fsck_head_link() to get_default_heads() to avoid some globals
  fsck: check HEAD and reflog from other worktrees

Nguyễn Thái Ngọc Duy (6):
  refs.c: indent with tabs, not spaces
  Add a place for (not) sharing stuff between worktrees
  refs: new ref types to make per-worktree refs visible to all worktrees
  revision.c: correct a parameter name
  revision.c: better error reporting on ref from different worktrees
  reflog expire: cover reflog from all worktrees

 Documentation/git-reflog.txt           |  7 ++-
 Documentation/gitrepository-layout.txt | 11 ++++-
 builtin/fsck.c                         | 68 ++++++++++++++++++--------
 builtin/reflog.c                       | 22 +++++++--
 path.c                                 |  1 +
 refs.c                                 | 23 ++++++++-
 refs.h                                 |  8 +--
 refs/files-backend.c                   | 42 ++++++++++++++--
 revision.c                             | 22 ++++++---
 t/t0060-path-utils.sh                  |  2 +
 t/t1415-worktree-refs.sh               | 66 +++++++++++++++++++++++++
 t/t1450-fsck.sh                        | 39 +++++++++++++++
 worktree.c                             | 32 ++++++++++--
 worktree.h                             | 14 ++++++
 14 files changed, 312 insertions(+), 45 deletions(-)
 create mode 100755 t/t1415-worktree-refs.sh

-- 
2.19.0.647.gb9a6049235

[PATCH 1/8] refs.c: indent with tabs, not spaces

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

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index a7a75b4cc0..9f7268d5fe 100644
--- a/refs.c
+++ b/refs.c
@@ -646,7 +646,7 @@ enum ref_type ref_type(const char *refname)
 		return REF_TYPE_PER_WORKTREE;
 	if (is_pseudoref_syntax(refname))
 		return REF_TYPE_PSEUDOREF;
-       return REF_TYPE_NORMAL;
+	return REF_TYPE_NORMAL;
 }
 
 long get_files_ref_lock_timeout_ms(void)
-- 
2.19.0.647.gb9a6049235

[PATCH 2/8] Add a place for (not) sharing stuff between worktrees

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

When multiple worktrees are used, we need rules to determine if
something belongs to one worktree or all of them. Instead of keeping
adding rules when new stuff comes, have a generic rule:

- Inside $GIT_DIR, which is per-worktree by default, add
  $GIT_DIR/common which is always shared. New features that want to
  share stuff should put stuff under this directory.

- Inside refs/, which is shared by default except refs/bisect, add
  refs/local/ which is per-worktree. We may eventually move
  refs/bisect to this new location and remove the exception in refs
  code.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/gitrepository-layout.txt | 11 ++++++--
 path.c                                 |  1 +
 refs.c                                 |  1 +
 refs/files-backend.c                   | 14 +++++++---
 t/t0060-path-utils.sh                  |  2 ++
 t/t1415-worktree-refs.sh               | 36 ++++++++++++++++++++++++++
 6 files changed, 60 insertions(+), 5 deletions(-)
 create mode 100755 t/t1415-worktree-refs.sh

diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index e85148f05e..fad404ed7c 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -95,8 +95,10 @@ refs::
 	References are stored in subdirectories of this
 	directory.  The 'git prune' command knows to preserve
 	objects reachable from refs found in this directory and
-	its subdirectories. This directory is ignored if $GIT_COMMON_DIR
-	is set and "$GIT_COMMON_DIR/refs" will be used instead.
+	its subdirectories.
+	This directory is ignored (except refs/bisect and refs/local)
+	if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/refs" will be
+	used instead.
 
 refs/heads/`name`::
 	records tip-of-the-tree commit objects of branch `name`
@@ -165,6 +167,11 @@ hooks::
 	each hook. This directory is ignored if $GIT_COMMON_DIR is set
 	and "$GIT_COMMON_DIR/hooks" will be used instead.
 
+common::
+	When multiple working trees are used, most of files in
+	$GIT_DIR are per-worktree with a few known exceptions. All
+	files under 'common' however will be shared between all
+	working trees.
 
 index::
 	The current index file for the repository.  It is
diff --git a/path.c b/path.c
index 34f0f98349..7eb61bf31b 100644
--- a/path.c
+++ b/path.c
@@ -108,6 +108,7 @@ struct common_dir {
 
 static struct common_dir common_list[] = {
 	{ 0, 1, 0, "branches" },
+	{ 0, 1, 0, "common" },
 	{ 0, 1, 0, "hooks" },
 	{ 0, 1, 0, "info" },
 	{ 0, 0, 1, "info/sparse-checkout" },
diff --git a/refs.c b/refs.c
index 9f7268d5fe..a851ef085b 100644
--- a/refs.c
+++ b/refs.c
@@ -624,6 +624,7 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)
 static int is_per_worktree_ref(const char *refname)
 {
 	return !strcmp(refname, "HEAD") ||
+		starts_with(refname, "refs/local/") ||
 		starts_with(refname, "refs/bisect/") ||
 		starts_with(refname, "refs/rewritten/");
 }
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 16ef9325e0..416eafa453 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -269,9 +269,9 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
 	closedir(d);
 
 	/*
-	 * Manually add refs/bisect, which, being per-worktree, might
-	 * not appear in the directory listing for refs/ in the main
-	 * repo.
+	 * Manually add refs/bisect and refs/local, 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);
@@ -281,6 +281,14 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
 					dir->cache, "refs/bisect/", 12, 1);
 			add_entry_to_dir(dir, child_entry);
 		}
+
+		pos = search_ref_dir(dir, "refs/local/", 11);
+
+		if (pos < 0) {
+			struct ref_entry *child_entry = create_dir_entry(
+					dir->cache, "refs/local/", 11, 1);
+			add_entry_to_dir(dir, child_entry);
+		}
 	}
 }
 
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index cd74c0a471..c7b53e494b 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -306,6 +306,8 @@ test_git_path GIT_COMMON_DIR=bar hooks/me                 bar/hooks/me
 test_git_path GIT_COMMON_DIR=bar config                   bar/config
 test_git_path GIT_COMMON_DIR=bar packed-refs              bar/packed-refs
 test_git_path GIT_COMMON_DIR=bar shallow                  bar/shallow
+test_git_path GIT_COMMON_DIR=bar common                   bar/common
+test_git_path GIT_COMMON_DIR=bar common/file              bar/common/file
 
 # In the tests below, $(pwd) must be used because it is a native path on
 # Windows and avoids MSYS's path mangling (which simplifies "foo/../bar" and
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
new file mode 100755
index 0000000000..0c2d5f89a9
--- /dev/null
+++ b/t/t1415-worktree-refs.sh
@@ -0,0 +1,36 @@
+#!/bin/sh
+
+test_description='per-worktree refs'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit initial &&
+	test_commit wt1 &&
+	test_commit wt2 &&
+	git worktree add wt1 wt1 &&
+	git worktree add wt2 wt2 &&
+	git checkout initial
+'
+
+test_expect_success 'add refs/local' '
+	git update-ref refs/local/foo HEAD &&
+	git -C wt1 update-ref refs/local/foo HEAD &&
+	git -C wt2 update-ref refs/local/foo HEAD
+'
+
+test_expect_success 'refs/local must not be packed' '
+	git pack-refs --all &&
+	test_path_is_missing .git/refs/tags/wt1 &&
+	test_path_is_file .git/refs/local/foo &&
+	test_path_is_file .git/worktrees/wt1/refs/local/foo &&
+	test_path_is_file .git/worktrees/wt2/refs/local/foo
+'
+
+test_expect_success 'refs/local are per-worktree' '
+	test_cmp_rev local/foo initial &&
+	( cd wt1 && test_cmp_rev local/foo wt1 ) &&
+	( cd wt2 && test_cmp_rev local/foo wt2 )
+'
+
+test_done
-- 
2.19.0.647.gb9a6049235

[PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

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

One of the problems with multiple worktree is accessing per-worktree
refs of one worktree from another worktree. This was sort of solved by
multiple ref store, where the code can open the ref store of another
worktree and has access to the ref space of that worktree.

The problem with this is reporting. "HEAD" in another ref space is
also called "HEAD" like in the current ref space. In order to
differentiate them, all the code must somehow carry the ref store
around and print something like "HEAD from this ref store".

But that is not feasible (or possible with a _lot_ of work). With the
current design, we pass a reference around as a string (so called
"refname"). Extending this design to pass a string _and_ a ref store
is a nightmare, especially when handling extended SHA-1 syntax.

So we do it another way. Instead of entering a separate ref space, we
make refs from other worktrees available in the current ref space. So
"HEAD" is always HEAD of the current worktree, but then we can have
"worktrees/blah/HEAD" to denote HEAD from a worktree named
"blah". This syntax coincidentally matches the underlying directory
structure which makes implementation a bit easier.

The main worktree has to be treated specially because well.. it's
special from the beginning. So HEAD from the main worktree is
acccessible via the name "main/HEAD" (we can't use
"worktrees/main/HEAD" because "main" under "worktrees" is not
reserved).

This patch also makes it possible to specify refs from one worktree in
another one, e.g.

    git log worktrees/foo/HEAD

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs.c                   | 20 ++++++++++++++++++++
 refs.h                   |  8 +++++---
 refs/files-backend.c     | 28 ++++++++++++++++++++++++++++
 t/t1415-worktree-refs.sh | 30 ++++++++++++++++++++++++++++++
 4 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/refs.c b/refs.c
index a851ef085b..90b73c7334 100644
--- a/refs.c
+++ b/refs.c
@@ -641,12 +641,32 @@ static int is_pseudoref_syntax(const char *refname)
 	return 1;
 }
 
+static int is_main_pseudoref_syntax(const char *refname)
+{
+	return skip_prefix(refname, "main/", &refname) &&
+		is_pseudoref_syntax(refname);
+}
+
+static int is_other_pseudoref_syntax(const char *refname)
+{
+	if (!skip_prefix(refname, "worktrees/", &refname))
+		return 0;
+	refname = strchr(refname, '/');
+	if (!refname)
+		return 0;
+	return is_pseudoref_syntax(refname + 1);
+}
+
 enum ref_type ref_type(const char *refname)
 {
 	if (is_per_worktree_ref(refname))
 		return REF_TYPE_PER_WORKTREE;
 	if (is_pseudoref_syntax(refname))
 		return REF_TYPE_PSEUDOREF;
+	if (is_main_pseudoref_syntax(refname))
+		return REF_TYPE_MAIN_PSEUDOREF;
+	if (is_other_pseudoref_syntax(refname))
+		return REF_TYPE_OTHER_PSEUDOREF;
 	return REF_TYPE_NORMAL;
 }
 
diff --git a/refs.h b/refs.h
index bd52c1bbae..9b53dbeae8 100644
--- a/refs.h
+++ b/refs.h
@@ -704,9 +704,11 @@ int parse_hide_refs_config(const char *var, const char *value, const char *);
 int ref_is_hidden(const char *, const char *);
 
 enum ref_type {
-	REF_TYPE_PER_WORKTREE,
-	REF_TYPE_PSEUDOREF,
-	REF_TYPE_NORMAL,
+	REF_TYPE_PER_WORKTREE,	  /* refs inside refs/ but not shared       */
+	REF_TYPE_PSEUDOREF,	  /* refs outside refs/ in current worktree */
+	REF_TYPE_MAIN_PSEUDOREF,  /* pseudo refs from the main worktree     */
+	REF_TYPE_OTHER_PSEUDOREF, /* pseudo refs from other worktrees       */
+	REF_TYPE_NORMAL,	  /* normal/shared refs inside refs/        */
 };
 
 enum ref_type ref_type(const char *refname);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 416eafa453..bf9ed633b1 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -149,6 +149,23 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store,
 	return refs;
 }
 
+static void files_reflog_path_other_worktrees(struct files_ref_store *refs,
+					      struct strbuf *sb,
+					      const char *refname)
+{
+	const char *real_ref;
+
+	if (!skip_prefix(refname, "worktrees/", &real_ref))
+		BUG("refname %s is not a other-worktree ref", refname);
+	real_ref = strchr(real_ref, '/');
+	if (!real_ref)
+		BUG("refname %s is not a other-worktree ref", refname);
+	real_ref++;
+
+	strbuf_addf(sb, "%s/%.*slogs/%s", refs->gitcommondir,
+		    (int)(real_ref - refname), refname, real_ref);
+}
+
 static void files_reflog_path(struct files_ref_store *refs,
 			      struct strbuf *sb,
 			      const char *refname)
@@ -158,6 +175,12 @@ static void files_reflog_path(struct files_ref_store *refs,
 	case REF_TYPE_PSEUDOREF:
 		strbuf_addf(sb, "%s/logs/%s", refs->gitdir, refname);
 		break;
+	case REF_TYPE_OTHER_PSEUDOREF:
+		return files_reflog_path_other_worktrees(refs, sb, refname);
+	case REF_TYPE_MAIN_PSEUDOREF:
+		if (!skip_prefix(refname, "main/", &refname))
+			BUG("ref %s is not a main pseudoref", refname);
+		/* passthru */
 	case REF_TYPE_NORMAL:
 		strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir, refname);
 		break;
@@ -176,6 +199,11 @@ static void files_ref_path(struct files_ref_store *refs,
 	case REF_TYPE_PSEUDOREF:
 		strbuf_addf(sb, "%s/%s", refs->gitdir, refname);
 		break;
+	case REF_TYPE_MAIN_PSEUDOREF:
+		if (!skip_prefix(refname, "main/", &refname))
+			BUG("ref %s is not a main pseudoref", refname);
+		/* passthru */
+	case REF_TYPE_OTHER_PSEUDOREF:
 	case REF_TYPE_NORMAL:
 		strbuf_addf(sb, "%s/%s", refs->gitcommondir, refname);
 		break;
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
index 0c2d5f89a9..46ca7bfc19 100755
--- a/t/t1415-worktree-refs.sh
+++ b/t/t1415-worktree-refs.sh
@@ -33,4 +33,34 @@ test_expect_success 'refs/local are per-worktree' '
 	( cd wt2 && test_cmp_rev local/foo wt2 )
 '
 
+test_expect_success 'resolve main/HEAD' '
+	test_cmp_rev main/HEAD initial &&
+	( cd wt1 && test_cmp_rev main/HEAD initial ) &&
+	( cd wt2 && test_cmp_rev main/HEAD initial )
+'
+
+test_expect_success 'resolve worktrees/xx/HEAD' '
+	test_cmp_rev worktrees/wt1/HEAD wt1 &&
+	( cd wt1 && test_cmp_rev worktrees/wt1/HEAD wt1 ) &&
+	( cd wt2 && test_cmp_rev worktrees/wt1/HEAD wt1 )
+'
+
+test_expect_success 'reflog of main/HEAD' '
+	git reflog HEAD | sed "s/HEAD/main\/HEAD/" >expected &&
+	git reflog main/HEAD >actual &&
+	test_cmp expected actual &&
+	git -C wt1 reflog main/HEAD >actual.wt1 &&
+	test_cmp expected actual.wt1
+'
+
+test_expect_success 'reflog of worktrees/xx/HEAD' '
+	git -C wt2 reflog HEAD | sed "s/HEAD/worktrees\/wt2\/HEAD/" >expected &&
+	git reflog worktrees/wt2/HEAD >actual &&
+	test_cmp expected actual &&
+	git -C wt1 reflog worktrees/wt2/HEAD >actual.wt1 &&
+	test_cmp expected actual.wt1 &&
+	git -C wt2 reflog worktrees/wt2/HEAD >actual.wt2 &&
+	test_cmp expected actual.wt2
+'
+
 test_done
-- 
2.19.0.647.gb9a6049235

[PATCH 4/8] revision.c: correct a parameter name

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

This function is a callback of for_each_reflog() which will pass a ref
name as the first argument, not a path (to a reflog file).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/revision.c b/revision.c
index e18bd530e4..63aae722c1 100644
--- a/revision.c
+++ b/revision.c
@@ -1277,13 +1277,14 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
 	return 0;
 }
 
-static int handle_one_reflog(const char *path, const struct object_id *oid,
+static int handle_one_reflog(const char *refname,
+			     const struct object_id *oid,
 			     int flag, void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
 	cb->warned_bad_reflog = 0;
-	cb->name_for_errormsg = path;
-	refs_for_each_reflog_ent(cb->refs, path,
+	cb->name_for_errormsg = refname;
+	refs_for_each_reflog_ent(cb->refs, refname,
 				 handle_one_reflog_ent, cb_data);
 	return 0;
 }
-- 
2.19.0.647.gb9a6049235

[PATCH 5/8] revision.c: better error reporting on ref from different worktrees

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

Make use of the new ref aliases to pass refs from another worktree
around and access them from the current ref store instead. This does
not change any functionality, but when a problem shows up, we would
report something like

    fatal: bad object worktrees/ztemp/HEAD
    warning: reflog of 'main/HEAD' references pruned commits

instead of

    fatal: bad object HEAD
    warning: reflog of 'HEAD' references pruned commits

which does not really tell where the refs are from.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 21 +++++++++++++--------
 worktree.c | 32 +++++++++++++++++++++++++++++---
 worktree.h | 14 ++++++++++++++
 3 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/revision.c b/revision.c
index 63aae722c1..8ce660e3b1 100644
--- a/revision.c
+++ b/revision.c
@@ -1177,7 +1177,7 @@ struct all_refs_cb {
 	int warned_bad_reflog;
 	struct rev_info *all_revs;
 	const char *name_for_errormsg;
-	struct ref_store *refs;
+	struct worktree *wt;
 };
 
 int ref_excluded(struct string_list *ref_excludes, const char *path)
@@ -1214,7 +1214,7 @@ static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs,
 	cb->all_revs = revs;
 	cb->all_flags = flags;
 	revs->rev_input_given = 1;
-	cb->refs = NULL;
+	cb->wt = NULL;
 }
 
 void clear_ref_exclusion(struct string_list **ref_excludes_p)
@@ -1277,15 +1277,20 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
 	return 0;
 }
 
-static int handle_one_reflog(const char *refname,
+static int handle_one_reflog(const char *refname_in_wt,
 			     const struct object_id *oid,
 			     int flag, void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
+	struct strbuf refname = STRBUF_INIT;
+
 	cb->warned_bad_reflog = 0;
-	cb->name_for_errormsg = refname;
-	refs_for_each_reflog_ent(cb->refs, refname,
+	strbuf_worktree_ref(cb->wt, &refname, refname_in_wt);
+	cb->name_for_errormsg = refname.buf;
+	refs_for_each_reflog_ent(get_main_ref_store(the_repository),
+				 refname.buf,
 				 handle_one_reflog_ent, cb_data);
+	strbuf_release(&refname);
 	return 0;
 }
 
@@ -1300,8 +1305,8 @@ static void add_other_reflogs_to_pending(struct all_refs_cb *cb)
 		if (wt->is_current)
 			continue;
 
-		cb->refs = get_worktree_ref_store(wt);
-		refs_for_each_reflog(cb->refs,
+		cb->wt = wt;
+		refs_for_each_reflog(get_worktree_ref_store(wt),
 				     handle_one_reflog,
 				     cb);
 	}
@@ -1314,7 +1319,7 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags)
 
 	cb.all_revs = revs;
 	cb.all_flags = flags;
-	cb.refs = get_main_ref_store(the_repository);
+	cb.wt = NULL;
 	for_each_reflog(handle_one_reflog, &cb);
 
 	if (!revs->single_worktree)
diff --git a/worktree.c b/worktree.c
index b0d0b5426d..ec1a5bc511 100644
--- a/worktree.c
+++ b/worktree.c
@@ -487,6 +487,28 @@ int submodule_uses_worktrees(const char *path)
 	return ret;
 }
 
+void strbuf_worktree_ref(const struct worktree *wt,
+			 struct strbuf *sb,
+			 const char *refname)
+{
+	if (wt && !wt->is_current) {
+		if (is_main_worktree(wt))
+			strbuf_addstr(sb, "main/");
+		else
+			strbuf_addf(sb, "worktrees/%s/", wt->id);
+	}
+	strbuf_addstr(sb, refname);
+}
+
+const char *worktree_ref(const struct worktree *wt, const char *refname)
+{
+	static struct strbuf sb = STRBUF_INIT;
+
+	strbuf_reset(&sb);
+	strbuf_worktree_ref(wt, &sb, refname);
+	return sb.buf;
+}
+
 int other_head_refs(each_ref_fn fn, void *cb_data)
 {
 	struct worktree **worktrees, **p;
@@ -495,13 +517,17 @@ int other_head_refs(each_ref_fn fn, void *cb_data)
 	worktrees = get_worktrees(0);
 	for (p = worktrees; *p; p++) {
 		struct worktree *wt = *p;
-		struct ref_store *refs;
+		struct object_id oid;
+		int flag;
 
 		if (wt->is_current)
 			continue;
 
-		refs = get_worktree_ref_store(wt);
-		ret = refs_head_ref(refs, fn, cb_data);
+		if (!refs_read_ref_full(get_main_ref_store(the_repository),
+					worktree_ref(wt, "HEAD"),
+					RESOLVE_REF_READING,
+					&oid, &flag))
+			ret = fn(worktree_ref(wt, "HEAD"), &oid, flag, cb_data);
 		if (ret)
 			break;
 	}
diff --git a/worktree.h b/worktree.h
index df3fc30f73..0016eb9e88 100644
--- a/worktree.h
+++ b/worktree.h
@@ -108,4 +108,18 @@ extern const char *worktree_git_path(const struct worktree *wt,
 				     const char *fmt, ...)
 	__attribute__((format (printf, 2, 3)));
 
+/*
+ * Return a refname suitable for access from the current ref store.
+ */
+void strbuf_worktree_ref(const struct worktree *wt,
+			 struct strbuf *sb,
+			 const char *refname);
+
+/*
+ * Return a refname suitable for access from the current ref
+ * store. The result may be destroyed at the next call.
+ */
+const char *worktree_ref(const struct worktree *wt,
+			 const char *refname);
+
 #endif
-- 
2.19.0.647.gb9a6049235

[PATCH 6/8] fsck: Move fsck_head_link() to get_default_heads() to avoid some globals

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

From: Elijah Newren <newren@gmail.com>

This will make it easier to check the HEAD of other worktrees from fsck.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/fsck.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 63c8578cc1..24f8a09a3c 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -36,8 +36,6 @@ static int check_strict;
 static int keep_cache_objects;
 static struct fsck_options fsck_walk_options = FSCK_OPTIONS_DEFAULT;
 static struct fsck_options fsck_obj_options = FSCK_OPTIONS_DEFAULT;
-static struct object_id head_oid;
-static const char *head_points_at;
 static int errors_found;
 static int write_lost_and_found;
 static int verbose;
@@ -484,8 +482,15 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,
 	return 0;
 }
 
+static int fsck_head_link(const char **head_points_at,
+			  struct object_id *head_oid);
+
 static void get_default_heads(void)
 {
+	const char *head_points_at;
+	struct object_id head_oid;
+
+	fsck_head_link(&head_points_at, &head_oid);
 	if (head_points_at && !is_null_oid(&head_oid))
 		fsck_handle_ref("HEAD", &head_oid, 0, NULL);
 	for_each_rawref(fsck_handle_ref, NULL);
@@ -579,33 +584,34 @@ static void fsck_object_dir(const char *path)
 	stop_progress(&progress);
 }
 
-static int fsck_head_link(void)
+static int fsck_head_link(const char **head_points_at,
+			  struct object_id *head_oid)
 {
 	int null_is_error = 0;
 
 	if (verbose)
 		fprintf(stderr, "Checking HEAD link\n");
 
-	head_points_at = resolve_ref_unsafe("HEAD", 0, &head_oid, NULL);
-	if (!head_points_at) {
+	*head_points_at = resolve_ref_unsafe("HEAD", 0, head_oid, NULL);
+	if (!*head_points_at) {
 		errors_found |= ERROR_REFS;
 		return error("Invalid HEAD");
 	}
-	if (!strcmp(head_points_at, "HEAD"))
+	if (!strcmp(*head_points_at, "HEAD"))
 		/* detached HEAD */
 		null_is_error = 1;
-	else if (!starts_with(head_points_at, "refs/heads/")) {
+	else if (!starts_with(*head_points_at, "refs/heads/")) {
 		errors_found |= ERROR_REFS;
 		return error("HEAD points to something strange (%s)",
-			     head_points_at);
+			     *head_points_at);
 	}
-	if (is_null_oid(&head_oid)) {
+	if (is_null_oid(head_oid)) {
 		if (null_is_error) {
 			errors_found |= ERROR_REFS;
 			return error("HEAD: detached HEAD points at nothing");
 		}
 		fprintf(stderr, "notice: HEAD points to an unborn branch (%s)\n",
-			head_points_at + 11);
+			*head_points_at + 11);
 	}
 	return 0;
 }
@@ -720,7 +726,6 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 
 	git_config(fsck_config, NULL);
 
-	fsck_head_link();
 	if (connectivity_only) {
 		for_each_loose_object(mark_loose_for_connectivity, NULL, 0);
 		for_each_packed_object(mark_packed_for_connectivity, NULL, 0);
-- 
2.19.0.647.gb9a6049235

[PATCH 7/8] fsck: check HEAD and reflog from other worktrees

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

From: Elijah Newren <newren@gmail.com>

fsck is a repo-wide operation and should check all references no
matter which worktree they are associated to.

Reported-by: Jeff King <peff@peff.net>
Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/fsck.c  | 55 ++++++++++++++++++++++++++++++++++---------------
 t/t1450-fsck.sh | 39 +++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 17 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 24f8a09a3c..71492c158d 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -19,6 +19,7 @@
 #include "packfile.h"
 #include "object-store.h"
 #include "run-command.h"
+#include "worktree.h"
 
 #define REACHABLE 0x0001
 #define SEEN      0x0002
@@ -444,7 +445,11 @@ static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noid
 static int fsck_handle_reflog(const char *logname, const struct object_id *oid,
 			      int flag, void *cb_data)
 {
-	for_each_reflog_ent(logname, fsck_handle_reflog_ent, (void *)logname);
+	struct strbuf refname = STRBUF_INIT;
+
+	strbuf_worktree_ref(cb_data, &refname, logname);
+	for_each_reflog_ent(refname.buf, fsck_handle_reflog_ent, refname.buf);
+	strbuf_release(&refname);
 	return 0;
 }
 
@@ -482,20 +487,34 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,
 	return 0;
 }
 
-static int fsck_head_link(const char **head_points_at,
+static int fsck_head_link(const char *head_ref_name,
+			  const char **head_points_at,
 			  struct object_id *head_oid);
 
 static void get_default_heads(void)
 {
+	struct worktree **worktrees, **p;
 	const char *head_points_at;
 	struct object_id head_oid;
 
-	fsck_head_link(&head_points_at, &head_oid);
-	if (head_points_at && !is_null_oid(&head_oid))
-		fsck_handle_ref("HEAD", &head_oid, 0, NULL);
 	for_each_rawref(fsck_handle_ref, NULL);
-	if (include_reflogs)
-		for_each_reflog(fsck_handle_reflog, NULL);
+
+	worktrees = get_worktrees(0);
+	for (p = worktrees; *p; p++) {
+		struct worktree *wt = *p;
+		struct strbuf ref = STRBUF_INIT;
+
+		strbuf_worktree_ref(wt, &ref, "HEAD");
+		fsck_head_link(ref.buf, &head_points_at, &head_oid);
+		if (head_points_at && !is_null_oid(&head_oid))
+			fsck_handle_ref(ref.buf, &head_oid, 0, NULL);
+		strbuf_release(&ref);
+
+		if (include_reflogs)
+			refs_for_each_reflog(get_worktree_ref_store(wt),
+					     fsck_handle_reflog, wt);
+	}
+	free_worktrees(worktrees);
 
 	/*
 	 * Not having any default heads isn't really fatal, but
@@ -584,34 +603,36 @@ static void fsck_object_dir(const char *path)
 	stop_progress(&progress);
 }
 
-static int fsck_head_link(const char **head_points_at,
+static int fsck_head_link(const char *head_ref_name,
+			  const char **head_points_at,
 			  struct object_id *head_oid)
 {
 	int null_is_error = 0;
 
 	if (verbose)
-		fprintf(stderr, "Checking HEAD link\n");
+		fprintf(stderr, "Checking %s link\n", head_ref_name);
 
-	*head_points_at = resolve_ref_unsafe("HEAD", 0, head_oid, NULL);
+	*head_points_at = resolve_ref_unsafe(head_ref_name, 0, head_oid, NULL);
 	if (!*head_points_at) {
 		errors_found |= ERROR_REFS;
-		return error("Invalid HEAD");
+		return error("Invalid %s", head_ref_name);
 	}
-	if (!strcmp(*head_points_at, "HEAD"))
+	if (!strcmp(*head_points_at, head_ref_name))
 		/* detached HEAD */
 		null_is_error = 1;
 	else if (!starts_with(*head_points_at, "refs/heads/")) {
 		errors_found |= ERROR_REFS;
-		return error("HEAD points to something strange (%s)",
-			     *head_points_at);
+		return error("%s points to something strange (%s)",
+			     head_ref_name, *head_points_at);
 	}
 	if (is_null_oid(head_oid)) {
 		if (null_is_error) {
 			errors_found |= ERROR_REFS;
-			return error("HEAD: detached HEAD points at nothing");
+			return error("%s: detached HEAD points at nothing",
+				     head_ref_name);
 		}
-		fprintf(stderr, "notice: HEAD points to an unborn branch (%s)\n",
-			*head_points_at + 11);
+		fprintf(stderr, "notice: %s points to an unborn branch (%s)\n",
+			head_ref_name, *head_points_at + 11);
 	}
 	return 0;
 }
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 0f2dd26f74..444e8c1ad9 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -101,6 +101,45 @@ test_expect_success 'HEAD link pointing at a funny place' '
 	grep "HEAD points to something strange" out
 '
 
+test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
+	test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
+	test_when_finished "rm -rf .git/worktrees wt" &&
+	git worktree add wt &&
+	mv .git/HEAD .git/SAVED_HEAD &&
+	echo 0000000000000000000000000000000000000000 >.git/HEAD &&
+	# avoid corrupt/broken HEAD from interfering with repo discovery
+	test_must_fail git -C wt fsck 2>out &&
+	cat out &&
+	grep "main/HEAD: detached HEAD points" out
+'
+
+test_expect_success 'other worktree HEAD link pointing at a funny object' '
+	test_when_finished "rm -rf .git/worktrees other" &&
+	git worktree add other &&
+	echo 0000000000000000000000000000000000000000 >.git/worktrees/other/HEAD &&
+	test_must_fail git fsck 2>out &&
+	cat out &&
+	grep "worktrees/other/HEAD: detached HEAD points" out
+'
+
+test_expect_success 'other worktree HEAD link pointing at missing object' '
+	test_when_finished "rm -rf .git/worktrees other" &&
+	git worktree add other &&
+	echo "Contents missing from repo" | git hash-object --stdin >.git/worktrees/other/HEAD &&
+	test_must_fail git fsck 2>out &&
+	cat out &&
+	grep "worktrees/other/HEAD: invalid sha1 pointer" out
+'
+
+test_expect_success 'other worktree HEAD link pointing at a funny place' '
+	test_when_finished "rm -rf .git/worktrees other" &&
+	git worktree add other &&
+	echo "ref: refs/funny/place" >.git/worktrees/other/HEAD &&
+	test_must_fail git fsck 2>out &&
+	cat out &&
+	grep "worktrees/other/HEAD points to something strange" out
+'
+
 test_expect_success 'email without @ is okay' '
 	git cat-file commit HEAD >basis &&
 	sed "s/@/AT/" basis >okay &&
-- 
2.19.0.647.gb9a6049235

[PATCH 8/8] reflog expire: cover reflog from all worktrees

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

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-reflog.txt |  7 ++++++-
 builtin/reflog.c             | 22 +++++++++++++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt
index 472a6808cd..ff487ff77d 100644
--- a/Documentation/git-reflog.txt
+++ b/Documentation/git-reflog.txt
@@ -20,7 +20,7 @@ depending on the subcommand:
 'git reflog' ['show'] [log-options] [<ref>]
 'git reflog expire' [--expire=<time>] [--expire-unreachable=<time>]
 	[--rewrite] [--updateref] [--stale-fix]
-	[--dry-run | -n] [--verbose] [--all | <refs>...]
+	[--dry-run | -n] [--verbose] [--all [--single-worktree] | <refs>...]
 'git reflog delete' [--rewrite] [--updateref]
 	[--dry-run | -n] [--verbose] ref@\{specifier\}...
 'git reflog exists' <ref>
@@ -72,6 +72,11 @@ Options for `expire`
 --all::
 	Process the reflogs of all references.
 
+--single-worktree::
+	By default when `--all` is specified, reflogs from all working
+	trees are processed. This option limits the processing to reflogs
+	from the current working tree only.
+
 --expire=<time>::
 	Prune entries older than the specified time. If this option is
 	not specified, the expiration time is taken from the
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 3acef5a0ab..eed956851e 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -10,6 +10,7 @@
 #include "diff.h"
 #include "revision.h"
 #include "reachable.h"
+#include "worktree.h"
 
 /* NEEDSWORK: switch to using parse_options */
 static const char reflog_expire_usage[] =
@@ -52,6 +53,7 @@ struct collect_reflog_cb {
 	struct collected_reflog **e;
 	int alloc;
 	int nr;
+	struct worktree *wt;
 };
 
 /* Remember to update object flag allocation in object.h */
@@ -388,8 +390,12 @@ static int collect_reflog(const char *ref, const struct object_id *oid, int unus
 {
 	struct collected_reflog *e;
 	struct collect_reflog_cb *cb = cb_data;
+	struct strbuf newref = STRBUF_INIT;
+
+	strbuf_worktree_ref(cb->wt, &newref, ref);
+	FLEX_ALLOC_STR(e, reflog, newref.buf);
+	strbuf_release(&newref);
 
-	FLEX_ALLOC_STR(e, reflog, ref);
 	oidcpy(&e->oid, oid);
 	ALLOC_GROW(cb->e, cb->nr + 1, cb->alloc);
 	cb->e[cb->nr++] = e;
@@ -512,7 +518,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 {
 	struct expire_reflog_policy_cb cb;
 	timestamp_t now = time(NULL);
-	int i, status, do_all;
+	int i, status, do_all, all_worktrees = 1;
 	int explicit_expiry = 0;
 	unsigned int flags = 0;
 
@@ -549,6 +555,8 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 			flags |= EXPIRE_REFLOGS_UPDATE_REF;
 		else if (!strcmp(arg, "--all"))
 			do_all = 1;
+		else if (!strcmp(arg, "--single-worktree"))
+			all_worktrees = 0;
 		else if (!strcmp(arg, "--verbose"))
 			flags |= EXPIRE_REFLOGS_VERBOSE;
 		else if (!strcmp(arg, "--")) {
@@ -577,10 +585,18 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 
 	if (do_all) {
 		struct collect_reflog_cb collected;
+		struct worktree **worktrees, **p;
 		int i;
 
 		memset(&collected, 0, sizeof(collected));
-		for_each_reflog(collect_reflog, &collected);
+		worktrees = get_worktrees(0);
+		for (p = worktrees; *p; p++) {
+			if (!all_worktrees && !(*p)->is_current)
+				continue;
+			collected.wt = *p;
+			for_each_reflog(collect_reflog, &collected);
+		}
+		free_worktrees(worktrees);
 		for (i = 0; i < collected.nr; i++) {
 			struct collected_reflog *e = collected.e[i];
 			set_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog);
-- 
2.19.0.647.gb9a6049235

Re: [PATCH 2/8] Add a place for (not) sharing stuff between worktrees

[Eric Sunshine]

On Sat, Sep 22, 2018 at 2:05 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> When multiple worktrees are used, we need rules to determine if
> something belongs to one worktree or all of them. Instead of keeping
> adding rules when new stuff comes, have a generic rule:
> [...]
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
> @@ -0,0 +1,36 @@
> +test_expect_success 'setup' '
> +       test_commit initial &&
> +       test_commit wt1 &&
> +       test_commit wt2 &&
> +       git worktree add wt1 wt1 &&
> +       git worktree add wt2 wt2 &&
> +       git checkout initial
> +'
> +
> +test_expect_success 'add refs/local' '
> +       git update-ref refs/local/foo HEAD &&
> +       git -C wt1 update-ref refs/local/foo HEAD &&
> +       git -C wt2 update-ref refs/local/foo HEAD
> +'

Not at all worth a re-roll, but the "add refs/local" test seems like
just more setup, thus could be rolled into the "setup" test (unless it
will be growing in some non-setup way in later patches).

Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

[Eric Sunshine]

On Sat, Sep 22, 2018 at 2:05 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> [...]
> The main worktree has to be treated specially because well.. it's
> special from the beginning. So HEAD from the main worktree is
> acccessible via the name "main/HEAD" (we can't use
> "worktrees/main/HEAD" because "main" under "worktrees" is not
> reserved).

Bikeshedding: I wonder if this would be more intuitive if called
simply "/HEAD" rather than "main/HEAD".

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/refs.c b/refs.c
> @@ -641,12 +641,32 @@ static int is_pseudoref_syntax(const char *refname)
> +static int is_main_pseudoref_syntax(const char *refname)
> +{
> +       return skip_prefix(refname, "main/", &refname) &&
> +               is_pseudoref_syntax(refname);
> +}
> +
> +static int is_other_pseudoref_syntax(const char *refname)
> +{
> +       if (!skip_prefix(refname, "worktrees/", &refname))
> +               return 0;
> +       refname = strchr(refname, '/');
> +       if (!refname)
> +               return 0;
> +       return is_pseudoref_syntax(refname + 1);
> +}

If the input is "worktrees/refs/" (nothing following the trailing
'/'), then an empty string will be passed to is_pseudoref_syntax(),
which will return true. Does that result in correct behavior? (Same
question about "main/" being passed to is_main_pseudoref_syntax().)

Re: [PATCH 5/8] revision.c: better error reporting on ref from different worktrees

[Eric Sunshine]

On Sat, Sep 22, 2018 at 2:05 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Make use of the new ref aliases to pass refs from another worktree
> around and access them from the current ref store instead. This does
> not change any functionality, but when a problem shows up, we would
> report something like

From this description, I had a hard time grasping that the first
example output is the desired one. Not necessarily worth a re-roll,
but had it said instead something like this:

    ...but when a problem arises, we would like the reported
    messages to mention full ref aliases, like this:

it would have been more obvious.

More below...

>     fatal: bad object worktrees/ztemp/HEAD
>     warning: reflog of 'main/HEAD' references pruned commits
>
> instead of
>
>     fatal: bad object HEAD
>     warning: reflog of 'HEAD' references pruned commits
>
> which does not really tell where the refs are from.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/worktree.c b/worktree.c
> @@ -487,6 +487,28 @@ int submodule_uses_worktrees(const char *path)
> +void strbuf_worktree_ref(const struct worktree *wt,
> +                        struct strbuf *sb,
> +                        const char *refname)
> +{
> +       if (wt && !wt->is_current) {
> +               if (is_main_worktree(wt))
> +                       strbuf_addstr(sb, "main/");
> +               else
> +                       strbuf_addf(sb, "worktrees/%s/", wt->id);
> +       }
> +       strbuf_addstr(sb, refname);
> +}

Seeing this use worktree->id to construct "worktrees/<id>/<refname>"
makes me wonder how the user is going to know the ID of a worktree in
order to specify such refs in the first place. For example, how is the
user going to know the <id> in "git rev-parse worktrees/<id>/HEAD"? I
don't think we print the worktree ID anywhere, do we? Certainly, "git
worktree list" doesn't show it, and "git worktree add" stopped showing
it as of 2c27002a0a (worktree: improve message when creating a new
worktree, 2018-04-24).

> diff --git a/worktree.h b/worktree.h
> @@ -108,4 +108,18 @@ extern const char *worktree_git_path(const struct worktree *wt,
> +/*
> + * Return a refname suitable for access from the current ref
> + * store. The result may be destroyed at the next call.
> + */

If you re-roll, perhaps: s/may/will/

Re: [PATCH 7/8] fsck: check HEAD and reflog from other worktrees

[Eric Sunshine]

On Sat, Sep 22, 2018 at 2:05 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> fsck is a repo-wide operation and should check all references no
> matter which worktree they are associated to.
>
> Reported-by: Jeff King <peff@peff.net>
> Helped-by: Elijah Newren <newren@gmail.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
> @@ -101,6 +101,45 @@ test_expect_success 'HEAD link pointing at a funny place' '
> +test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
> +       test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
> +       test_when_finished "rm -rf .git/worktrees wt" &&
> +       git worktree add wt &&
> +       mv .git/HEAD .git/SAVED_HEAD &&
> +       echo 0000000000000000000000000000000000000000 >.git/HEAD &&

Perhaps use $ZERO_OID here instead of hardcoded "0..." string to play
friendly with brian's bc/hash-independent-tests (in 'next'). Same for
following test.

> +       # avoid corrupt/broken HEAD from interfering with repo discovery
> +       test_must_fail git -C wt fsck 2>out &&
> +       cat out &&

Unneeded 'cat'. Debugging cruft? Same for other tests.

> +       grep "main/HEAD: detached HEAD points" out

This message doesn't get localized, so no need for test_i18ngrep(). Okay.

> +'

Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

[Duy Nguyen]

On Sun, Sep 23, 2018 at 10:06 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Sat, Sep 22, 2018 at 2:05 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> > [...]
> > The main worktree has to be treated specially because well.. it's
> > special from the beginning. So HEAD from the main worktree is
> > acccessible via the name "main/HEAD" (we can't use
> > "worktrees/main/HEAD" because "main" under "worktrees" is not
> > reserved).
>
> Bikeshedding: I wonder if this would be more intuitive if called
> simply "/HEAD" rather than "main/HEAD".

A ref name cannot start with '/'. I'm open to a different name than
"main" though, it just felt a better name than "main-worktree".
-- 
Duy

Re: [PATCH 5/8] revision.c: better error reporting on ref from different worktrees

[Duy Nguyen]

On Sun, Sep 23, 2018 at 10:25 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > @@ -487,6 +487,28 @@ int submodule_uses_worktrees(const char *path)
> > +void strbuf_worktree_ref(const struct worktree *wt,
> > +                        struct strbuf *sb,
> > +                        const char *refname)
> > +{
> > +       if (wt && !wt->is_current) {
> > +               if (is_main_worktree(wt))
> > +                       strbuf_addstr(sb, "main/");
> > +               else
> > +                       strbuf_addf(sb, "worktrees/%s/", wt->id);
> > +       }
> > +       strbuf_addstr(sb, refname);
> > +}
>
> Seeing this use worktree->id to construct "worktrees/<id>/<refname>"
> makes me wonder how the user is going to know the ID of a worktree in
> order to specify such refs in the first place. For example, how is the
> user going to know the <id> in "git rev-parse worktrees/<id>/HEAD"? I
> don't think we print the worktree ID anywhere, do we? Certainly, "git
> worktree list" doesn't show it, and "git worktree add" stopped showing
> it as of 2c27002a0a (worktree: improve message when creating a new
> worktree, 2018-04-24).

Oh yes I forgot to point this out. With this approach we have to have
an id, and the directory name inside $GIT_DIR/worktrees seems a
natural choice. I was hoping to go with extended ref syntax instead
[1] which gives us more flexibility in identifying a worktree. But I
don't think that's going to happen. "git worktree" would need some
improvements to show this id, specify it at "git worktree add" and
potentially rename it even.

[1] https://public-inbox.org/git/CACsJy8BOvU_z-uLrFmzFyryMXHNDfc_FUN_4i4NnVXWoShaBLQ@mail.gmail.com/
-- 
Duy

Re: [PATCH 2/8] Add a place for (not) sharing stuff between worktrees

[Stefan Beller]

On Sat, Sep 22, 2018 at 11:05 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>
> When multiple worktrees are used, we need rules to determine if
> something belongs to one worktree or all of them. Instead of keeping
> adding rules when new stuff comes, have a generic rule:
>
> - Inside $GIT_DIR, which is per-worktree by default, add
>   $GIT_DIR/common which is always shared. New features that want to
>   share stuff should put stuff under this directory.

So that /common is a directory and you have to use it specifically
in new code? That would be easy to overlook when coming up
with $GIT_DIR/foo for implementing the git-foo.

>
> - Inside refs/, which is shared by default except refs/bisect, add
>   refs/local/ which is per-worktree. We may eventually move
>   refs/bisect to this new location and remove the exception in refs
>   code.

That sounds dangerous to me. There is already a concept of
local and remote-tracking branches. So I would think that local
may soon become an overused word, (just like "index" today or
"recursive" to a lesser extend).

Could this special area be more explicit?
(refs/worktree-local/ ? or after peeking at the docs below
refs/un-common/ ?)

Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

[Stefan Beller]

On Sat, Sep 22, 2018 at 11:05 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>
> One of the problems with multiple worktree is accessing per-worktree
> refs of one worktree from another worktree. This was sort of solved by
> multiple ref store, where the code can open the ref store of another
> worktree and has access to the ref space of that worktree.
>
> The problem with this is reporting. "HEAD" in another ref space is
> also called "HEAD" like in the current ref space. In order to
> differentiate them, all the code must somehow carry the ref store
> around and print something like "HEAD from this ref store".
>
> But that is not feasible (or possible with a _lot_ of work). With the
> current design, we pass a reference around as a string (so called
> "refname"). Extending this design to pass a string _and_ a ref store
> is a nightmare, especially when handling extended SHA-1 syntax.
>
> So we do it another way. Instead of entering a separate ref space, we
> make refs from other worktrees available in the current ref space. So
> "HEAD" is always HEAD of the current worktree, but then we can have
> "worktrees/blah/HEAD" to denote HEAD from a worktree named
> "blah". This syntax coincidentally matches the underlying directory
> structure which makes implementation a bit easier.
>
> The main worktree has to be treated specially because well.. it's
> special from the beginning. So HEAD from the main worktree is
> acccessible via the name "main/HEAD" (we can't use
> "worktrees/main/HEAD" because "main" under "worktrees" is not
> reserved).
>
> This patch also makes it possible to specify refs from one worktree in
> another one, e.g.
>
>     git log worktrees/foo/HEAD

This has strong similarities to remote refs:
Locally I may have a branch master, whose (stale local copy of its
distributed) counterpart is named origin/master.

It is also possible to have a working tree named origin
(just I like to name my worktree "git", when working on git.git),
how do we differentiate between the neighbor-worktree
"origin/master" and the remote-tracking branch "origin/master" ?

As the remote tracking branches are shared between all
worktree there is no need to differentiate between a
local-worktree remote tracking branch and a
neighbor-worktree remote tracking branch.

Now that you introduce the magic main working tree,
we also need to disallow working trees to be named "main",
i.e.
    $ git worktree add main HEAD

produces

  $ ls .git/worktrees/
  main

How do we deal with that?

Re: [PATCH 2/8] Add a place for (not) sharing stuff between worktrees

[Duy Nguyen]

On Tue, Sep 25, 2018 at 4:35 AM Stefan Beller <sbeller@google.com> wrote:
>
> On Sat, Sep 22, 2018 at 11:05 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> >
> > When multiple worktrees are used, we need rules to determine if
> > something belongs to one worktree or all of them. Instead of keeping
> > adding rules when new stuff comes, have a generic rule:
> >
> > - Inside $GIT_DIR, which is per-worktree by default, add
> >   $GIT_DIR/common which is always shared. New features that want to
> >   share stuff should put stuff under this directory.
>
> So that /common is a directory and you have to use it specifically
> in new code? That would be easy to overlook when coming up
> with $GIT_DIR/foo for implementing the git-foo.

There's no easy way out. I have to do _something_ if you want to share
$GIT_DIR/foo to all worktrees. Either we have to update path.c and add
"foo" which is not even an option for external commands, or we put
"foo" in a common place, e.g. $GIT_DIR/common/foo.

> > - Inside refs/, which is shared by default except refs/bisect, add
> >   refs/local/ which is per-worktree. We may eventually move
> >   refs/bisect to this new location and remove the exception in refs
> >   code.
>
> That sounds dangerous to me. There is already a concept of
> local and remote-tracking branches. So I would think that local
> may soon become an overused word, (just like "index" today or
> "recursive" to a lesser extend).
>
> Could this special area be more explicit?
> (refs/worktree-local/ ? or after peeking at the docs below
> refs/un-common/ ?)

refs/un-common sounds really "uncommon" :D. If refs/local is bad, I
guess we could go with either refs/worktree-local, refs/worktree,
refs/private, refs/per-worktree... My vote is on refs/worktree. I
think as long as the word "worktree" is in there, people would notice
the difference.
-- 
Duy

Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

[Duy Nguyen]

On Tue, Sep 25, 2018 at 4:48 AM Stefan Beller <sbeller@google.com> wrote:
> > This patch also makes it possible to specify refs from one worktree in
> > another one, e.g.
> >
> >     git log worktrees/foo/HEAD
>
> This has strong similarities to remote refs:
> Locally I may have a branch master, whose (stale local copy of its
> distributed) counterpart is named origin/master.

If you think of each worktree as independent clones (which is more or
less true, the fact that they share ODB is more like an implementation
detail) then yes it's almost like remotes.

> It is also possible to have a working tree named origin
> (just I like to name my worktree "git", when working on git.git),
> how do we differentiate between the neighbor-worktree
> "origin/master" and the remote-tracking branch "origin/master" ?

Hmm.. I think you're thinking that origin/master could either mean
refs/worktrees/origin/master or refs/remotes/origin/master. I do not
think we're going to support expanding origin/master to
refs/worktrees/origin/master. This part about ref resolution did cross
my mind but I didn't see a good reason to support it.

Even if we do support it, this is not even a new problem. If you have
refs/heads/origin/master and refs/remotes/origin/master now, we have
ref ambiguity anyway and a solution for this should handle
refs/worktrees/origin/master well if it comes into the picture.

> As the remote tracking branches are shared between all
> worktree there is no need to differentiate between a
> local-worktree remote tracking branch and a
> neighbor-worktree remote tracking branch.
>
> Now that you introduce the magic main working tree,
> we also need to disallow working trees to be named "main",
> i.e.
>     $ git worktree add main HEAD
>
> produces
>
>   $ ls .git/worktrees/
>   main
>
> How do we deal with that?

main is accessed via worktrees/main/HEAD while the main worktree's
HEAD is accessed via main/HEAD (which is _not_ automatically expanded
to refs/worktrees/main/HEAD). But if it is, yes we need to detect
ambiguity and tell the user to specify full ref name, either
refs/main/HEAD or refs/worktrees/main/HEAD.
-- 
Duy

Re: [PATCH 2/8] Add a place for (not) sharing stuff between worktrees

[Stefan Beller]

On Tue, Sep 25, 2018 at 8:36 AM Duy Nguyen <pclouds@gmail.com> wrote:
>
> On Tue, Sep 25, 2018 at 4:35 AM Stefan Beller <sbeller@google.com> wrote:
> >
> > On Sat, Sep 22, 2018 at 11:05 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> > >
> > > When multiple worktrees are used, we need rules to determine if
> > > something belongs to one worktree or all of them. Instead of keeping
> > > adding rules when new stuff comes, have a generic rule:
> > >
> > > - Inside $GIT_DIR, which is per-worktree by default, add
> > >   $GIT_DIR/common which is always shared. New features that want to
> > >   share stuff should put stuff under this directory.
> >
> > So that /common is a directory and you have to use it specifically
> > in new code? That would be easy to overlook when coming up
> > with $GIT_DIR/foo for implementing the git-foo.
>
> There's no easy way out. I have to do _something_ if you want to share
> $GIT_DIR/foo to all worktrees. Either we have to update path.c and add
> "foo" which is not even an option for external commands, or we put
> "foo" in a common place, e.g. $GIT_DIR/common/foo.
>
> > > - Inside refs/, which is shared by default except refs/bisect, add
> > >   refs/local/ which is per-worktree. We may eventually move
> > >   refs/bisect to this new location and remove the exception in refs
> > >   code.
> >
> > That sounds dangerous to me. There is already a concept of
> > local and remote-tracking branches. So I would think that local
> > may soon become an overused word, (just like "index" today or
> > "recursive" to a lesser extend).
> >
> > Could this special area be more explicit?
> > (refs/worktree-local/ ? or after peeking at the docs below
> > refs/un-common/ ?)
>
> refs/un-common sounds really "uncommon" :D. If refs/local is bad, I
> guess we could go with either refs/worktree-local, refs/worktree,
> refs/private, refs/per-worktree... My vote is on refs/worktree. I

refs/worktree sounds good to me (I do not object), but I am not
overly enthused either, as when I think further worktrees and
submodules are both features with a very similar nature in that
they touch a lot of core concepts in Git, but seem to be a niche
feature for the masses for now.

For example I could think of submodules following this addressing
mode as well: submodule/<path>/master sounds similar to the
originally proposed worktree/<name>/<branch> convention.
For now it is not quite clear to me why you would want to have
access to the submodule refs in the superproject, but maybe
the use case will come later.

And with that said, I wonder if the "local" part should be feature agnostic,
or if we want to be "local" for worktrees, "local" for remotes, "local"
for submodules (i.e. our own refs vs submodule refs).

> think as long as the word "worktree" is in there, people would notice
> the difference.

That makes sense. But is refs/worktree shared or local? It's not quite
obvious to me, as I could have refs/worktree/<worktree-name>/master
instead when it is shared, so I tend to favor refs/local-worktree/ a bit
more, but that is more typing. :/

==
As we grow the worktree feature, do we ever expect the need to
reference the current worktree?

For example when there is a ref "test" that could be unique per
repo and in the common area, so refs/heads/test would describe
it and "test" would get there in DWIM mode.

But then I could also delete the common ref and recreate a "test"
ref in worktree A, in worktree B however DWIMming "test" could still
refer to A's "test" as it is unique (so far) in the repository.
And maybe I would want to check if test exists locally, so I'd
want to ask for "self/test" (with "self" == "B" as that is my cwd).

Stefan

Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

[Stefan Beller]

On Tue, Sep 25, 2018 at 8:49 AM Duy Nguyen <pclouds@gmail.com> wrote:
>
> On Tue, Sep 25, 2018 at 4:48 AM Stefan Beller <sbeller@google.com> wrote:
> > > This patch also makes it possible to specify refs from one worktree in
> > > another one, e.g.
> > >
> > >     git log worktrees/foo/HEAD
> >
> > This has strong similarities to remote refs:
> > Locally I may have a branch master, whose (stale local copy of its
> > distributed) counterpart is named origin/master.
>
> If you think of each worktree as independent clones (which is more or
> less true, the fact that they share ODB is more like an implementation
> detail) then yes it's almost like remotes.

Apart from the ODB and the refs subsystem, there is also the config
space, which is shared (but you have sent out patches to have local
config as well).

So I would think worktrees are better than having two clones not just
due to the shared ODB, but also due to the common config as then I
have to setup my repo only once and can add/remove worktrees
cheaply (in terms of "how much time do I need to spend to configure
it as I need").

> > It is also possible to have a working tree named origin
> > (just I like to name my worktree "git", when working on git.git),
> > how do we differentiate between the neighbor-worktree
> > "origin/master" and the remote-tracking branch "origin/master" ?
>
> Hmm.. I think you're thinking that origin/master could either mean
> refs/worktrees/origin/master or refs/remotes/origin/master. I do not
> think we're going to support expanding origin/master to
> refs/worktrees/origin/master. This part about ref resolution did cross
> my mind but I didn't see a good reason to support it.
>
> Even if we do support it, this is not even a new problem. If you have
> refs/heads/origin/master and refs/remotes/origin/master now, we have
> ref ambiguity anyway and a solution for this should handle
> refs/worktrees/origin/master well if it comes into the picture.

So once origin/master is overloaded, I would have to spell out
refs/worktrees/origin/master and refs/remotes/origin/master to
avoid confusing the DWIM machinery. Makes sense.

> > How do we deal with that?
>
> main is accessed via worktrees/main/HEAD while the main worktree's
> HEAD is accessed via main/HEAD (which is _not_ automatically expanded
> to refs/worktrees/main/HEAD). But if it is, yes we need to detect
> ambiguity and tell the user to specify full ref name, either
> refs/main/HEAD or refs/worktrees/main/HEAD.

Ah, I see. Now I actually understand the last paragraph of the
commit message. Thanks for explaining!

Stefan

Re: [PATCH 2/8] Add a place for (not) sharing stuff between worktrees

[Duy Nguyen]

On Tue, Sep 25, 2018 at 6:24 PM Stefan Beller <sbeller@google.com> wrote:
> > > That sounds dangerous to me. There is already a concept of
> > > local and remote-tracking branches. So I would think that local
> > > may soon become an overused word, (just like "index" today or
> > > "recursive" to a lesser extend).
> > >
> > > Could this special area be more explicit?
> > > (refs/worktree-local/ ? or after peeking at the docs below
> > > refs/un-common/ ?)
> >
> > refs/un-common sounds really "uncommon" :D. If refs/local is bad, I
> > guess we could go with either refs/worktree-local, refs/worktree,
> > refs/private, refs/per-worktree... My vote is on refs/worktree. I
>
> refs/worktree sounds good to me (I do not object), but I am not
> overly enthused either, as when I think further worktrees and
> submodules are both features with a very similar nature in that
> they touch a lot of core concepts in Git, but seem to be a niche
> feature for the masses for now.

I think the similarity is partly because submodule feature also has to
manage worktrees. My view is at some point, this "git worktree" would
be good enough that it can handle submodules as well (for the worktree
part only of course)

> For example I could think of submodules following this addressing
> mode as well: submodule/<path>/master sounds similar to the
> originally proposed worktree/<name>/<branch> convention.
> For now it is not quite clear to me why you would want to have
> access to the submodule refs in the superproject, but maybe
> the use case will come later.

Yeah. In theory we could "mount" the submodule ref store to a
superproject's ref store. I think it may be needed just for the same
reason it's needed for worktree: error reporting. If you peek into a
submodule and say "HEAD has an error", the user will get confused
whether it's superproject's HEAD or a submodule's HEAD.

> And with that said, I wonder if the "local" part should be feature agnostic,
> or if we want to be "local" for worktrees, "local" for remotes, "local"
> for submodules (i.e. our own refs vs submodule refs).

You lost me here.

>
> > think as long as the word "worktree" is in there, people would notice
> > the difference.
>
> That makes sense. But is refs/worktree shared or local? It's not quite
> obvious to me, as I could have refs/worktree/<worktree-name>/master
> instead when it is shared, so I tend to favor refs/local-worktree/ a bit
> more, but that is more typing. :/

OK I think mixing the two patches will different purposes messes you
(or me) up ;-)

refs/worktrees/xxx (and refs/main/xxx) are about visibility from other
worktrees. Or like Eric put it, they are simply aliases. These refs
are not shared because if they are, you can already see them without
new "ref mount points" like this.

refs/worktree (previously refs/local) is also per-worktree but it's
specifically because you can't have per-worktree inside "refs/" (the
only exception so far is refs/bisect which is hard coded). You can
have refs outside "refs/" (like HEAD or FETCH_HEAD) and they will not
be shared, but they cannot be iterated while those inside refs/ can
be. This is more about deciding what to share and I believe is really
worktree-specific and only matters to _current_ worktree.

Since refs/worktree is per-worktree, you can also view them from a
different worktree via refs/worktrees/. E.g. if you have
refs/worktree/foo then another worktree can see it via
refs/worktrees/xxx/refs/worktree/foo (besides pseudo refs like
refs/worktrees/xxx/HEAD)

> As we grow the worktree feature, do we ever expect the need to
> reference the current worktree?
>
> For example when there is a ref "test" that could be unique per
> repo and in the common area, so refs/heads/test would describe
> it and "test" would get there in DWIM mode.
>
> But then I could also delete the common ref and recreate a "test"
> ref in worktree A, in worktree B however DWIMming "test" could still
> refer to A's "test" as it is unique (so far) in the repository.
> And maybe I would want to check if test exists locally, so I'd
> want to ask for "self/test" (with "self" == "B" as that is my cwd).

You probably lost me again. In theory we must be able to detect
ambiguity and stop DWIMing. If you want to be ambiguity-free, you
specify full ref name, starting with "refs/" which should function
like "self/" because worktree design so far is always about the
current worktree's view.
-- 
Duy

Re: [PATCH 2/8] Add a place for (not) sharing stuff between worktrees

[Stefan Beller]

On Tue, Sep 25, 2018 at 9:55 AM Duy Nguyen <pclouds@gmail.com> wrote:

> > And with that said, I wonder if the "local" part should be feature agnostic,
> > or if we want to be "local" for worktrees, "local" for remotes, "local"
> > for submodules (i.e. our own refs vs submodule refs).
>
> You lost me here.

Yeah, me too after rereading. :P

I think the "local" part always implies that there is a part that is
not local and depending on the feature you call it remote or other
worktree.

When writing this comment I briefly wondered if we want to combine
the local aspects of the various features.
However the "local" part really depends on the feature
(e.g. a ref on a different worktree is still local from the here/remote
perspective or from the superproject/submodule perspective),
so I think I was misguided.

> > > think as long as the word "worktree" is in there, people would notice
> > > the difference.
> >
> > That makes sense. But is refs/worktree shared or local? It's not quite
> > obvious to me, as I could have refs/worktree/<worktree-name>/master
> > instead when it is shared, so I tend to favor refs/local-worktree/ a bit
> > more, but that is more typing. :/
>
> OK I think mixing the two patches will different purposes messes you
> (or me) up ;-)

possible.

>
> refs/worktrees/xxx (and refs/main/xxx) are about visibility from other
> worktrees. Or like Eric put it, they are simply aliases. These refs
> are not shared because if they are, you can already see them without
> new "ref mount points" like this.
>
> refs/worktree (previously refs/local) is also per-worktree but it's
> specifically because you can't have per-worktree inside "refs/" (the
> only exception so far is refs/bisect which is hard coded). You can
> have refs outside "refs/" (like HEAD or FETCH_HEAD) and they will not
> be shared, but they cannot be iterated while those inside refs/ can
> be. This is more about deciding what to share and I believe is really
> worktree-specific and only matters to _current_ worktree.
>
> Since refs/worktree is per-worktree, you can also view them from a
> different worktree via refs/worktrees/. E.g. if you have
> refs/worktree/foo then another worktree can see it via
> refs/worktrees/xxx/refs/worktree/foo (besides pseudo refs like
> refs/worktrees/xxx/HEAD)

Ah. now I seem to understand, thanks for explaining.

Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> The main worktree has to be treated specially because well.. it's
> special from the beginning. So HEAD from the main worktree is
> acccessible via the name "main/HEAD" (we can't use
> "worktrees/main/HEAD" because "main" under "worktrees" is not
> reserved).

I do not quite follow.  So with this, both refs/heads/master and
main/refs/heads/master are good names for the master branch (even
though the local branch names are not per worktree), because
in the main worktree, refs/bisect/bad and main/refs/bisect/bad ought
to mean the same thing.

	side note: Or is this only for pseudo-refs
	(i.e. $GIT_DIR/$name where $name consists of all caps or
	underscore and typically ends with HEAD)?  Even if that were
	the case, I do not think it essentially changes the issue
	around disambiguation that much.

The disambiguation rule has always been: if you have a confusingly
named ref, you can spell it out fully to avoid any ambiguity, e.g.
refs/heads/refs/heads/foo can be given to "git rev-parse" and will
mean the tip of the branch whose name is "refs/heads/foo", even when
another branch whose name is "foo" exists.

Would we have a reasonable disambiguation rules that work well with
the main/ and worktrees/* prefixes?  When somebody has main/HEAD branch
and writes "git rev-parse main/HEAD", does it find refs/heads/main/HEAD
or $GIT_DIR/HEAD, if the user is in the main worktree?

It could be simply that the design is underdocumented in this patch
set (in which case I would have appreciated 'RFC' near 'PATCH'), but
I have a feeling that the code came way too early before such design
issues are fleshed out.

> diff --git a/refs.h b/refs.h
> index bd52c1bbae..9b53dbeae8 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -704,9 +704,11 @@ int parse_hide_refs_config(const char *var, const char *value, const char *);
>  int ref_is_hidden(const char *, const char *);
>  
>  enum ref_type {
> -	REF_TYPE_PER_WORKTREE,
> -	REF_TYPE_PSEUDOREF,
> -	REF_TYPE_NORMAL,
> +	REF_TYPE_PER_WORKTREE,	  /* refs inside refs/ but not shared       */
> +	REF_TYPE_PSEUDOREF,	  /* refs outside refs/ in current worktree */
> +	REF_TYPE_MAIN_PSEUDOREF,  /* pseudo refs from the main worktree     */
> +	REF_TYPE_OTHER_PSEUDOREF, /* pseudo refs from other worktrees       */
> +	REF_TYPE_NORMAL,	  /* normal/shared refs inside refs/        */
>  };
>  
>  enum ref_type ref_type(const char *refname);
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 416eafa453..bf9ed633b1 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -149,6 +149,23 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store,
>  	return refs;
>  }
>  
> +static void files_reflog_path_other_worktrees(struct files_ref_store *refs,
> +					      struct strbuf *sb,
> +					      const char *refname)
> +{
> +	const char *real_ref;
> +
> +	if (!skip_prefix(refname, "worktrees/", &real_ref))
> +		BUG("refname %s is not a other-worktree ref", refname);
> +	real_ref = strchr(real_ref, '/');
> +	if (!real_ref)
> +		BUG("refname %s is not a other-worktree ref", refname);
> +	real_ref++;
> +
> +	strbuf_addf(sb, "%s/%.*slogs/%s", refs->gitcommondir,
> +		    (int)(real_ref - refname), refname, real_ref);
> +}
> +
>  static void files_reflog_path(struct files_ref_store *refs,
>  			      struct strbuf *sb,
>  			      const char *refname)
> @@ -158,6 +175,12 @@ static void files_reflog_path(struct files_ref_store *refs,
>  	case REF_TYPE_PSEUDOREF:
>  		strbuf_addf(sb, "%s/logs/%s", refs->gitdir, refname);
>  		break;
> +	case REF_TYPE_OTHER_PSEUDOREF:
> +		return files_reflog_path_other_worktrees(refs, sb, refname);
> +	case REF_TYPE_MAIN_PSEUDOREF:
> +		if (!skip_prefix(refname, "main/", &refname))
> +			BUG("ref %s is not a main pseudoref", refname);
> +		/* passthru */
>  	case REF_TYPE_NORMAL:
>  		strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir, refname);
>  		break;
> @@ -176,6 +199,11 @@ static void files_ref_path(struct files_ref_store *refs,
>  	case REF_TYPE_PSEUDOREF:
>  		strbuf_addf(sb, "%s/%s", refs->gitdir, refname);
>  		break;
> +	case REF_TYPE_MAIN_PSEUDOREF:
> +		if (!skip_prefix(refname, "main/", &refname))
> +			BUG("ref %s is not a main pseudoref", refname);
> +		/* passthru */
> +	case REF_TYPE_OTHER_PSEUDOREF:
>  	case REF_TYPE_NORMAL:
>  		strbuf_addf(sb, "%s/%s", refs->gitcommondir, refname);
>  		break;
> diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
> index 0c2d5f89a9..46ca7bfc19 100755
> --- a/t/t1415-worktree-refs.sh
> +++ b/t/t1415-worktree-refs.sh
> @@ -33,4 +33,34 @@ test_expect_success 'refs/local are per-worktree' '
>  	( cd wt2 && test_cmp_rev local/foo wt2 )
>  '
>  
> +test_expect_success 'resolve main/HEAD' '
> +	test_cmp_rev main/HEAD initial &&
> +	( cd wt1 && test_cmp_rev main/HEAD initial ) &&
> +	( cd wt2 && test_cmp_rev main/HEAD initial )
> +'
> +
> +test_expect_success 'resolve worktrees/xx/HEAD' '
> +	test_cmp_rev worktrees/wt1/HEAD wt1 &&
> +	( cd wt1 && test_cmp_rev worktrees/wt1/HEAD wt1 ) &&
> +	( cd wt2 && test_cmp_rev worktrees/wt1/HEAD wt1 )
> +'
> +
> +test_expect_success 'reflog of main/HEAD' '
> +	git reflog HEAD | sed "s/HEAD/main\/HEAD/" >expected &&
> +	git reflog main/HEAD >actual &&
> +	test_cmp expected actual &&
> +	git -C wt1 reflog main/HEAD >actual.wt1 &&
> +	test_cmp expected actual.wt1
> +'
> +
> +test_expect_success 'reflog of worktrees/xx/HEAD' '
> +	git -C wt2 reflog HEAD | sed "s/HEAD/worktrees\/wt2\/HEAD/" >expected &&
> +	git reflog worktrees/wt2/HEAD >actual &&
> +	test_cmp expected actual &&
> +	git -C wt1 reflog worktrees/wt2/HEAD >actual.wt1 &&
> +	test_cmp expected actual.wt1 &&
> +	git -C wt2 reflog worktrees/wt2/HEAD >actual.wt2 &&
> +	test_cmp expected actual.wt2
> +'
> +
>  test_done

Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

[Duy Nguyen]

On Tue, Sep 25, 2018 at 11:16 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
> > The main worktree has to be treated specially because well.. it's
> > special from the beginning. So HEAD from the main worktree is
> > acccessible via the name "main/HEAD" (we can't use
> > "worktrees/main/HEAD" because "main" under "worktrees" is not
> > reserved).
>
> I do not quite follow.  So with this, both refs/heads/master and
> main/refs/heads/master are good names for the master branch (even
> though the local branch names are not per worktree), because
> in the main worktree, refs/bisect/bad and main/refs/bisect/bad ought
> to mean the same thing.

True. I think the ambiguation here is about the main worktree versus a
secondary worktree that is accidentally named "main". Then suddenly we
have to worktrees of the same name, and accessing them both via
worktrees/<id>/HEAD will not work, and there is no other way to
disambiguate them.

>         side note: Or is this only for pseudo-refs
>         (i.e. $GIT_DIR/$name where $name consists of all caps or
>         underscore and typically ends with HEAD)?

Right now, due to implementation limitations, only pseudo refs (or
loose refs in the case of refs/bisect) are accessible. But I don't see
why main/refs/heads/master should not work.

> The disambiguation rule has always been: if you have a confusingly
> named ref, you can spell it out fully to avoid any ambiguity, e.g.
> refs/heads/refs/heads/foo can be given to "git rev-parse" and will
> mean the tip of the branch whose name is "refs/heads/foo", even when
> another branch whose name is "foo" exists.
>
> Would we have a reasonable disambiguation rules that work well with
> the main/ and worktrees/* prefixes?  When somebody has main/HEAD branch
> and writes "git rev-parse main/HEAD", does it find refs/heads/main/HEAD
> or $GIT_DIR/HEAD, if the user is in the main worktree?

The rules are not touched. But it looks like everything still works as
expected (I'm adding tests to verify this)
-- 
Duy

Re: [PATCH 7/8] fsck: check HEAD and reflog from other worktrees

[Duy Nguyen]

On Sun, Sep 23, 2018 at 04:41:32AM -0400, Eric Sunshine wrote:
> > +       grep "main/HEAD: detached HEAD points" out
> 
> This message doesn't get localized, so no need for
> test_i18ngrep(). Okay.

Don't remind me. I started to mark all strings in fsck for translation
and faced the reality that I would need to change a lot to
test_i18ngrep :(
--
Duy

[PATCH v2 0/8] fix per-worktree ref iteration in fsck/reflog expire

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

v2 changes

- more documentation
- main/ prefix is renamed to main-worktree/ to reduce ambiguation
  chances and make it clearer
- refs/local is renamed to refs/worktree
- bug fix in is_main_pseudoref_syntax() and
  is_other_pseudoref_syntax()

Interdiff

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e2ee9fc21b..58415f9207 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -204,6 +204,35 @@ working trees, it can be used to identify worktrees. For example if
 you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
 then "ghi" or "def/ghi" is enough to point to the former working tree.
 
+REFS
+----
+In multiple working trees, some refs may be shared between all working
+trees, some refs are local. One example is HEAD is different for all
+working trees. This section is about the sharing rules and how to access
+refs of one working tree from another.
+
+In general, all pseudo refs are per working tree and all refs starting
+with "refs/" are shared. Pseudo refs are ones like HEAD which are
+directly under GIT_DIR instead of inside GIT_DIR/refs. There are one
+exception to this: refs inside refs/bisect and refs/worktree is not
+shared.
+
+Refs that are per working tree can still be accessed from another
+working tree via two special paths main-worktree and worktrees. The
+former gives access to per-worktree refs of the main working tree,
+while the former to all linked working trees.
+
+For example, main-worktree/HEAD or main-worktree/refs/bisect/good
+resolve to the same value as the main working tree's HEAD and
+refs/bisect/good respectively. Similarly, worktrees/foo/HEAD or
+worktrees/bar/refs/bisect/bad are the same as
+GIT_COMMON_DIR/worktrees/foo/HEAD and
+GIT_COMMON_DIR/worktrees/bar/refs/bisect/bad.
+
+To access refs, it's best not to look inside GIT_DIR directly. Instead
+use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1]
+which will handle refs correctly.
+
 DETAILS
 -------
 Each linked working tree has a private sub-directory in the repository's
@@ -228,7 +257,8 @@ linked working tree `git rev-parse --git-path HEAD` returns
 `/path/other/test-next/.git/HEAD` or `/path/main/.git/HEAD`) while `git
 rev-parse --git-path refs/heads/master` uses
 $GIT_COMMON_DIR and returns `/path/main/.git/refs/heads/master`,
-since refs are shared across all working trees.
+since refs are shared across all working trees, except refs/bisect and
+refs/worktree.
 
 See linkgit:gitrepository-layout[5] for more information. The rule of
 thumb is do not make any assumption about whether a path belongs to
diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index fad404ed7c..89b616e049 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -96,9 +96,9 @@ refs::
 	directory.  The 'git prune' command knows to preserve
 	objects reachable from refs found in this directory and
 	its subdirectories.
-	This directory is ignored (except refs/bisect and refs/local)
-	if $GIT_COMMON_DIR is set and "$GIT_COMMON_DIR/refs" will be
-	used instead.
+	This directory is ignored (except refs/bisect and
+	refs/worktree) if $GIT_COMMON_DIR is set and
+	"$GIT_COMMON_DIR/refs" will be used instead.
 
 refs/heads/`name`::
 	records tip-of-the-tree commit objects of branch `name`
diff --git a/path.c b/path.c
index 7eb61bf31b..bf4bb02a27 100644
--- a/path.c
+++ b/path.c
@@ -119,6 +119,7 @@ static struct common_dir common_list[] = {
 	{ 0, 1, 0, "objects" },
 	{ 0, 1, 0, "refs" },
 	{ 0, 1, 1, "refs/bisect" },
+	{ 0, 1, 1, "refs/worktree" },
 	{ 0, 1, 0, "remotes" },
 	{ 0, 1, 0, "worktrees" },
 	{ 0, 1, 0, "rr-cache" },
diff --git a/refs.c b/refs.c
index 90b73c7334..2378b2e7fc 100644
--- a/refs.c
+++ b/refs.c
@@ -624,7 +624,7 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)
 static int is_per_worktree_ref(const char *refname)
 {
 	return !strcmp(refname, "HEAD") ||
-		starts_with(refname, "refs/local/") ||
+		starts_with(refname, "refs/worktree/") ||
 		starts_with(refname, "refs/bisect/") ||
 		starts_with(refname, "refs/rewritten/");
 }
@@ -643,7 +643,8 @@ static int is_pseudoref_syntax(const char *refname)
 
 static int is_main_pseudoref_syntax(const char *refname)
 {
-	return skip_prefix(refname, "main/", &refname) &&
+	return skip_prefix(refname, "main-worktree/", &refname) &&
+		*refname &&
 		is_pseudoref_syntax(refname);
 }
 
@@ -652,7 +653,7 @@ static int is_other_pseudoref_syntax(const char *refname)
 	if (!skip_prefix(refname, "worktrees/", &refname))
 		return 0;
 	refname = strchr(refname, '/');
-	if (!refname)
+	if (!refname || !refname[1])
 		return 0;
 	return is_pseudoref_syntax(refname + 1);
 }
diff --git a/refs/files-backend.c b/refs/files-backend.c
index bf9ed633b1..9ca2a3706c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -178,7 +178,7 @@ static void files_reflog_path(struct files_ref_store *refs,
 	case REF_TYPE_OTHER_PSEUDOREF:
 		return files_reflog_path_other_worktrees(refs, sb, refname);
 	case REF_TYPE_MAIN_PSEUDOREF:
-		if (!skip_prefix(refname, "main/", &refname))
+		if (!skip_prefix(refname, "main-worktree/", &refname))
 			BUG("ref %s is not a main pseudoref", refname);
 		/* passthru */
 	case REF_TYPE_NORMAL:
@@ -200,7 +200,7 @@ static void files_ref_path(struct files_ref_store *refs,
 		strbuf_addf(sb, "%s/%s", refs->gitdir, refname);
 		break;
 	case REF_TYPE_MAIN_PSEUDOREF:
-		if (!skip_prefix(refname, "main/", &refname))
+		if (!skip_prefix(refname, "main-worktree/", &refname))
 			BUG("ref %s is not a main pseudoref", refname);
 		/* passthru */
 	case REF_TYPE_OTHER_PSEUDOREF:
@@ -297,7 +297,7 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
 	closedir(d);
 
 	/*
-	 * Manually add refs/bisect and refs/local, which, being
+	 * Manually add refs/bisect and refs/worktree, which, being
 	 * per-worktree, might not appear in the directory listing for
 	 * refs/ in the main repo.
 	 */
@@ -310,11 +310,11 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
 			add_entry_to_dir(dir, child_entry);
 		}
 
-		pos = search_ref_dir(dir, "refs/local/", 11);
+		pos = search_ref_dir(dir, "refs/worktree/", 11);
 
 		if (pos < 0) {
 			struct ref_entry *child_entry = create_dir_entry(
-					dir->cache, "refs/local/", 11, 1);
+					dir->cache, "refs/worktree/", 11, 1);
 			add_entry_to_dir(dir, child_entry);
 		}
 	}
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
index 46ca7bfc19..8b701d07af 100755
--- a/t/t1415-worktree-refs.sh
+++ b/t/t1415-worktree-refs.sh
@@ -10,33 +10,38 @@ test_expect_success 'setup' '
 	test_commit wt2 &&
 	git worktree add wt1 wt1 &&
 	git worktree add wt2 wt2 &&
-	git checkout initial
+	git checkout initial &&
+	git update-ref refs/worktree/foo HEAD &&
+	git -C wt1 update-ref refs/worktree/foo HEAD &&
+	git -C wt2 update-ref refs/worktree/foo HEAD
 '
 
-test_expect_success 'add refs/local' '
-	git update-ref refs/local/foo HEAD &&
-	git -C wt1 update-ref refs/local/foo HEAD &&
-	git -C wt2 update-ref refs/local/foo HEAD
-'
-
-test_expect_success 'refs/local must not be packed' '
+test_expect_success 'refs/worktree must not be packed' '
 	git pack-refs --all &&
 	test_path_is_missing .git/refs/tags/wt1 &&
-	test_path_is_file .git/refs/local/foo &&
-	test_path_is_file .git/worktrees/wt1/refs/local/foo &&
-	test_path_is_file .git/worktrees/wt2/refs/local/foo
+	test_path_is_file .git/refs/worktree/foo &&
+	test_path_is_file .git/worktrees/wt1/refs/worktree/foo &&
+	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
 '
 
-test_expect_success 'refs/local are per-worktree' '
-	test_cmp_rev local/foo initial &&
-	( cd wt1 && test_cmp_rev local/foo wt1 ) &&
-	( cd wt2 && test_cmp_rev local/foo wt2 )
+test_expect_success 'refs/worktree are per-worktree' '
+	test_cmp_rev worktree/foo initial &&
+	( cd wt1 && test_cmp_rev worktree/foo wt1 ) &&
+	( cd wt2 && test_cmp_rev worktree/foo wt2 )
 '
 
-test_expect_success 'resolve main/HEAD' '
-	test_cmp_rev main/HEAD initial &&
-	( cd wt1 && test_cmp_rev main/HEAD initial ) &&
-	( cd wt2 && test_cmp_rev main/HEAD initial )
+test_expect_success 'resolve main-worktree/HEAD' '
+	test_cmp_rev main-worktree/HEAD initial &&
+	( cd wt1 && test_cmp_rev main-worktree/HEAD initial ) &&
+	( cd wt2 && test_cmp_rev main-worktree/HEAD initial )
+'
+
+test_expect_success 'ambiguous main-worktree/HEAD' '
+	mkdir -p .git/refs/heads/main-worktree &&
+	test_when_finished rm .git/refs/heads/main-worktree/HEAD &&
+	cp .git/HEAD .git/refs/heads/main-worktree/HEAD &&
+	git rev-parse main-worktree/HEAD 2>warn >/dev/null &&
+	grep "main-worktree/HEAD.*ambiguous" warn
 '
 
 test_expect_success 'resolve worktrees/xx/HEAD' '
@@ -45,11 +50,19 @@ test_expect_success 'resolve worktrees/xx/HEAD' '
 	( cd wt2 && test_cmp_rev worktrees/wt1/HEAD wt1 )
 '
 
-test_expect_success 'reflog of main/HEAD' '
-	git reflog HEAD | sed "s/HEAD/main\/HEAD/" >expected &&
-	git reflog main/HEAD >actual &&
+test_expect_success 'ambiguous worktrees/xx/HEAD' '
+	mkdir -p .git/refs/heads/worktrees/wt1 &&
+	test_when_finished rm .git/refs/heads/worktrees/wt1/HEAD &&
+	cp .git/HEAD .git/refs/heads/worktrees/wt1/HEAD &&
+	git rev-parse worktrees/wt1/HEAD 2>warn >/dev/null &&
+	grep "worktrees/wt1/HEAD.*ambiguous" warn
+'
+
+test_expect_success 'reflog of main-worktree/HEAD' '
+	git reflog HEAD | sed "s/HEAD/main-worktree\/HEAD/" >expected &&
+	git reflog main-worktree/HEAD >actual &&
 	test_cmp expected actual &&
-	git -C wt1 reflog main/HEAD >actual.wt1 &&
+	git -C wt1 reflog main-worktree/HEAD >actual.wt1 &&
 	test_cmp expected actual.wt1
 '
 
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 444e8c1ad9..e97e6a7c6d 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -106,19 +106,17 @@ test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
 	test_when_finished "rm -rf .git/worktrees wt" &&
 	git worktree add wt &&
 	mv .git/HEAD .git/SAVED_HEAD &&
-	echo 0000000000000000000000000000000000000000 >.git/HEAD &&
+	echo $ZERO_OID >.git/HEAD &&
 	# avoid corrupt/broken HEAD from interfering with repo discovery
 	test_must_fail git -C wt fsck 2>out &&
-	cat out &&
 	grep "main/HEAD: detached HEAD points" out
 '
 
 test_expect_success 'other worktree HEAD link pointing at a funny object' '
 	test_when_finished "rm -rf .git/worktrees other" &&
 	git worktree add other &&
-	echo 0000000000000000000000000000000000000000 >.git/worktrees/other/HEAD &&
+	echo $ZERO_OID >.git/worktrees/other/HEAD &&
 	test_must_fail git fsck 2>out &&
-	cat out &&
 	grep "worktrees/other/HEAD: detached HEAD points" out
 '
 
@@ -127,7 +125,6 @@ test_expect_success 'other worktree HEAD link pointing at missing object' '
 	git worktree add other &&
 	echo "Contents missing from repo" | git hash-object --stdin >.git/worktrees/other/HEAD &&
 	test_must_fail git fsck 2>out &&
-	cat out &&
 	grep "worktrees/other/HEAD: invalid sha1 pointer" out
 '
 
@@ -136,7 +133,6 @@ test_expect_success 'other worktree HEAD link pointing at a funny place' '
 	git worktree add other &&
 	echo "ref: refs/funny/place" >.git/worktrees/other/HEAD &&
 	test_must_fail git fsck 2>out &&
-	cat out &&
 	grep "worktrees/other/HEAD points to something strange" out
 '
 

Elijah Newren (1):
  fsck: Move fsck_head_link() to get_default_heads() to avoid some
    globals

Nguyễn Thái Ngọc Duy (7):
  refs.c: indent with tabs, not spaces
  Add a place for (not) sharing stuff between worktrees
  refs: new ref types to make per-worktree refs visible to all worktrees
  revision.c: correct a parameter name
  revision.c: better error reporting on ref from different worktrees
  fsck: check HEAD and reflog from other worktrees
  reflog expire: cover reflog from all worktrees

 Documentation/git-reflog.txt           |  7 ++-
 Documentation/git-worktree.txt         | 32 ++++++++++-
 Documentation/gitrepository-layout.txt | 11 +++-
 builtin/fsck.c                         | 68 +++++++++++++++-------
 builtin/reflog.c                       | 22 ++++++-
 path.c                                 |  2 +
 refs.c                                 | 24 +++++++-
 refs.h                                 |  8 ++-
 refs/files-backend.c                   | 42 +++++++++++++-
 revision.c                             | 22 ++++---
 t/t0060-path-utils.sh                  |  2 +
 t/t1415-worktree-refs.sh               | 79 ++++++++++++++++++++++++++
 t/t1450-fsck.sh                        | 35 ++++++++++++
 worktree.c                             | 32 ++++++++++-
 worktree.h                             | 14 +++++
 15 files changed, 354 insertions(+), 46 deletions(-)
 create mode 100755 t/t1415-worktree-refs.sh

-- 
2.19.0.341.g3acb95d729

[PATCH v2 1/8] refs.c: indent with tabs, not spaces

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

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index a7a75b4cc0..9f7268d5fe 100644
--- a/refs.c
+++ b/refs.c
@@ -646,7 +646,7 @@ enum ref_type ref_type(const char *refname)
 		return REF_TYPE_PER_WORKTREE;
 	if (is_pseudoref_syntax(refname))
 		return REF_TYPE_PSEUDOREF;
-       return REF_TYPE_NORMAL;
+	return REF_TYPE_NORMAL;
 }
 
 long get_files_ref_lock_timeout_ms(void)
-- 
2.19.0.341.g3acb95d729

[PATCH v2 2/8] Add a place for (not) sharing stuff between worktrees

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

When multiple worktrees are used, we need rules to determine if
something belongs to one worktree or all of them. Instead of keeping
adding rules when new stuff comes (*), have a generic rule:

- Inside $GIT_DIR, which is per-worktree by default, add
  $GIT_DIR/common which is always shared. New features that want to
  share stuff should put stuff under this directory.

- Inside refs/, which is shared by default except refs/bisect, add
  refs/worktree/ which is per-worktree. We may eventually move
  refs/bisect to this new location and remove the exception in refs
  code.

(*) And it may also include stuff from external commands which will
    have no way to modify common/per-worktree rules.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-worktree.txt         | 19 ++++++++++++++-
 Documentation/gitrepository-layout.txt | 11 +++++++--
 path.c                                 |  2 ++
 refs.c                                 |  1 +
 refs/files-backend.c                   | 14 ++++++++---
 t/t0060-path-utils.sh                  |  2 ++
 t/t1415-worktree-refs.sh               | 33 ++++++++++++++++++++++++++
 7 files changed, 76 insertions(+), 6 deletions(-)
 create mode 100755 t/t1415-worktree-refs.sh

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e2ee9fc21b..a50fbf8094 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -204,6 +204,22 @@ working trees, it can be used to identify worktrees. For example if
 you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
 then "ghi" or "def/ghi" is enough to point to the former working tree.
 
+REFS
+----
+In multiple working trees, some refs may be shared between all working
+trees, some refs are local. One example is HEAD is different for all
+working trees. This section is about the sharing rules.
+
+In general, all pseudo refs are per working tree and all refs starting
+with "refs/" are shared. Pseudo refs are ones like HEAD which are
+directly under GIT_DIR instead of inside GIT_DIR/refs. There are one
+exception to this: refs inside refs/bisect and refs/worktree is not
+shared.
+
+To access refs, it's best not to look inside GIT_DIR directly. Instead
+use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1]
+which will handle refs correctly.
+
 DETAILS
 -------
 Each linked working tree has a private sub-directory in the repository's
@@ -228,7 +244,8 @@ linked working tree `git rev-parse --git-path HEAD` returns
 `/path/other/test-next/.git/HEAD` or `/path/main/.git/HEAD`) while `git
 rev-parse --git-path refs/heads/master` uses
 $GIT_COMMON_DIR and returns `/path/main/.git/refs/heads/master`,
-since refs are shared across all working trees.
+since refs are shared across all working trees, except refs/bisect and
+refs/worktree.
 
 See linkgit:gitrepository-layout[5] for more information. The rule of
 thumb is do not make any assumption about whether a path belongs to
diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index e85148f05e..89b616e049 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -95,8 +95,10 @@ refs::
 	References are stored in subdirectories of this
 	directory.  The 'git prune' command knows to preserve
 	objects reachable from refs found in this directory and
-	its subdirectories. This directory is ignored if $GIT_COMMON_DIR
-	is set and "$GIT_COMMON_DIR/refs" will be used instead.
+	its subdirectories.
+	This directory is ignored (except refs/bisect and
+	refs/worktree) if $GIT_COMMON_DIR is set and
+	"$GIT_COMMON_DIR/refs" will be used instead.
 
 refs/heads/`name`::
 	records tip-of-the-tree commit objects of branch `name`
@@ -165,6 +167,11 @@ hooks::
 	each hook. This directory is ignored if $GIT_COMMON_DIR is set
 	and "$GIT_COMMON_DIR/hooks" will be used instead.
 
+common::
+	When multiple working trees are used, most of files in
+	$GIT_DIR are per-worktree with a few known exceptions. All
+	files under 'common' however will be shared between all
+	working trees.
 
 index::
 	The current index file for the repository.  It is
diff --git a/path.c b/path.c
index 34f0f98349..bf4bb02a27 100644
--- a/path.c
+++ b/path.c
@@ -108,6 +108,7 @@ struct common_dir {
 
 static struct common_dir common_list[] = {
 	{ 0, 1, 0, "branches" },
+	{ 0, 1, 0, "common" },
 	{ 0, 1, 0, "hooks" },
 	{ 0, 1, 0, "info" },
 	{ 0, 0, 1, "info/sparse-checkout" },
@@ -118,6 +119,7 @@ static struct common_dir common_list[] = {
 	{ 0, 1, 0, "objects" },
 	{ 0, 1, 0, "refs" },
 	{ 0, 1, 1, "refs/bisect" },
+	{ 0, 1, 1, "refs/worktree" },
 	{ 0, 1, 0, "remotes" },
 	{ 0, 1, 0, "worktrees" },
 	{ 0, 1, 0, "rr-cache" },
diff --git a/refs.c b/refs.c
index 9f7268d5fe..1bc4ed301b 100644
--- a/refs.c
+++ b/refs.c
@@ -624,6 +624,7 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)
 static int is_per_worktree_ref(const char *refname)
 {
 	return !strcmp(refname, "HEAD") ||
+		starts_with(refname, "refs/worktree/") ||
 		starts_with(refname, "refs/bisect/") ||
 		starts_with(refname, "refs/rewritten/");
 }
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 16ef9325e0..2dd77f9485 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -269,9 +269,9 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
 	closedir(d);
 
 	/*
-	 * Manually add refs/bisect, which, being per-worktree, might
-	 * not appear in the directory listing for refs/ in the main
-	 * repo.
+	 * 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);
@@ -281,6 +281,14 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
 					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);
+		}
 	}
 }
 
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index cd74c0a471..c7b53e494b 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -306,6 +306,8 @@ test_git_path GIT_COMMON_DIR=bar hooks/me                 bar/hooks/me
 test_git_path GIT_COMMON_DIR=bar config                   bar/config
 test_git_path GIT_COMMON_DIR=bar packed-refs              bar/packed-refs
 test_git_path GIT_COMMON_DIR=bar shallow                  bar/shallow
+test_git_path GIT_COMMON_DIR=bar common                   bar/common
+test_git_path GIT_COMMON_DIR=bar common/file              bar/common/file
 
 # In the tests below, $(pwd) must be used because it is a native path on
 # Windows and avoids MSYS's path mangling (which simplifies "foo/../bar" and
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
new file mode 100755
index 0000000000..16c91bef57
--- /dev/null
+++ b/t/t1415-worktree-refs.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+
+test_description='per-worktree refs'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit initial &&
+	test_commit wt1 &&
+	test_commit wt2 &&
+	git worktree add wt1 wt1 &&
+	git worktree add wt2 wt2 &&
+	git checkout initial &&
+	git update-ref refs/worktree/foo HEAD &&
+	git -C wt1 update-ref refs/worktree/foo HEAD &&
+	git -C wt2 update-ref refs/worktree/foo HEAD
+'
+
+test_expect_success 'refs/worktree must not be packed' '
+	git pack-refs --all &&
+	test_path_is_missing .git/refs/tags/wt1 &&
+	test_path_is_file .git/refs/worktree/foo &&
+	test_path_is_file .git/worktrees/wt1/refs/worktree/foo &&
+	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
+'
+
+test_expect_success 'refs/worktree are per-worktree' '
+	test_cmp_rev worktree/foo initial &&
+	( cd wt1 && test_cmp_rev worktree/foo wt1 ) &&
+	( cd wt2 && test_cmp_rev worktree/foo wt2 )
+'
+
+test_done
-- 
2.19.0.341.g3acb95d729

[PATCH v2 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

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

One of the problems with multiple worktree is accessing per-worktree
refs of one worktree from another worktree. This was sort of solved by
multiple ref store, where the code can open the ref store of another
worktree and has access to the ref space of that worktree.

The problem with this is reporting. "HEAD" in another ref space is
also called "HEAD" like in the current ref space. In order to
differentiate them, all the code must somehow carry the ref store
around and print something like "HEAD from this ref store".

But that is not feasible (or possible with a _lot_ of work). With the
current design, we pass a reference around as a string (so called
"refname"). Extending this design to pass a string _and_ a ref store
is a nightmare, especially when handling extended SHA-1 syntax.

So we do it another way. Instead of entering a separate ref space, we
make refs from other worktrees available in the current ref space. So
"HEAD" is always HEAD of the current worktree, but then we can have
"worktrees/blah/HEAD" to denote HEAD from a worktree named
"blah". This syntax coincidentally matches the underlying directory
structure which makes implementation a bit easier.

The main worktree has to be treated specially because well.. it's
special from the beginning. So HEAD from the main worktree is
acccessible via the name "main-worktree/HEAD" instead of
"worktrees/main/HEAD" because "main" could be just another secondary
worktree.

This patch also makes it possible to specify refs from one worktree in
another one, e.g.

    git log worktrees/foo/HEAD

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-worktree.txt | 15 ++++++++++-
 refs.c                         | 21 ++++++++++++++++
 refs.h                         |  8 +++---
 refs/files-backend.c           | 28 +++++++++++++++++++++
 t/t1415-worktree-refs.sh       | 46 ++++++++++++++++++++++++++++++++++
 5 files changed, 114 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index a50fbf8094..58415f9207 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -208,7 +208,8 @@ REFS
 ----
 In multiple working trees, some refs may be shared between all working
 trees, some refs are local. One example is HEAD is different for all
-working trees. This section is about the sharing rules.
+working trees. This section is about the sharing rules and how to access
+refs of one working tree from another.
 
 In general, all pseudo refs are per working tree and all refs starting
 with "refs/" are shared. Pseudo refs are ones like HEAD which are
@@ -216,6 +217,18 @@ directly under GIT_DIR instead of inside GIT_DIR/refs. There are one
 exception to this: refs inside refs/bisect and refs/worktree is not
 shared.
 
+Refs that are per working tree can still be accessed from another
+working tree via two special paths main-worktree and worktrees. The
+former gives access to per-worktree refs of the main working tree,
+while the former to all linked working trees.
+
+For example, main-worktree/HEAD or main-worktree/refs/bisect/good
+resolve to the same value as the main working tree's HEAD and
+refs/bisect/good respectively. Similarly, worktrees/foo/HEAD or
+worktrees/bar/refs/bisect/bad are the same as
+GIT_COMMON_DIR/worktrees/foo/HEAD and
+GIT_COMMON_DIR/worktrees/bar/refs/bisect/bad.
+
 To access refs, it's best not to look inside GIT_DIR directly. Instead
 use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1]
 which will handle refs correctly.
diff --git a/refs.c b/refs.c
index 1bc4ed301b..2378b2e7fc 100644
--- a/refs.c
+++ b/refs.c
@@ -641,12 +641,33 @@ static int is_pseudoref_syntax(const char *refname)
 	return 1;
 }
 
+static int is_main_pseudoref_syntax(const char *refname)
+{
+	return skip_prefix(refname, "main-worktree/", &refname) &&
+		*refname &&
+		is_pseudoref_syntax(refname);
+}
+
+static int is_other_pseudoref_syntax(const char *refname)
+{
+	if (!skip_prefix(refname, "worktrees/", &refname))
+		return 0;
+	refname = strchr(refname, '/');
+	if (!refname || !refname[1])
+		return 0;
+	return is_pseudoref_syntax(refname + 1);
+}
+
 enum ref_type ref_type(const char *refname)
 {
 	if (is_per_worktree_ref(refname))
 		return REF_TYPE_PER_WORKTREE;
 	if (is_pseudoref_syntax(refname))
 		return REF_TYPE_PSEUDOREF;
+	if (is_main_pseudoref_syntax(refname))
+		return REF_TYPE_MAIN_PSEUDOREF;
+	if (is_other_pseudoref_syntax(refname))
+		return REF_TYPE_OTHER_PSEUDOREF;
 	return REF_TYPE_NORMAL;
 }
 
diff --git a/refs.h b/refs.h
index bd52c1bbae..9b53dbeae8 100644
--- a/refs.h
+++ b/refs.h
@@ -704,9 +704,11 @@ int parse_hide_refs_config(const char *var, const char *value, const char *);
 int ref_is_hidden(const char *, const char *);
 
 enum ref_type {
-	REF_TYPE_PER_WORKTREE,
-	REF_TYPE_PSEUDOREF,
-	REF_TYPE_NORMAL,
+	REF_TYPE_PER_WORKTREE,	  /* refs inside refs/ but not shared       */
+	REF_TYPE_PSEUDOREF,	  /* refs outside refs/ in current worktree */
+	REF_TYPE_MAIN_PSEUDOREF,  /* pseudo refs from the main worktree     */
+	REF_TYPE_OTHER_PSEUDOREF, /* pseudo refs from other worktrees       */
+	REF_TYPE_NORMAL,	  /* normal/shared refs inside refs/        */
 };
 
 enum ref_type ref_type(const char *refname);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 2dd77f9485..9ca2a3706c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -149,6 +149,23 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store,
 	return refs;
 }
 
+static void files_reflog_path_other_worktrees(struct files_ref_store *refs,
+					      struct strbuf *sb,
+					      const char *refname)
+{
+	const char *real_ref;
+
+	if (!skip_prefix(refname, "worktrees/", &real_ref))
+		BUG("refname %s is not a other-worktree ref", refname);
+	real_ref = strchr(real_ref, '/');
+	if (!real_ref)
+		BUG("refname %s is not a other-worktree ref", refname);
+	real_ref++;
+
+	strbuf_addf(sb, "%s/%.*slogs/%s", refs->gitcommondir,
+		    (int)(real_ref - refname), refname, real_ref);
+}
+
 static void files_reflog_path(struct files_ref_store *refs,
 			      struct strbuf *sb,
 			      const char *refname)
@@ -158,6 +175,12 @@ static void files_reflog_path(struct files_ref_store *refs,
 	case REF_TYPE_PSEUDOREF:
 		strbuf_addf(sb, "%s/logs/%s", refs->gitdir, refname);
 		break;
+	case REF_TYPE_OTHER_PSEUDOREF:
+		return files_reflog_path_other_worktrees(refs, sb, refname);
+	case REF_TYPE_MAIN_PSEUDOREF:
+		if (!skip_prefix(refname, "main-worktree/", &refname))
+			BUG("ref %s is not a main pseudoref", refname);
+		/* passthru */
 	case REF_TYPE_NORMAL:
 		strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir, refname);
 		break;
@@ -176,6 +199,11 @@ static void files_ref_path(struct files_ref_store *refs,
 	case REF_TYPE_PSEUDOREF:
 		strbuf_addf(sb, "%s/%s", refs->gitdir, refname);
 		break;
+	case REF_TYPE_MAIN_PSEUDOREF:
+		if (!skip_prefix(refname, "main-worktree/", &refname))
+			BUG("ref %s is not a main pseudoref", refname);
+		/* passthru */
+	case REF_TYPE_OTHER_PSEUDOREF:
 	case REF_TYPE_NORMAL:
 		strbuf_addf(sb, "%s/%s", refs->gitcommondir, refname);
 		break;
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
index 16c91bef57..8b701d07af 100755
--- a/t/t1415-worktree-refs.sh
+++ b/t/t1415-worktree-refs.sh
@@ -30,4 +30,50 @@ test_expect_success 'refs/worktree are per-worktree' '
 	( cd wt2 && test_cmp_rev worktree/foo wt2 )
 '
 
+test_expect_success 'resolve main-worktree/HEAD' '
+	test_cmp_rev main-worktree/HEAD initial &&
+	( cd wt1 && test_cmp_rev main-worktree/HEAD initial ) &&
+	( cd wt2 && test_cmp_rev main-worktree/HEAD initial )
+'
+
+test_expect_success 'ambiguous main-worktree/HEAD' '
+	mkdir -p .git/refs/heads/main-worktree &&
+	test_when_finished rm .git/refs/heads/main-worktree/HEAD &&
+	cp .git/HEAD .git/refs/heads/main-worktree/HEAD &&
+	git rev-parse main-worktree/HEAD 2>warn >/dev/null &&
+	grep "main-worktree/HEAD.*ambiguous" warn
+'
+
+test_expect_success 'resolve worktrees/xx/HEAD' '
+	test_cmp_rev worktrees/wt1/HEAD wt1 &&
+	( cd wt1 && test_cmp_rev worktrees/wt1/HEAD wt1 ) &&
+	( cd wt2 && test_cmp_rev worktrees/wt1/HEAD wt1 )
+'
+
+test_expect_success 'ambiguous worktrees/xx/HEAD' '
+	mkdir -p .git/refs/heads/worktrees/wt1 &&
+	test_when_finished rm .git/refs/heads/worktrees/wt1/HEAD &&
+	cp .git/HEAD .git/refs/heads/worktrees/wt1/HEAD &&
+	git rev-parse worktrees/wt1/HEAD 2>warn >/dev/null &&
+	grep "worktrees/wt1/HEAD.*ambiguous" warn
+'
+
+test_expect_success 'reflog of main-worktree/HEAD' '
+	git reflog HEAD | sed "s/HEAD/main-worktree\/HEAD/" >expected &&
+	git reflog main-worktree/HEAD >actual &&
+	test_cmp expected actual &&
+	git -C wt1 reflog main-worktree/HEAD >actual.wt1 &&
+	test_cmp expected actual.wt1
+'
+
+test_expect_success 'reflog of worktrees/xx/HEAD' '
+	git -C wt2 reflog HEAD | sed "s/HEAD/worktrees\/wt2\/HEAD/" >expected &&
+	git reflog worktrees/wt2/HEAD >actual &&
+	test_cmp expected actual &&
+	git -C wt1 reflog worktrees/wt2/HEAD >actual.wt1 &&
+	test_cmp expected actual.wt1 &&
+	git -C wt2 reflog worktrees/wt2/HEAD >actual.wt2 &&
+	test_cmp expected actual.wt2
+'
+
 test_done
-- 
2.19.0.341.g3acb95d729

[PATCH v2 4/8] revision.c: correct a parameter name

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

This function is a callback of for_each_reflog() which will pass a ref
name as the first argument, not a path (to a reflog file).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/revision.c b/revision.c
index e18bd530e4..63aae722c1 100644
--- a/revision.c
+++ b/revision.c
@@ -1277,13 +1277,14 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
 	return 0;
 }
 
-static int handle_one_reflog(const char *path, const struct object_id *oid,
+static int handle_one_reflog(const char *refname,
+			     const struct object_id *oid,
 			     int flag, void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
 	cb->warned_bad_reflog = 0;
-	cb->name_for_errormsg = path;
-	refs_for_each_reflog_ent(cb->refs, path,
+	cb->name_for_errormsg = refname;
+	refs_for_each_reflog_ent(cb->refs, refname,
 				 handle_one_reflog_ent, cb_data);
 	return 0;
 }
-- 
2.19.0.341.g3acb95d729

[PATCH v2 5/8] revision.c: better error reporting on ref from different worktrees

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

Make use of the new ref aliases to pass refs from another worktree
around and access them from the current ref store instead. This does
not change any functionality, but when a problem arises, we would like
the reported messages to mention full ref aliases, like this:

    fatal: bad object worktrees/ztemp/HEAD
    warning: reflog of 'main-worktree/HEAD' references pruned commits

instead of

    fatal: bad object HEAD
    warning: reflog of 'HEAD' references pruned commits

which does not really tell where the refs are from.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 21 +++++++++++++--------
 worktree.c | 32 +++++++++++++++++++++++++++++---
 worktree.h | 14 ++++++++++++++
 3 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/revision.c b/revision.c
index 63aae722c1..8ce660e3b1 100644
--- a/revision.c
+++ b/revision.c
@@ -1177,7 +1177,7 @@ struct all_refs_cb {
 	int warned_bad_reflog;
 	struct rev_info *all_revs;
 	const char *name_for_errormsg;
-	struct ref_store *refs;
+	struct worktree *wt;
 };
 
 int ref_excluded(struct string_list *ref_excludes, const char *path)
@@ -1214,7 +1214,7 @@ static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs,
 	cb->all_revs = revs;
 	cb->all_flags = flags;
 	revs->rev_input_given = 1;
-	cb->refs = NULL;
+	cb->wt = NULL;
 }
 
 void clear_ref_exclusion(struct string_list **ref_excludes_p)
@@ -1277,15 +1277,20 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
 	return 0;
 }
 
-static int handle_one_reflog(const char *refname,
+static int handle_one_reflog(const char *refname_in_wt,
 			     const struct object_id *oid,
 			     int flag, void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
+	struct strbuf refname = STRBUF_INIT;
+
 	cb->warned_bad_reflog = 0;
-	cb->name_for_errormsg = refname;
-	refs_for_each_reflog_ent(cb->refs, refname,
+	strbuf_worktree_ref(cb->wt, &refname, refname_in_wt);
+	cb->name_for_errormsg = refname.buf;
+	refs_for_each_reflog_ent(get_main_ref_store(the_repository),
+				 refname.buf,
 				 handle_one_reflog_ent, cb_data);
+	strbuf_release(&refname);
 	return 0;
 }
 
@@ -1300,8 +1305,8 @@ static void add_other_reflogs_to_pending(struct all_refs_cb *cb)
 		if (wt->is_current)
 			continue;
 
-		cb->refs = get_worktree_ref_store(wt);
-		refs_for_each_reflog(cb->refs,
+		cb->wt = wt;
+		refs_for_each_reflog(get_worktree_ref_store(wt),
 				     handle_one_reflog,
 				     cb);
 	}
@@ -1314,7 +1319,7 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags)
 
 	cb.all_revs = revs;
 	cb.all_flags = flags;
-	cb.refs = get_main_ref_store(the_repository);
+	cb.wt = NULL;
 	for_each_reflog(handle_one_reflog, &cb);
 
 	if (!revs->single_worktree)
diff --git a/worktree.c b/worktree.c
index b0d0b5426d..ec1a5bc511 100644
--- a/worktree.c
+++ b/worktree.c
@@ -487,6 +487,28 @@ int submodule_uses_worktrees(const char *path)
 	return ret;
 }
 
+void strbuf_worktree_ref(const struct worktree *wt,
+			 struct strbuf *sb,
+			 const char *refname)
+{
+	if (wt && !wt->is_current) {
+		if (is_main_worktree(wt))
+			strbuf_addstr(sb, "main/");
+		else
+			strbuf_addf(sb, "worktrees/%s/", wt->id);
+	}
+	strbuf_addstr(sb, refname);
+}
+
+const char *worktree_ref(const struct worktree *wt, const char *refname)
+{
+	static struct strbuf sb = STRBUF_INIT;
+
+	strbuf_reset(&sb);
+	strbuf_worktree_ref(wt, &sb, refname);
+	return sb.buf;
+}
+
 int other_head_refs(each_ref_fn fn, void *cb_data)
 {
 	struct worktree **worktrees, **p;
@@ -495,13 +517,17 @@ int other_head_refs(each_ref_fn fn, void *cb_data)
 	worktrees = get_worktrees(0);
 	for (p = worktrees; *p; p++) {
 		struct worktree *wt = *p;
-		struct ref_store *refs;
+		struct object_id oid;
+		int flag;
 
 		if (wt->is_current)
 			continue;
 
-		refs = get_worktree_ref_store(wt);
-		ret = refs_head_ref(refs, fn, cb_data);
+		if (!refs_read_ref_full(get_main_ref_store(the_repository),
+					worktree_ref(wt, "HEAD"),
+					RESOLVE_REF_READING,
+					&oid, &flag))
+			ret = fn(worktree_ref(wt, "HEAD"), &oid, flag, cb_data);
 		if (ret)
 			break;
 	}
diff --git a/worktree.h b/worktree.h
index df3fc30f73..0016eb9e88 100644
--- a/worktree.h
+++ b/worktree.h
@@ -108,4 +108,18 @@ extern const char *worktree_git_path(const struct worktree *wt,
 				     const char *fmt, ...)
 	__attribute__((format (printf, 2, 3)));
 
+/*
+ * Return a refname suitable for access from the current ref store.
+ */
+void strbuf_worktree_ref(const struct worktree *wt,
+			 struct strbuf *sb,
+			 const char *refname);
+
+/*
+ * Return a refname suitable for access from the current ref
+ * store. The result may be destroyed at the next call.
+ */
+const char *worktree_ref(const struct worktree *wt,
+			 const char *refname);
+
 #endif
-- 
2.19.0.341.g3acb95d729

[PATCH v2 6/8] fsck: Move fsck_head_link() to get_default_heads() to avoid some globals

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

From: Elijah Newren <newren@gmail.com>

This will make it easier to check the HEAD of other worktrees from fsck.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/fsck.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 63c8578cc1..24f8a09a3c 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -36,8 +36,6 @@ static int check_strict;
 static int keep_cache_objects;
 static struct fsck_options fsck_walk_options = FSCK_OPTIONS_DEFAULT;
 static struct fsck_options fsck_obj_options = FSCK_OPTIONS_DEFAULT;
-static struct object_id head_oid;
-static const char *head_points_at;
 static int errors_found;
 static int write_lost_and_found;
 static int verbose;
@@ -484,8 +482,15 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,
 	return 0;
 }
 
+static int fsck_head_link(const char **head_points_at,
+			  struct object_id *head_oid);
+
 static void get_default_heads(void)
 {
+	const char *head_points_at;
+	struct object_id head_oid;
+
+	fsck_head_link(&head_points_at, &head_oid);
 	if (head_points_at && !is_null_oid(&head_oid))
 		fsck_handle_ref("HEAD", &head_oid, 0, NULL);
 	for_each_rawref(fsck_handle_ref, NULL);
@@ -579,33 +584,34 @@ static void fsck_object_dir(const char *path)
 	stop_progress(&progress);
 }
 
-static int fsck_head_link(void)
+static int fsck_head_link(const char **head_points_at,
+			  struct object_id *head_oid)
 {
 	int null_is_error = 0;
 
 	if (verbose)
 		fprintf(stderr, "Checking HEAD link\n");
 
-	head_points_at = resolve_ref_unsafe("HEAD", 0, &head_oid, NULL);
-	if (!head_points_at) {
+	*head_points_at = resolve_ref_unsafe("HEAD", 0, head_oid, NULL);
+	if (!*head_points_at) {
 		errors_found |= ERROR_REFS;
 		return error("Invalid HEAD");
 	}
-	if (!strcmp(head_points_at, "HEAD"))
+	if (!strcmp(*head_points_at, "HEAD"))
 		/* detached HEAD */
 		null_is_error = 1;
-	else if (!starts_with(head_points_at, "refs/heads/")) {
+	else if (!starts_with(*head_points_at, "refs/heads/")) {
 		errors_found |= ERROR_REFS;
 		return error("HEAD points to something strange (%s)",
-			     head_points_at);
+			     *head_points_at);
 	}
-	if (is_null_oid(&head_oid)) {
+	if (is_null_oid(head_oid)) {
 		if (null_is_error) {
 			errors_found |= ERROR_REFS;
 			return error("HEAD: detached HEAD points at nothing");
 		}
 		fprintf(stderr, "notice: HEAD points to an unborn branch (%s)\n",
-			head_points_at + 11);
+			*head_points_at + 11);
 	}
 	return 0;
 }
@@ -720,7 +726,6 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 
 	git_config(fsck_config, NULL);
 
-	fsck_head_link();
 	if (connectivity_only) {
 		for_each_loose_object(mark_loose_for_connectivity, NULL, 0);
 		for_each_packed_object(mark_packed_for_connectivity, NULL, 0);
-- 
2.19.0.341.g3acb95d729

[PATCH v2 7/8] fsck: check HEAD and reflog from other worktrees

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

fsck is a repo-wide operation and should check all references no
matter which worktree they are associated to.

Reported-by: Jeff King <peff@peff.net>
Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/fsck.c  | 55 ++++++++++++++++++++++++++++++++++---------------
 t/t1450-fsck.sh | 35 +++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 17 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 24f8a09a3c..71492c158d 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -19,6 +19,7 @@
 #include "packfile.h"
 #include "object-store.h"
 #include "run-command.h"
+#include "worktree.h"
 
 #define REACHABLE 0x0001
 #define SEEN      0x0002
@@ -444,7 +445,11 @@ static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noid
 static int fsck_handle_reflog(const char *logname, const struct object_id *oid,
 			      int flag, void *cb_data)
 {
-	for_each_reflog_ent(logname, fsck_handle_reflog_ent, (void *)logname);
+	struct strbuf refname = STRBUF_INIT;
+
+	strbuf_worktree_ref(cb_data, &refname, logname);
+	for_each_reflog_ent(refname.buf, fsck_handle_reflog_ent, refname.buf);
+	strbuf_release(&refname);
 	return 0;
 }
 
@@ -482,20 +487,34 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,
 	return 0;
 }
 
-static int fsck_head_link(const char **head_points_at,
+static int fsck_head_link(const char *head_ref_name,
+			  const char **head_points_at,
 			  struct object_id *head_oid);
 
 static void get_default_heads(void)
 {
+	struct worktree **worktrees, **p;
 	const char *head_points_at;
 	struct object_id head_oid;
 
-	fsck_head_link(&head_points_at, &head_oid);
-	if (head_points_at && !is_null_oid(&head_oid))
-		fsck_handle_ref("HEAD", &head_oid, 0, NULL);
 	for_each_rawref(fsck_handle_ref, NULL);
-	if (include_reflogs)
-		for_each_reflog(fsck_handle_reflog, NULL);
+
+	worktrees = get_worktrees(0);
+	for (p = worktrees; *p; p++) {
+		struct worktree *wt = *p;
+		struct strbuf ref = STRBUF_INIT;
+
+		strbuf_worktree_ref(wt, &ref, "HEAD");
+		fsck_head_link(ref.buf, &head_points_at, &head_oid);
+		if (head_points_at && !is_null_oid(&head_oid))
+			fsck_handle_ref(ref.buf, &head_oid, 0, NULL);
+		strbuf_release(&ref);
+
+		if (include_reflogs)
+			refs_for_each_reflog(get_worktree_ref_store(wt),
+					     fsck_handle_reflog, wt);
+	}
+	free_worktrees(worktrees);
 
 	/*
 	 * Not having any default heads isn't really fatal, but
@@ -584,34 +603,36 @@ static void fsck_object_dir(const char *path)
 	stop_progress(&progress);
 }
 
-static int fsck_head_link(const char **head_points_at,
+static int fsck_head_link(const char *head_ref_name,
+			  const char **head_points_at,
 			  struct object_id *head_oid)
 {
 	int null_is_error = 0;
 
 	if (verbose)
-		fprintf(stderr, "Checking HEAD link\n");
+		fprintf(stderr, "Checking %s link\n", head_ref_name);
 
-	*head_points_at = resolve_ref_unsafe("HEAD", 0, head_oid, NULL);
+	*head_points_at = resolve_ref_unsafe(head_ref_name, 0, head_oid, NULL);
 	if (!*head_points_at) {
 		errors_found |= ERROR_REFS;
-		return error("Invalid HEAD");
+		return error("Invalid %s", head_ref_name);
 	}
-	if (!strcmp(*head_points_at, "HEAD"))
+	if (!strcmp(*head_points_at, head_ref_name))
 		/* detached HEAD */
 		null_is_error = 1;
 	else if (!starts_with(*head_points_at, "refs/heads/")) {
 		errors_found |= ERROR_REFS;
-		return error("HEAD points to something strange (%s)",
-			     *head_points_at);
+		return error("%s points to something strange (%s)",
+			     head_ref_name, *head_points_at);
 	}
 	if (is_null_oid(head_oid)) {
 		if (null_is_error) {
 			errors_found |= ERROR_REFS;
-			return error("HEAD: detached HEAD points at nothing");
+			return error("%s: detached HEAD points at nothing",
+				     head_ref_name);
 		}
-		fprintf(stderr, "notice: HEAD points to an unborn branch (%s)\n",
-			*head_points_at + 11);
+		fprintf(stderr, "notice: %s points to an unborn branch (%s)\n",
+			head_ref_name, *head_points_at + 11);
 	}
 	return 0;
 }
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 0f2dd26f74..e97e6a7c6d 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -101,6 +101,41 @@ test_expect_success 'HEAD link pointing at a funny place' '
 	grep "HEAD points to something strange" out
 '
 
+test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
+	test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
+	test_when_finished "rm -rf .git/worktrees wt" &&
+	git worktree add wt &&
+	mv .git/HEAD .git/SAVED_HEAD &&
+	echo $ZERO_OID >.git/HEAD &&
+	# avoid corrupt/broken HEAD from interfering with repo discovery
+	test_must_fail git -C wt fsck 2>out &&
+	grep "main/HEAD: detached HEAD points" out
+'
+
+test_expect_success 'other worktree HEAD link pointing at a funny object' '
+	test_when_finished "rm -rf .git/worktrees other" &&
+	git worktree add other &&
+	echo $ZERO_OID >.git/worktrees/other/HEAD &&
+	test_must_fail git fsck 2>out &&
+	grep "worktrees/other/HEAD: detached HEAD points" out
+'
+
+test_expect_success 'other worktree HEAD link pointing at missing object' '
+	test_when_finished "rm -rf .git/worktrees other" &&
+	git worktree add other &&
+	echo "Contents missing from repo" | git hash-object --stdin >.git/worktrees/other/HEAD &&
+	test_must_fail git fsck 2>out &&
+	grep "worktrees/other/HEAD: invalid sha1 pointer" out
+'
+
+test_expect_success 'other worktree HEAD link pointing at a funny place' '
+	test_when_finished "rm -rf .git/worktrees other" &&
+	git worktree add other &&
+	echo "ref: refs/funny/place" >.git/worktrees/other/HEAD &&
+	test_must_fail git fsck 2>out &&
+	grep "worktrees/other/HEAD points to something strange" out
+'
+
 test_expect_success 'email without @ is okay' '
 	git cat-file commit HEAD >basis &&
 	sed "s/@/AT/" basis >okay &&
-- 
2.19.0.341.g3acb95d729

[PATCH v2 8/8] reflog expire: cover reflog from all worktrees

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

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-reflog.txt |  7 ++++++-
 builtin/reflog.c             | 22 +++++++++++++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt
index 472a6808cd..ff487ff77d 100644
--- a/Documentation/git-reflog.txt
+++ b/Documentation/git-reflog.txt
@@ -20,7 +20,7 @@ depending on the subcommand:
 'git reflog' ['show'] [log-options] [<ref>]
 'git reflog expire' [--expire=<time>] [--expire-unreachable=<time>]
 	[--rewrite] [--updateref] [--stale-fix]
-	[--dry-run | -n] [--verbose] [--all | <refs>...]
+	[--dry-run | -n] [--verbose] [--all [--single-worktree] | <refs>...]
 'git reflog delete' [--rewrite] [--updateref]
 	[--dry-run | -n] [--verbose] ref@\{specifier\}...
 'git reflog exists' <ref>
@@ -72,6 +72,11 @@ Options for `expire`
 --all::
 	Process the reflogs of all references.
 
+--single-worktree::
+	By default when `--all` is specified, reflogs from all working
+	trees are processed. This option limits the processing to reflogs
+	from the current working tree only.
+
 --expire=<time>::
 	Prune entries older than the specified time. If this option is
 	not specified, the expiration time is taken from the
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 3acef5a0ab..eed956851e 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -10,6 +10,7 @@
 #include "diff.h"
 #include "revision.h"
 #include "reachable.h"
+#include "worktree.h"
 
 /* NEEDSWORK: switch to using parse_options */
 static const char reflog_expire_usage[] =
@@ -52,6 +53,7 @@ struct collect_reflog_cb {
 	struct collected_reflog **e;
 	int alloc;
 	int nr;
+	struct worktree *wt;
 };
 
 /* Remember to update object flag allocation in object.h */
@@ -388,8 +390,12 @@ static int collect_reflog(const char *ref, const struct object_id *oid, int unus
 {
 	struct collected_reflog *e;
 	struct collect_reflog_cb *cb = cb_data;
+	struct strbuf newref = STRBUF_INIT;
+
+	strbuf_worktree_ref(cb->wt, &newref, ref);
+	FLEX_ALLOC_STR(e, reflog, newref.buf);
+	strbuf_release(&newref);
 
-	FLEX_ALLOC_STR(e, reflog, ref);
 	oidcpy(&e->oid, oid);
 	ALLOC_GROW(cb->e, cb->nr + 1, cb->alloc);
 	cb->e[cb->nr++] = e;
@@ -512,7 +518,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 {
 	struct expire_reflog_policy_cb cb;
 	timestamp_t now = time(NULL);
-	int i, status, do_all;
+	int i, status, do_all, all_worktrees = 1;
 	int explicit_expiry = 0;
 	unsigned int flags = 0;
 
@@ -549,6 +555,8 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 			flags |= EXPIRE_REFLOGS_UPDATE_REF;
 		else if (!strcmp(arg, "--all"))
 			do_all = 1;
+		else if (!strcmp(arg, "--single-worktree"))
+			all_worktrees = 0;
 		else if (!strcmp(arg, "--verbose"))
 			flags |= EXPIRE_REFLOGS_VERBOSE;
 		else if (!strcmp(arg, "--")) {
@@ -577,10 +585,18 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 
 	if (do_all) {
 		struct collect_reflog_cb collected;
+		struct worktree **worktrees, **p;
 		int i;
 
 		memset(&collected, 0, sizeof(collected));
-		for_each_reflog(collect_reflog, &collected);
+		worktrees = get_worktrees(0);
+		for (p = worktrees; *p; p++) {
+			if (!all_worktrees && !(*p)->is_current)
+				continue;
+			collected.wt = *p;
+			for_each_reflog(collect_reflog, &collected);
+		}
+		free_worktrees(worktrees);
 		for (i = 0; i < collected.nr; i++) {
 			struct collected_reflog *e = collected.e[i];
 			set_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog);
-- 
2.19.0.341.g3acb95d729

Re: [PATCH v2 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

[Eric Sunshine]

On Sat, Sep 29, 2018 at 3:10 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> The main worktree has to be treated specially because well.. it's

Nit: s/well../well.../

> special from the beginning. So HEAD from the main worktree is
> acccessible via the name "main-worktree/HEAD" instead of
> "worktrees/main/HEAD" because "main" could be just another secondary
> worktree.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> @@ -216,6 +217,18 @@ directly under GIT_DIR instead of inside GIT_DIR/refs. There are one
> +Refs that are per working tree can still be accessed from another
> +working tree via two special paths main-worktree and worktrees. The

s/paths/paths,/

> +former gives access to per-worktree refs of the main working tree,
> +while the former to all linked working trees.

s/former/latter/

> diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
> @@ -30,4 +30,50 @@ test_expect_success 'refs/worktree are per-worktree' '
> +test_expect_success 'ambiguous main-worktree/HEAD' '
> +       mkdir -p .git/refs/heads/main-worktree &&
> +       test_when_finished rm .git/refs/heads/main-worktree/HEAD &&
> +       cp .git/HEAD .git/refs/heads/main-worktree/HEAD &&

Better to use "rm -f" for cleanup in case this 'cp' fails for some reason.

> +       git rev-parse main-worktree/HEAD 2>warn >/dev/null &&

You could probably omit the /dev/null redirect.

> +       grep "main-worktree/HEAD.*ambiguous" warn
> +'
> +
> +test_expect_success 'ambiguous worktrees/xx/HEAD' '
> +       mkdir -p .git/refs/heads/worktrees/wt1 &&
> +       test_when_finished rm .git/refs/heads/worktrees/wt1/HEAD &&

Ditto "rm -f".

> +       cp .git/HEAD .git/refs/heads/worktrees/wt1/HEAD &&
> +       git rev-parse worktrees/wt1/HEAD 2>warn >/dev/null &&

Ditto /dev/null.

> +       grep "worktrees/wt1/HEAD.*ambiguous" warn
> +'

Re: [PATCH v2 5/8] revision.c: better error reporting on ref from different worktrees

[Eric Sunshine]

On Sat, Sep 29, 2018 at 3:11 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Make use of the new ref aliases to pass refs from another worktree
> around and access them from the current ref store instead. This does
> not change any functionality, but when a problem arises, we would like
> the reported messages to mention full ref aliases, like this:
> [...]
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/worktree.c b/worktree.c
> @@ -487,6 +487,28 @@ int submodule_uses_worktrees(const char *path)
> +void strbuf_worktree_ref(const struct worktree *wt,
> +                        struct strbuf *sb,
> +                        const char *refname)
> +{
> +       if (wt && !wt->is_current) {
> +               if (is_main_worktree(wt))
> +                       strbuf_addstr(sb, "main/");

I think this needs to be "main-worktree/", doesn't it?

> +               else
> +                       strbuf_addf(sb, "worktrees/%s/", wt->id);
> +       }
> +       strbuf_addstr(sb, refname);
> +}
> diff --git a/worktree.h b/worktree.h
> @@ -108,4 +108,18 @@ extern const char *worktree_git_path(const struct worktree *wt,
> +/*
> + * Return a refname suitable for access from the current ref
> + * store. The result may be destroyed at the next call.

s/may/will/

Re: [PATCH v2 8/8] reflog expire: cover reflog from all worktrees

[Eric Sunshine]

On Sat, Sep 29, 2018 at 3:11 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt
> @@ -72,6 +72,11 @@ Options for `expire`
> +--single-worktree::
> +       By default when `--all` is specified, reflogs from all working
> +       trees are processed. This option limits the processing to reflogs
> +       from the current working tree only.

Bikeshedding: I wonder if this should be named "--this-worktree" or
"--this-worktree-only" or if it should somehow be orthogonal to --all
rather than modifying it. (Genuine questions. I don't have the
answers.)

> diff --git a/builtin/reflog.c b/builtin/reflog.c
> @@ -577,10 +585,18 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
>         if (do_all) {
>                 struct collect_reflog_cb collected;
> +               struct worktree **worktrees, **p;
>                 int i;
>
>                 memset(&collected, 0, sizeof(collected));
> -               for_each_reflog(collect_reflog, &collected);
> +               worktrees = get_worktrees(0);
> +               for (p = worktrees; *p; p++) {
> +                       if (!all_worktrees && !(*p)->is_current)
> +                               continue;
> +                       collected.wt = *p;
> +                       for_each_reflog(collect_reflog, &collected);
> +               }
> +               free_worktrees(worktrees);

Should this have a test in the test suite?

Re: [PATCH v2 8/8] reflog expire: cover reflog from all worktrees

[Duy Nguyen]

On Sun, Sep 30, 2018 at 7:36 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
>
> On Sat, Sep 29, 2018 at 3:11 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> > ---
> > diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt
> > @@ -72,6 +72,11 @@ Options for `expire`
> > +--single-worktree::
> > +       By default when `--all` is specified, reflogs from all working
> > +       trees are processed. This option limits the processing to reflogs
> > +       from the current working tree only.
>
> Bikeshedding: I wonder if this should be named "--this-worktree" or
> "--this-worktree-only" or if it should somehow be orthogonal to --all
> rather than modifying it. (Genuine questions. I don't have the
> answers.)

It follows a precedent (made by me :p) which is rev-list
--single-worktree. I doubt that option is widely used though so we
could still rename it if there's a better name. I made
--single-worktree to contrast "all worktrees" by default. Even if it's
"this/current worktree" it still has to somehow say "everything in
this worktree" so I felt modifying --all was a good idea.

> > diff --git a/builtin/reflog.c b/builtin/reflog.c
> > @@ -577,10 +585,18 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
> >         if (do_all) {
> >                 struct collect_reflog_cb collected;
> > +               struct worktree **worktrees, **p;
> >                 int i;
> >
> >                 memset(&collected, 0, sizeof(collected));
> > -               for_each_reflog(collect_reflog, &collected);
> > +               worktrees = get_worktrees(0);
> > +               for (p = worktrees; *p; p++) {
> > +                       if (!all_worktrees && !(*p)->is_current)
> > +                               continue;
> > +                       collected.wt = *p;
> > +                       for_each_reflog(collect_reflog, &collected);
> > +               }
> > +               free_worktrees(worktrees);
>
> Should this have a test in the test suite?

Of course. I was partly lazy/tired near the end, and anticipated more
comments anyway so I did not do it :D
-- 
Duy

Re: [PATCH v2 8/8] reflog expire: cover reflog from all worktrees

[Eric Sunshine]

On Tue, Oct 2, 2018 at 12:16 PM Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Sep 30, 2018 at 7:36 AM Eric Sunshine <sunshine@sunshineco.com> wrote:
> > On Sat, Sep 29, 2018 at 3:11 PM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> > > +--single-worktree::
> > > +       By default when `--all` is specified, reflogs from all working
> > > +       trees are processed. This option limits the processing to reflogs
> > > +       from the current working tree only.
> >
> > Bikeshedding: I wonder if this should be named "--this-worktree" or
> > "--this-worktree-only" or if it should somehow be orthogonal to --all
> > rather than modifying it. (Genuine questions. I don't have the
> > answers.)
>
> It follows a precedent (made by me :p) which is rev-list
> --single-worktree. I doubt that option is widely used though so we
> could still rename it if there's a better name. I made
> --single-worktree to contrast "all worktrees" by default. Even if it's
> "this/current worktree" it still has to somehow say "everything in
> this worktree" so I felt modifying --all was a good idea.

Precedent overrides bikeshedding, so leaving it as-is is fine.

Re: [PATCH 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

>> > The main worktree has to be treated specially because well.. it's
>> > special from the beginning. So HEAD from the main worktree is
>> > acccessible via the name "main/HEAD" (we can't use
>> > "worktrees/main/HEAD" because "main" under "worktrees" is not
>> > reserved).
>>
>> I do not quite follow.  So with this, both refs/heads/master and
>> main/refs/heads/master are good names for the master branch (even
>> though the local branch names are not per worktree), because
>> in the main worktree, refs/bisect/bad and main/refs/bisect/bad ought
>> to mean the same thing.
>
> True. I think the ambiguation here is about the main worktree versus a
> secondary worktree that is accidentally named "main". Then suddenly we
> have to worktrees of the same name, and accessing them both via
> worktrees/<id>/HEAD will not work, and there is no other way to
> disambiguate them.

So those who have happily been referring 'refs/heads/main/foo' as
'main/foo' now suddenly have to say 'refs/heads/main/foo' instead?

> The rules are not touched. But it looks like everything still works as
> expected (I'm adding tests to verify this)

What I am worried about is that _your_ expectation may not coincide
with the expectations of users, especially with ones with existing
refs that overlap with the namespaces this series suddenly starts
carving out and squatting on.  As long as that won't be a problem, I
think it is OK, even with 'main' not renamed to 'worktree-main' or
somesuch.

Re: [PATCH v2 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 2dd77f9485..9ca2a3706c 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> ...
>  	case REF_TYPE_PSEUDOREF:
>  		strbuf_addf(sb, "%s/logs/%s", refs->gitdir, refname);
>  		break;
> +	case REF_TYPE_OTHER_PSEUDOREF:
> +		return files_reflog_path_other_worktrees(refs, sb, refname);
> +	case REF_TYPE_MAIN_PSEUDOREF:
> +		if (!skip_prefix(refname, "main-worktree/", &refname))
> +			BUG("ref %s is not a main pseudoref", refname);
> +		/* passthru */

Correct spelling of the colloquial phrase is fallthru, but see
1cf01a34 ("consistently use "fallthrough" comments in switches",
2017-09-21) that encourages use of "fallthrough" fully spelled out.

Otherwise you'd see something like this.

    CC refs/files-backend.o
refs/files-backend.c: In function 'files_ref_path':
refs/files-backend.c:203:6: error: this statement may fall through [-Werror=implicit-fallthrough=]
   if (!skip_prefix(refname, "main-worktree/", &refname))
      ^
refs/files-backend.c:206:2: note: here
  case REF_TYPE_OTHER_PSEUDOREF:
  ^~~~
refs/files-backend.c: In function 'files_reflog_path':
refs/files-backend.c:181:6: error: this statement may fall through [-Werror=implicit-fallthrough=]
   if (!skip_prefix(refname, "main-worktree/", &refname))
      ^
refs/files-backend.c:184:2: note: here
  case REF_TYPE_NORMAL:
  ^~~~
cc1: all warnings being treated as errors
Makefile:2289: recipe for target 'refs/files-backend.o' failed
make: *** [refs/files-backend.o] Error 1

[PATCH v3 0/8] fix per-worktree ref iteration in fsck/reflog expire

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

v3 changes

- fix incorrect ref reporting (it reported main/HEAD instead of
  main-worktree/HEAD)
- other document typos
- fix strbuf_worktree_ref() producing refs that cannot be handled by
  files-backend.c, e.g. worktrees/foo/refs/heads/master (one day the
  ref store will, but not now)
- make sure the "HEAD" special case in reflog expire still works
  with main-worktree/HEAD and worktrees/xxx/HEAD
- tests added for reflog

Elijah Newren (1):
  fsck: Move fsck_head_link() to get_default_heads() to avoid some
    globals

Nguyễn Thái Ngọc Duy (7):
  refs.c: indent with tabs, not spaces
  Add a place for (not) sharing stuff between worktrees
  refs: new ref types to make per-worktree refs visible to all worktrees
  revision.c: correct a parameter name
  revision.c: better error reporting on ref from different worktrees
  fsck: check HEAD and reflog from other worktrees
  reflog expire: cover reflog from all worktrees

 Documentation/git-reflog.txt           |  7 ++-
 Documentation/git-worktree.txt         | 32 ++++++++++-
 Documentation/gitrepository-layout.txt | 11 +++-
 builtin/fsck.c                         | 68 +++++++++++++++-------
 builtin/reflog.c                       | 46 +++++++++++++--
 path.c                                 |  2 +
 refs.c                                 | 24 +++++++-
 refs.h                                 |  8 ++-
 refs/files-backend.c                   | 42 +++++++++++++-
 revision.c                             | 22 ++++---
 t/t0060-path-utils.sh                  |  2 +
 t/t1410-reflog.sh                      | 15 +++++
 t/t1415-worktree-refs.sh               | 79 ++++++++++++++++++++++++++
 t/t1450-fsck.sh                        | 35 ++++++++++++
 worktree.c                             | 79 +++++++++++++++++++++++++-
 worktree.h                             | 24 ++++++++
 16 files changed, 449 insertions(+), 47 deletions(-)
 create mode 100755 t/t1415-worktree-refs.sh

Range-diff against v2:
1:  328a4d1263 ! 1:  fff4cfcc93 refs: new ref types to make per-worktree refs visible to all worktrees
    @@ -24,7 +24,7 @@
         "blah". This syntax coincidentally matches the underlying directory
         structure which makes implementation a bit easier.
     
    -    The main worktree has to be treated specially because well.. it's
    +    The main worktree has to be treated specially because well... it's
         special from the beginning. So HEAD from the main worktree is
         acccessible via the name "main-worktree/HEAD" instead of
         "worktrees/main/HEAD" because "main" could be just another secondary
    @@ -53,9 +53,9 @@
      shared.
      
     +Refs that are per working tree can still be accessed from another
    -+working tree via two special paths main-worktree and worktrees. The
    ++working tree via two special paths, main-worktree and worktrees. The
     +former gives access to per-worktree refs of the main working tree,
    -+while the former to all linked working trees.
    ++while the latter to all linked working trees.
     +
     +For example, main-worktree/HEAD or main-worktree/refs/bisect/good
     +resolve to the same value as the main working tree's HEAD and
    @@ -128,6 +128,14 @@
      diff --git a/refs/files-backend.c b/refs/files-backend.c
      --- a/refs/files-backend.c
      +++ b/refs/files-backend.c
    +@@
    + #include "../object.h"
    + #include "../dir.h"
    + #include "../chdir-notify.h"
    ++#include "worktree.h"
    + 
    + /*
    +  * This backend uses the following flags in `ref_update::flags` for
     @@
      	return refs;
      }
    @@ -137,16 +145,18 @@
     +					      const char *refname)
     +{
     +	const char *real_ref;
    ++	const char *worktree_name;
    ++	int length;
     +
    -+	if (!skip_prefix(refname, "worktrees/", &real_ref))
    -+		BUG("refname %s is not a other-worktree ref", refname);
    -+	real_ref = strchr(real_ref, '/');
    -+	if (!real_ref)
    ++	if (parse_worktree_ref(refname, &worktree_name, &length, &real_ref))
     +		BUG("refname %s is not a other-worktree ref", refname);
    -+	real_ref++;
     +
    -+	strbuf_addf(sb, "%s/%.*slogs/%s", refs->gitcommondir,
    -+		    (int)(real_ref - refname), refname, real_ref);
    ++	if (worktree_name)
    ++		strbuf_addf(sb, "%s/worktrees/%.*s/logs/%s", refs->gitcommondir,
    ++			    length, worktree_name, real_ref);
    ++	else
    ++		strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir,
    ++			    real_ref);
     +}
     +
      static void files_reflog_path(struct files_ref_store *refs,
    @@ -157,11 +167,8 @@
      		strbuf_addf(sb, "%s/logs/%s", refs->gitdir, refname);
      		break;
     +	case REF_TYPE_OTHER_PSEUDOREF:
    -+		return files_reflog_path_other_worktrees(refs, sb, refname);
     +	case REF_TYPE_MAIN_PSEUDOREF:
    -+		if (!skip_prefix(refname, "main-worktree/", &refname))
    -+			BUG("ref %s is not a main pseudoref", refname);
    -+		/* passthru */
    ++		return files_reflog_path_other_worktrees(refs, sb, refname);
      	case REF_TYPE_NORMAL:
      		strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir, refname);
      		break;
    @@ -172,7 +179,7 @@
     +	case REF_TYPE_MAIN_PSEUDOREF:
     +		if (!skip_prefix(refname, "main-worktree/", &refname))
     +			BUG("ref %s is not a main pseudoref", refname);
    -+		/* passthru */
    ++		/* fallthrough */
     +	case REF_TYPE_OTHER_PSEUDOREF:
      	case REF_TYPE_NORMAL:
      		strbuf_addf(sb, "%s/%s", refs->gitcommondir, refname);
    @@ -193,9 +200,9 @@
     +
     +test_expect_success 'ambiguous main-worktree/HEAD' '
     +	mkdir -p .git/refs/heads/main-worktree &&
    -+	test_when_finished rm .git/refs/heads/main-worktree/HEAD &&
    ++	test_when_finished rm -f .git/refs/heads/main-worktree/HEAD &&
     +	cp .git/HEAD .git/refs/heads/main-worktree/HEAD &&
    -+	git rev-parse main-worktree/HEAD 2>warn >/dev/null &&
    ++	git rev-parse main-worktree/HEAD 2>warn &&
     +	grep "main-worktree/HEAD.*ambiguous" warn
     +'
     +
    @@ -207,9 +214,9 @@
     +
     +test_expect_success 'ambiguous worktrees/xx/HEAD' '
     +	mkdir -p .git/refs/heads/worktrees/wt1 &&
    -+	test_when_finished rm .git/refs/heads/worktrees/wt1/HEAD &&
    ++	test_when_finished rm -f .git/refs/heads/worktrees/wt1/HEAD &&
     +	cp .git/HEAD .git/refs/heads/worktrees/wt1/HEAD &&
    -+	git rev-parse worktrees/wt1/HEAD 2>warn >/dev/null &&
    ++	git rev-parse worktrees/wt1/HEAD 2>warn &&
     +	grep "worktrees/wt1/HEAD.*ambiguous" warn
     +'
     +
    @@ -232,3 +239,62 @@
     +'
     +
      test_done
    +
    + diff --git a/worktree.c b/worktree.c
    + --- a/worktree.c
    + +++ b/worktree.c
    +@@
    + 	return ret;
    + }
    + 
    ++int parse_worktree_ref(const char *worktree_ref, const char **name,
    ++		       int *name_length, const char **ref)
    ++{
    ++	if (skip_prefix(worktree_ref, "main-worktree/", &worktree_ref)) {
    ++		if (!*worktree_ref)
    ++			return -1;
    ++		if (name)
    ++			*name = NULL;
    ++		if (name_length)
    ++			*name_length = 0;
    ++		if (ref)
    ++			*ref = worktree_ref;
    ++		return 0;
    ++	}
    ++	if (skip_prefix(worktree_ref, "worktrees/", &worktree_ref)) {
    ++		const char *slash = strchr(worktree_ref, '/');
    ++
    ++		if (!slash || slash == worktree_ref || !slash[1])
    ++			return -1;
    ++		if (name)
    ++			*name = worktree_ref;
    ++		if (name_length)
    ++			*name_length = slash - worktree_ref;
    ++		if (ref)
    ++			*ref = slash + 1;
    ++		return 0;
    ++	}
    ++	return -1;
    ++}
    ++
    + int other_head_refs(each_ref_fn fn, void *cb_data)
    + {
    + 	struct worktree **worktrees, **p;
    +
    + diff --git a/worktree.h b/worktree.h
    + --- a/worktree.h
    + +++ b/worktree.h
    +@@
    + 				     const char *fmt, ...)
    + 	__attribute__((format (printf, 2, 3)));
    + 
    ++/*
    ++ * Parse a worktree ref (i.e. with prefix main-worktree/ or
    ++ * worktrees/) and return the position of the worktree's name and
    ++ * length (or NULL and zero if it's main worktree), and ref.
    ++ *
    ++ * All name, name_length and ref arguments could be NULL.
    ++ */
    ++int parse_worktree_ref(const char *worktree_ref, const char **name,
    ++		       int *name_length, const char **ref);
    + #endif
2:  ffdd30f7fc = 2:  e936a0af1e revision.c: correct a parameter name
3:  6809bab191 ! 3:  382c645d73 revision.c: better error reporting on ref from different worktrees
    @@ -87,18 +87,35 @@
      --- a/worktree.c
      +++ b/worktree.c
     @@
    - 	return ret;
    + 	return -1;
      }
      
     +void strbuf_worktree_ref(const struct worktree *wt,
     +			 struct strbuf *sb,
     +			 const char *refname)
     +{
    -+	if (wt && !wt->is_current) {
    -+		if (is_main_worktree(wt))
    -+			strbuf_addstr(sb, "main/");
    -+		else
    -+			strbuf_addf(sb, "worktrees/%s/", wt->id);
    ++	switch (ref_type(refname)) {
    ++	case REF_TYPE_PSEUDOREF:
    ++	case REF_TYPE_PER_WORKTREE:
    ++		if (wt && !wt->is_current) {
    ++			if (is_main_worktree(wt))
    ++				strbuf_addstr(sb, "main-worktree/");
    ++			else
    ++				strbuf_addf(sb, "worktrees/%s/", wt->id);
    ++		}
    ++		break;
    ++
    ++	case REF_TYPE_MAIN_PSEUDOREF:
    ++	case REF_TYPE_OTHER_PSEUDOREF:
    ++		break;
    ++
    ++	case REF_TYPE_NORMAL:
    ++		/*
    ++		 * For shared refs, don't prefix worktrees/ or
    ++		 * main-worktree/. It's not necessary and
    ++		 * files-backend.c can't handle it anyway.
    ++		 */
    ++		break;
     +	}
     +	strbuf_addstr(sb, refname);
     +}
    @@ -141,9 +158,10 @@
      --- a/worktree.h
      +++ b/worktree.h
     @@
    - 				     const char *fmt, ...)
    - 	__attribute__((format (printf, 2, 3)));
    - 
    +  */
    + int parse_worktree_ref(const char *worktree_ref, const char **name,
    + 		       int *name_length, const char **ref);
    ++
     +/*
     + * Return a refname suitable for access from the current ref store.
     + */
    @@ -153,7 +171,7 @@
     +
     +/*
     + * Return a refname suitable for access from the current ref
    -+ * store. The result may be destroyed at the next call.
    ++ * store. The result will be destroyed at the next call.
     + */
     +const char *worktree_ref(const struct worktree *wt,
     +			 const char *refname);
4:  2e13fa8361 = 4:  e2b99ef955 fsck: Move fsck_head_link() to get_default_heads() to avoid some globals
5:  65f1547df3 ! 5:  8fbff370c3 fsck: check HEAD and reflog from other worktrees
    @@ -136,7 +136,7 @@
     +	echo $ZERO_OID >.git/HEAD &&
     +	# avoid corrupt/broken HEAD from interfering with repo discovery
     +	test_must_fail git -C wt fsck 2>out &&
    -+	grep "main/HEAD: detached HEAD points" out
    ++	grep "main-worktree/HEAD: detached HEAD points" out
     +'
     +
     +test_expect_success 'other worktree HEAD link pointing at a funny object' '
6:  86326b44b5 ! 6:  2cd501f2ce reflog expire: cover reflog from all worktrees
    @@ -48,12 +48,48 @@
      };
      
      /* Remember to update object flag allocation in object.h */
    +@@
    + 	return 0;
    + }
    + 
    ++static int is_head(const char *refname)
    ++{
    ++	switch (ref_type(refname)) {
    ++	case REF_TYPE_OTHER_PSEUDOREF:
    ++	case REF_TYPE_MAIN_PSEUDOREF:
    ++		if (parse_worktree_ref(refname, NULL, NULL, &refname))
    ++			BUG("not a worktree ref: %s", refname);
    ++		break;
    ++	default:
    ++		break;
    ++	}
    ++	return !strcmp(refname, "HEAD");
    ++}
    ++
    + static void reflog_expiry_prepare(const char *refname,
    + 				  const struct object_id *oid,
    + 				  void *cb_data)
    + {
    + 	struct expire_reflog_policy_cb *cb = cb_data;
    + 
    +-	if (!cb->cmd.expire_unreachable || !strcmp(refname, "HEAD")) {
    ++	if (!cb->cmd.expire_unreachable || is_head(refname)) {
    + 		cb->tip_commit = NULL;
    + 		cb->unreachable_expire_kind = UE_HEAD;
    + 	} else {
     @@
      {
      	struct collected_reflog *e;
      	struct collect_reflog_cb *cb = cb_data;
     +	struct strbuf newref = STRBUF_INIT;
     +
    ++	/*
    ++	 * Avoid collecting the same shared ref multiple times because
    ++	 * they are available via all worktrees.
    ++	 */
    ++	if (!cb->wt->is_current && ref_type(ref) == REF_TYPE_NORMAL)
    ++		return 0;
    ++
     +	strbuf_worktree_ref(cb->wt, &newref, ref);
     +	FLEX_ALLOC_STR(e, reflog, newref.buf);
     +	strbuf_release(&newref);
    @@ -94,9 +130,34 @@
     +			if (!all_worktrees && !(*p)->is_current)
     +				continue;
     +			collected.wt = *p;
    -+			for_each_reflog(collect_reflog, &collected);
    ++			refs_for_each_reflog(get_worktree_ref_store(*p),
    ++					     collect_reflog, &collected);
     +		}
     +		free_worktrees(worktrees);
      		for (i = 0; i < collected.nr; i++) {
      			struct collected_reflog *e = collected.e[i];
      			set_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog);
    +
    + diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
    + --- a/t/t1410-reflog.sh
    + +++ b/t/t1410-reflog.sh
    +@@
    + 	)
    + '
    + 
    ++test_expect_success 'expire with multiple worktrees' '
    ++	git init main-wt &&
    ++	(
    ++		cd main-wt &&
    ++		test_tick &&
    ++		test_commit foo &&
    ++		git  worktree add link-wt &&
    ++		test_tick &&
    ++		test_commit -C link-wt foobar &&
    ++		test_tick &&
    ++		git reflog expire --verbose --all --expire=$test_tick &&
    ++		test_must_be_empty .git/worktrees/link-wt/logs/HEAD
    ++	)
    ++'
    ++
    + test_done
-- 
2.19.1.647.g708186aaf9

[PATCH v3 1/8] refs.c: indent with tabs, not spaces

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

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index bbcac921b6..f07c775b50 100644
--- a/refs.c
+++ b/refs.c
@@ -646,7 +646,7 @@ enum ref_type ref_type(const char *refname)
 		return REF_TYPE_PER_WORKTREE;
 	if (is_pseudoref_syntax(refname))
 		return REF_TYPE_PSEUDOREF;
-       return REF_TYPE_NORMAL;
+	return REF_TYPE_NORMAL;
 }
 
 long get_files_ref_lock_timeout_ms(void)
-- 
2.19.1.647.g708186aaf9

[PATCH v3 2/8] Add a place for (not) sharing stuff between worktrees

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

When multiple worktrees are used, we need rules to determine if
something belongs to one worktree or all of them. Instead of keeping
adding rules when new stuff comes (*), have a generic rule:

- Inside $GIT_DIR, which is per-worktree by default, add
  $GIT_DIR/common which is always shared. New features that want to
  share stuff should put stuff under this directory.

- Inside refs/, which is shared by default except refs/bisect, add
  refs/worktree/ which is per-worktree. We may eventually move
  refs/bisect to this new location and remove the exception in refs
  code.

(*) And it may also include stuff from external commands which will
    have no way to modify common/per-worktree rules.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-worktree.txt         | 19 ++++++++++++++-
 Documentation/gitrepository-layout.txt | 11 +++++++--
 path.c                                 |  2 ++
 refs.c                                 |  1 +
 refs/files-backend.c                   | 14 ++++++++---
 t/t0060-path-utils.sh                  |  2 ++
 t/t1415-worktree-refs.sh               | 33 ++++++++++++++++++++++++++
 7 files changed, 76 insertions(+), 6 deletions(-)
 create mode 100755 t/t1415-worktree-refs.sh

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index e2ee9fc21b..a50fbf8094 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -204,6 +204,22 @@ working trees, it can be used to identify worktrees. For example if
 you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
 then "ghi" or "def/ghi" is enough to point to the former working tree.
 
+REFS
+----
+In multiple working trees, some refs may be shared between all working
+trees, some refs are local. One example is HEAD is different for all
+working trees. This section is about the sharing rules.
+
+In general, all pseudo refs are per working tree and all refs starting
+with "refs/" are shared. Pseudo refs are ones like HEAD which are
+directly under GIT_DIR instead of inside GIT_DIR/refs. There are one
+exception to this: refs inside refs/bisect and refs/worktree is not
+shared.
+
+To access refs, it's best not to look inside GIT_DIR directly. Instead
+use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1]
+which will handle refs correctly.
+
 DETAILS
 -------
 Each linked working tree has a private sub-directory in the repository's
@@ -228,7 +244,8 @@ linked working tree `git rev-parse --git-path HEAD` returns
 `/path/other/test-next/.git/HEAD` or `/path/main/.git/HEAD`) while `git
 rev-parse --git-path refs/heads/master` uses
 $GIT_COMMON_DIR and returns `/path/main/.git/refs/heads/master`,
-since refs are shared across all working trees.
+since refs are shared across all working trees, except refs/bisect and
+refs/worktree.
 
 See linkgit:gitrepository-layout[5] for more information. The rule of
 thumb is do not make any assumption about whether a path belongs to
diff --git a/Documentation/gitrepository-layout.txt b/Documentation/gitrepository-layout.txt
index e85148f05e..89b616e049 100644
--- a/Documentation/gitrepository-layout.txt
+++ b/Documentation/gitrepository-layout.txt
@@ -95,8 +95,10 @@ refs::
 	References are stored in subdirectories of this
 	directory.  The 'git prune' command knows to preserve
 	objects reachable from refs found in this directory and
-	its subdirectories. This directory is ignored if $GIT_COMMON_DIR
-	is set and "$GIT_COMMON_DIR/refs" will be used instead.
+	its subdirectories.
+	This directory is ignored (except refs/bisect and
+	refs/worktree) if $GIT_COMMON_DIR is set and
+	"$GIT_COMMON_DIR/refs" will be used instead.
 
 refs/heads/`name`::
 	records tip-of-the-tree commit objects of branch `name`
@@ -165,6 +167,11 @@ hooks::
 	each hook. This directory is ignored if $GIT_COMMON_DIR is set
 	and "$GIT_COMMON_DIR/hooks" will be used instead.
 
+common::
+	When multiple working trees are used, most of files in
+	$GIT_DIR are per-worktree with a few known exceptions. All
+	files under 'common' however will be shared between all
+	working trees.
 
 index::
 	The current index file for the repository.  It is
diff --git a/path.c b/path.c
index 34f0f98349..bf4bb02a27 100644
--- a/path.c
+++ b/path.c
@@ -108,6 +108,7 @@ struct common_dir {
 
 static struct common_dir common_list[] = {
 	{ 0, 1, 0, "branches" },
+	{ 0, 1, 0, "common" },
 	{ 0, 1, 0, "hooks" },
 	{ 0, 1, 0, "info" },
 	{ 0, 0, 1, "info/sparse-checkout" },
@@ -118,6 +119,7 @@ static struct common_dir common_list[] = {
 	{ 0, 1, 0, "objects" },
 	{ 0, 1, 0, "refs" },
 	{ 0, 1, 1, "refs/bisect" },
+	{ 0, 1, 1, "refs/worktree" },
 	{ 0, 1, 0, "remotes" },
 	{ 0, 1, 0, "worktrees" },
 	{ 0, 1, 0, "rr-cache" },
diff --git a/refs.c b/refs.c
index f07c775b50..67daf0918e 100644
--- a/refs.c
+++ b/refs.c
@@ -624,6 +624,7 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)
 static int is_per_worktree_ref(const char *refname)
 {
 	return !strcmp(refname, "HEAD") ||
+		starts_with(refname, "refs/worktree/") ||
 		starts_with(refname, "refs/bisect/") ||
 		starts_with(refname, "refs/rewritten/");
 }
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 16ef9325e0..2dd77f9485 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -269,9 +269,9 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
 	closedir(d);
 
 	/*
-	 * Manually add refs/bisect, which, being per-worktree, might
-	 * not appear in the directory listing for refs/ in the main
-	 * repo.
+	 * 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);
@@ -281,6 +281,14 @@ static void loose_fill_ref_dir(struct ref_store *ref_store,
 					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);
+		}
 	}
 }
 
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index cd74c0a471..c7b53e494b 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -306,6 +306,8 @@ test_git_path GIT_COMMON_DIR=bar hooks/me                 bar/hooks/me
 test_git_path GIT_COMMON_DIR=bar config                   bar/config
 test_git_path GIT_COMMON_DIR=bar packed-refs              bar/packed-refs
 test_git_path GIT_COMMON_DIR=bar shallow                  bar/shallow
+test_git_path GIT_COMMON_DIR=bar common                   bar/common
+test_git_path GIT_COMMON_DIR=bar common/file              bar/common/file
 
 # In the tests below, $(pwd) must be used because it is a native path on
 # Windows and avoids MSYS's path mangling (which simplifies "foo/../bar" and
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
new file mode 100755
index 0000000000..16c91bef57
--- /dev/null
+++ b/t/t1415-worktree-refs.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+
+test_description='per-worktree refs'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	test_commit initial &&
+	test_commit wt1 &&
+	test_commit wt2 &&
+	git worktree add wt1 wt1 &&
+	git worktree add wt2 wt2 &&
+	git checkout initial &&
+	git update-ref refs/worktree/foo HEAD &&
+	git -C wt1 update-ref refs/worktree/foo HEAD &&
+	git -C wt2 update-ref refs/worktree/foo HEAD
+'
+
+test_expect_success 'refs/worktree must not be packed' '
+	git pack-refs --all &&
+	test_path_is_missing .git/refs/tags/wt1 &&
+	test_path_is_file .git/refs/worktree/foo &&
+	test_path_is_file .git/worktrees/wt1/refs/worktree/foo &&
+	test_path_is_file .git/worktrees/wt2/refs/worktree/foo
+'
+
+test_expect_success 'refs/worktree are per-worktree' '
+	test_cmp_rev worktree/foo initial &&
+	( cd wt1 && test_cmp_rev worktree/foo wt1 ) &&
+	( cd wt2 && test_cmp_rev worktree/foo wt2 )
+'
+
+test_done
-- 
2.19.1.647.g708186aaf9

[PATCH v3 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

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

One of the problems with multiple worktree is accessing per-worktree
refs of one worktree from another worktree. This was sort of solved by
multiple ref store, where the code can open the ref store of another
worktree and has access to the ref space of that worktree.

The problem with this is reporting. "HEAD" in another ref space is
also called "HEAD" like in the current ref space. In order to
differentiate them, all the code must somehow carry the ref store
around and print something like "HEAD from this ref store".

But that is not feasible (or possible with a _lot_ of work). With the
current design, we pass a reference around as a string (so called
"refname"). Extending this design to pass a string _and_ a ref store
is a nightmare, especially when handling extended SHA-1 syntax.

So we do it another way. Instead of entering a separate ref space, we
make refs from other worktrees available in the current ref space. So
"HEAD" is always HEAD of the current worktree, but then we can have
"worktrees/blah/HEAD" to denote HEAD from a worktree named
"blah". This syntax coincidentally matches the underlying directory
structure which makes implementation a bit easier.

The main worktree has to be treated specially because well... it's
special from the beginning. So HEAD from the main worktree is
acccessible via the name "main-worktree/HEAD" instead of
"worktrees/main/HEAD" because "main" could be just another secondary
worktree.

This patch also makes it possible to specify refs from one worktree in
another one, e.g.

    git log worktrees/foo/HEAD

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-worktree.txt | 15 ++++++++++-
 refs.c                         | 21 ++++++++++++++++
 refs.h                         |  8 +++---
 refs/files-backend.c           | 28 +++++++++++++++++++++
 t/t1415-worktree-refs.sh       | 46 ++++++++++++++++++++++++++++++++++
 worktree.c                     | 30 ++++++++++++++++++++++
 worktree.h                     |  9 +++++++
 7 files changed, 153 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index a50fbf8094..9117e4fb50 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -208,7 +208,8 @@ REFS
 ----
 In multiple working trees, some refs may be shared between all working
 trees, some refs are local. One example is HEAD is different for all
-working trees. This section is about the sharing rules.
+working trees. This section is about the sharing rules and how to access
+refs of one working tree from another.
 
 In general, all pseudo refs are per working tree and all refs starting
 with "refs/" are shared. Pseudo refs are ones like HEAD which are
@@ -216,6 +217,18 @@ directly under GIT_DIR instead of inside GIT_DIR/refs. There are one
 exception to this: refs inside refs/bisect and refs/worktree is not
 shared.
 
+Refs that are per working tree can still be accessed from another
+working tree via two special paths, main-worktree and worktrees. The
+former gives access to per-worktree refs of the main working tree,
+while the latter to all linked working trees.
+
+For example, main-worktree/HEAD or main-worktree/refs/bisect/good
+resolve to the same value as the main working tree's HEAD and
+refs/bisect/good respectively. Similarly, worktrees/foo/HEAD or
+worktrees/bar/refs/bisect/bad are the same as
+GIT_COMMON_DIR/worktrees/foo/HEAD and
+GIT_COMMON_DIR/worktrees/bar/refs/bisect/bad.
+
 To access refs, it's best not to look inside GIT_DIR directly. Instead
 use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1]
 which will handle refs correctly.
diff --git a/refs.c b/refs.c
index 67daf0918e..17e4307f31 100644
--- a/refs.c
+++ b/refs.c
@@ -641,12 +641,33 @@ static int is_pseudoref_syntax(const char *refname)
 	return 1;
 }
 
+static int is_main_pseudoref_syntax(const char *refname)
+{
+	return skip_prefix(refname, "main-worktree/", &refname) &&
+		*refname &&
+		is_pseudoref_syntax(refname);
+}
+
+static int is_other_pseudoref_syntax(const char *refname)
+{
+	if (!skip_prefix(refname, "worktrees/", &refname))
+		return 0;
+	refname = strchr(refname, '/');
+	if (!refname || !refname[1])
+		return 0;
+	return is_pseudoref_syntax(refname + 1);
+}
+
 enum ref_type ref_type(const char *refname)
 {
 	if (is_per_worktree_ref(refname))
 		return REF_TYPE_PER_WORKTREE;
 	if (is_pseudoref_syntax(refname))
 		return REF_TYPE_PSEUDOREF;
+	if (is_main_pseudoref_syntax(refname))
+		return REF_TYPE_MAIN_PSEUDOREF;
+	if (is_other_pseudoref_syntax(refname))
+		return REF_TYPE_OTHER_PSEUDOREF;
 	return REF_TYPE_NORMAL;
 }
 
diff --git a/refs.h b/refs.h
index 6cc0397679..308fa1f03b 100644
--- a/refs.h
+++ b/refs.h
@@ -714,9 +714,11 @@ int parse_hide_refs_config(const char *var, const char *value, const char *);
 int ref_is_hidden(const char *, const char *);
 
 enum ref_type {
-	REF_TYPE_PER_WORKTREE,
-	REF_TYPE_PSEUDOREF,
-	REF_TYPE_NORMAL,
+	REF_TYPE_PER_WORKTREE,	  /* refs inside refs/ but not shared       */
+	REF_TYPE_PSEUDOREF,	  /* refs outside refs/ in current worktree */
+	REF_TYPE_MAIN_PSEUDOREF,  /* pseudo refs from the main worktree     */
+	REF_TYPE_OTHER_PSEUDOREF, /* pseudo refs from other worktrees       */
+	REF_TYPE_NORMAL,	  /* normal/shared refs inside refs/        */
 };
 
 enum ref_type ref_type(const char *refname);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 2dd77f9485..9183875dad 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -10,6 +10,7 @@
 #include "../object.h"
 #include "../dir.h"
 #include "../chdir-notify.h"
+#include "worktree.h"
 
 /*
  * This backend uses the following flags in `ref_update::flags` for
@@ -149,6 +150,25 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store,
 	return refs;
 }
 
+static void files_reflog_path_other_worktrees(struct files_ref_store *refs,
+					      struct strbuf *sb,
+					      const char *refname)
+{
+	const char *real_ref;
+	const char *worktree_name;
+	int length;
+
+	if (parse_worktree_ref(refname, &worktree_name, &length, &real_ref))
+		BUG("refname %s is not a other-worktree ref", refname);
+
+	if (worktree_name)
+		strbuf_addf(sb, "%s/worktrees/%.*s/logs/%s", refs->gitcommondir,
+			    length, worktree_name, real_ref);
+	else
+		strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir,
+			    real_ref);
+}
+
 static void files_reflog_path(struct files_ref_store *refs,
 			      struct strbuf *sb,
 			      const char *refname)
@@ -158,6 +178,9 @@ static void files_reflog_path(struct files_ref_store *refs,
 	case REF_TYPE_PSEUDOREF:
 		strbuf_addf(sb, "%s/logs/%s", refs->gitdir, refname);
 		break;
+	case REF_TYPE_OTHER_PSEUDOREF:
+	case REF_TYPE_MAIN_PSEUDOREF:
+		return files_reflog_path_other_worktrees(refs, sb, refname);
 	case REF_TYPE_NORMAL:
 		strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir, refname);
 		break;
@@ -176,6 +199,11 @@ static void files_ref_path(struct files_ref_store *refs,
 	case REF_TYPE_PSEUDOREF:
 		strbuf_addf(sb, "%s/%s", refs->gitdir, refname);
 		break;
+	case REF_TYPE_MAIN_PSEUDOREF:
+		if (!skip_prefix(refname, "main-worktree/", &refname))
+			BUG("ref %s is not a main pseudoref", refname);
+		/* fallthrough */
+	case REF_TYPE_OTHER_PSEUDOREF:
 	case REF_TYPE_NORMAL:
 		strbuf_addf(sb, "%s/%s", refs->gitcommondir, refname);
 		break;
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
index 16c91bef57..b664e51250 100755
--- a/t/t1415-worktree-refs.sh
+++ b/t/t1415-worktree-refs.sh
@@ -30,4 +30,50 @@ test_expect_success 'refs/worktree are per-worktree' '
 	( cd wt2 && test_cmp_rev worktree/foo wt2 )
 '
 
+test_expect_success 'resolve main-worktree/HEAD' '
+	test_cmp_rev main-worktree/HEAD initial &&
+	( cd wt1 && test_cmp_rev main-worktree/HEAD initial ) &&
+	( cd wt2 && test_cmp_rev main-worktree/HEAD initial )
+'
+
+test_expect_success 'ambiguous main-worktree/HEAD' '
+	mkdir -p .git/refs/heads/main-worktree &&
+	test_when_finished rm -f .git/refs/heads/main-worktree/HEAD &&
+	cp .git/HEAD .git/refs/heads/main-worktree/HEAD &&
+	git rev-parse main-worktree/HEAD 2>warn &&
+	grep "main-worktree/HEAD.*ambiguous" warn
+'
+
+test_expect_success 'resolve worktrees/xx/HEAD' '
+	test_cmp_rev worktrees/wt1/HEAD wt1 &&
+	( cd wt1 && test_cmp_rev worktrees/wt1/HEAD wt1 ) &&
+	( cd wt2 && test_cmp_rev worktrees/wt1/HEAD wt1 )
+'
+
+test_expect_success 'ambiguous worktrees/xx/HEAD' '
+	mkdir -p .git/refs/heads/worktrees/wt1 &&
+	test_when_finished rm -f .git/refs/heads/worktrees/wt1/HEAD &&
+	cp .git/HEAD .git/refs/heads/worktrees/wt1/HEAD &&
+	git rev-parse worktrees/wt1/HEAD 2>warn &&
+	grep "worktrees/wt1/HEAD.*ambiguous" warn
+'
+
+test_expect_success 'reflog of main-worktree/HEAD' '
+	git reflog HEAD | sed "s/HEAD/main-worktree\/HEAD/" >expected &&
+	git reflog main-worktree/HEAD >actual &&
+	test_cmp expected actual &&
+	git -C wt1 reflog main-worktree/HEAD >actual.wt1 &&
+	test_cmp expected actual.wt1
+'
+
+test_expect_success 'reflog of worktrees/xx/HEAD' '
+	git -C wt2 reflog HEAD | sed "s/HEAD/worktrees\/wt2\/HEAD/" >expected &&
+	git reflog worktrees/wt2/HEAD >actual &&
+	test_cmp expected actual &&
+	git -C wt1 reflog worktrees/wt2/HEAD >actual.wt1 &&
+	test_cmp expected actual.wt1 &&
+	git -C wt2 reflog worktrees/wt2/HEAD >actual.wt2 &&
+	test_cmp expected actual.wt2
+'
+
 test_done
diff --git a/worktree.c b/worktree.c
index b0d0b5426d..4193496f2a 100644
--- a/worktree.c
+++ b/worktree.c
@@ -487,6 +487,36 @@ int submodule_uses_worktrees(const char *path)
 	return ret;
 }
 
+int parse_worktree_ref(const char *worktree_ref, const char **name,
+		       int *name_length, const char **ref)
+{
+	if (skip_prefix(worktree_ref, "main-worktree/", &worktree_ref)) {
+		if (!*worktree_ref)
+			return -1;
+		if (name)
+			*name = NULL;
+		if (name_length)
+			*name_length = 0;
+		if (ref)
+			*ref = worktree_ref;
+		return 0;
+	}
+	if (skip_prefix(worktree_ref, "worktrees/", &worktree_ref)) {
+		const char *slash = strchr(worktree_ref, '/');
+
+		if (!slash || slash == worktree_ref || !slash[1])
+			return -1;
+		if (name)
+			*name = worktree_ref;
+		if (name_length)
+			*name_length = slash - worktree_ref;
+		if (ref)
+			*ref = slash + 1;
+		return 0;
+	}
+	return -1;
+}
+
 int other_head_refs(each_ref_fn fn, void *cb_data)
 {
 	struct worktree **worktrees, **p;
diff --git a/worktree.h b/worktree.h
index df3fc30f73..440bb219dd 100644
--- a/worktree.h
+++ b/worktree.h
@@ -108,4 +108,13 @@ extern const char *worktree_git_path(const struct worktree *wt,
 				     const char *fmt, ...)
 	__attribute__((format (printf, 2, 3)));
 
+/*
+ * Parse a worktree ref (i.e. with prefix main-worktree/ or
+ * worktrees/) and return the position of the worktree's name and
+ * length (or NULL and zero if it's main worktree), and ref.
+ *
+ * All name, name_length and ref arguments could be NULL.
+ */
+int parse_worktree_ref(const char *worktree_ref, const char **name,
+		       int *name_length, const char **ref);
 #endif
-- 
2.19.1.647.g708186aaf9

[PATCH v3 4/8] revision.c: correct a parameter name

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

This function is a callback of for_each_reflog() which will pass a ref
name as the first argument, not a path (to a reflog file).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/revision.c b/revision.c
index b5108b75ab..ea1d695e23 100644
--- a/revision.c
+++ b/revision.c
@@ -1278,13 +1278,14 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
 	return 0;
 }
 
-static int handle_one_reflog(const char *path, const struct object_id *oid,
+static int handle_one_reflog(const char *refname,
+			     const struct object_id *oid,
 			     int flag, void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
 	cb->warned_bad_reflog = 0;
-	cb->name_for_errormsg = path;
-	refs_for_each_reflog_ent(cb->refs, path,
+	cb->name_for_errormsg = refname;
+	refs_for_each_reflog_ent(cb->refs, refname,
 				 handle_one_reflog_ent, cb_data);
 	return 0;
 }
-- 
2.19.1.647.g708186aaf9

[PATCH v3 5/8] revision.c: better error reporting on ref from different worktrees

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

Make use of the new ref aliases to pass refs from another worktree
around and access them from the current ref store instead. This does
not change any functionality, but when a problem arises, we would like
the reported messages to mention full ref aliases, like this:

    fatal: bad object worktrees/ztemp/HEAD
    warning: reflog of 'main-worktree/HEAD' references pruned commits

instead of

    fatal: bad object HEAD
    warning: reflog of 'HEAD' references pruned commits

which does not really tell where the refs are from.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 revision.c | 21 +++++++++++++--------
 worktree.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++---
 worktree.h | 15 +++++++++++++++
 3 files changed, 74 insertions(+), 11 deletions(-)

diff --git a/revision.c b/revision.c
index ea1d695e23..1737aad3ec 100644
--- a/revision.c
+++ b/revision.c
@@ -1178,7 +1178,7 @@ struct all_refs_cb {
 	int warned_bad_reflog;
 	struct rev_info *all_revs;
 	const char *name_for_errormsg;
-	struct ref_store *refs;
+	struct worktree *wt;
 };
 
 int ref_excluded(struct string_list *ref_excludes, const char *path)
@@ -1215,7 +1215,7 @@ static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs,
 	cb->all_revs = revs;
 	cb->all_flags = flags;
 	revs->rev_input_given = 1;
-	cb->refs = NULL;
+	cb->wt = NULL;
 }
 
 void clear_ref_exclusion(struct string_list **ref_excludes_p)
@@ -1278,15 +1278,20 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
 	return 0;
 }
 
-static int handle_one_reflog(const char *refname,
+static int handle_one_reflog(const char *refname_in_wt,
 			     const struct object_id *oid,
 			     int flag, void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
+	struct strbuf refname = STRBUF_INIT;
+
 	cb->warned_bad_reflog = 0;
-	cb->name_for_errormsg = refname;
-	refs_for_each_reflog_ent(cb->refs, refname,
+	strbuf_worktree_ref(cb->wt, &refname, refname_in_wt);
+	cb->name_for_errormsg = refname.buf;
+	refs_for_each_reflog_ent(get_main_ref_store(the_repository),
+				 refname.buf,
 				 handle_one_reflog_ent, cb_data);
+	strbuf_release(&refname);
 	return 0;
 }
 
@@ -1301,8 +1306,8 @@ static void add_other_reflogs_to_pending(struct all_refs_cb *cb)
 		if (wt->is_current)
 			continue;
 
-		cb->refs = get_worktree_ref_store(wt);
-		refs_for_each_reflog(cb->refs,
+		cb->wt = wt;
+		refs_for_each_reflog(get_worktree_ref_store(wt),
 				     handle_one_reflog,
 				     cb);
 	}
@@ -1315,7 +1320,7 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags)
 
 	cb.all_revs = revs;
 	cb.all_flags = flags;
-	cb.refs = get_main_ref_store(revs->repo);
+	cb.wt = NULL;
 	for_each_reflog(handle_one_reflog, &cb);
 
 	if (!revs->single_worktree)
diff --git a/worktree.c b/worktree.c
index 4193496f2a..e6a65ec684 100644
--- a/worktree.c
+++ b/worktree.c
@@ -517,6 +517,45 @@ int parse_worktree_ref(const char *worktree_ref, const char **name,
 	return -1;
 }
 
+void strbuf_worktree_ref(const struct worktree *wt,
+			 struct strbuf *sb,
+			 const char *refname)
+{
+	switch (ref_type(refname)) {
+	case REF_TYPE_PSEUDOREF:
+	case REF_TYPE_PER_WORKTREE:
+		if (wt && !wt->is_current) {
+			if (is_main_worktree(wt))
+				strbuf_addstr(sb, "main-worktree/");
+			else
+				strbuf_addf(sb, "worktrees/%s/", wt->id);
+		}
+		break;
+
+	case REF_TYPE_MAIN_PSEUDOREF:
+	case REF_TYPE_OTHER_PSEUDOREF:
+		break;
+
+	case REF_TYPE_NORMAL:
+		/*
+		 * For shared refs, don't prefix worktrees/ or
+		 * main-worktree/. It's not necessary and
+		 * files-backend.c can't handle it anyway.
+		 */
+		break;
+	}
+	strbuf_addstr(sb, refname);
+}
+
+const char *worktree_ref(const struct worktree *wt, const char *refname)
+{
+	static struct strbuf sb = STRBUF_INIT;
+
+	strbuf_reset(&sb);
+	strbuf_worktree_ref(wt, &sb, refname);
+	return sb.buf;
+}
+
 int other_head_refs(each_ref_fn fn, void *cb_data)
 {
 	struct worktree **worktrees, **p;
@@ -525,13 +564,17 @@ int other_head_refs(each_ref_fn fn, void *cb_data)
 	worktrees = get_worktrees(0);
 	for (p = worktrees; *p; p++) {
 		struct worktree *wt = *p;
-		struct ref_store *refs;
+		struct object_id oid;
+		int flag;
 
 		if (wt->is_current)
 			continue;
 
-		refs = get_worktree_ref_store(wt);
-		ret = refs_head_ref(refs, fn, cb_data);
+		if (!refs_read_ref_full(get_main_ref_store(the_repository),
+					worktree_ref(wt, "HEAD"),
+					RESOLVE_REF_READING,
+					&oid, &flag))
+			ret = fn(worktree_ref(wt, "HEAD"), &oid, flag, cb_data);
 		if (ret)
 			break;
 	}
diff --git a/worktree.h b/worktree.h
index 440bb219dd..1164ca396f 100644
--- a/worktree.h
+++ b/worktree.h
@@ -117,4 +117,19 @@ extern const char *worktree_git_path(const struct worktree *wt,
  */
 int parse_worktree_ref(const char *worktree_ref, const char **name,
 		       int *name_length, const char **ref);
+
+/*
+ * Return a refname suitable for access from the current ref store.
+ */
+void strbuf_worktree_ref(const struct worktree *wt,
+			 struct strbuf *sb,
+			 const char *refname);
+
+/*
+ * Return a refname suitable for access from the current ref
+ * store. The result will be destroyed at the next call.
+ */
+const char *worktree_ref(const struct worktree *wt,
+			 const char *refname);
+
 #endif
-- 
2.19.1.647.g708186aaf9

[PATCH v3 6/8] fsck: Move fsck_head_link() to get_default_heads() to avoid some globals

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

From: Elijah Newren <newren@gmail.com>

This will make it easier to check the HEAD of other worktrees from fsck.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/fsck.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 06eb421720..0d39462058 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -36,8 +36,6 @@ static int check_strict;
 static int keep_cache_objects;
 static struct fsck_options fsck_walk_options = FSCK_OPTIONS_DEFAULT;
 static struct fsck_options fsck_obj_options = FSCK_OPTIONS_DEFAULT;
-static struct object_id head_oid;
-static const char *head_points_at;
 static int errors_found;
 static int write_lost_and_found;
 static int verbose;
@@ -484,8 +482,15 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,
 	return 0;
 }
 
+static int fsck_head_link(const char **head_points_at,
+			  struct object_id *head_oid);
+
 static void get_default_heads(void)
 {
+	const char *head_points_at;
+	struct object_id head_oid;
+
+	fsck_head_link(&head_points_at, &head_oid);
 	if (head_points_at && !is_null_oid(&head_oid))
 		fsck_handle_ref("HEAD", &head_oid, 0, NULL);
 	for_each_rawref(fsck_handle_ref, NULL);
@@ -579,33 +584,34 @@ static void fsck_object_dir(const char *path)
 	stop_progress(&progress);
 }
 
-static int fsck_head_link(void)
+static int fsck_head_link(const char **head_points_at,
+			  struct object_id *head_oid)
 {
 	int null_is_error = 0;
 
 	if (verbose)
 		fprintf(stderr, "Checking HEAD link\n");
 
-	head_points_at = resolve_ref_unsafe("HEAD", 0, &head_oid, NULL);
-	if (!head_points_at) {
+	*head_points_at = resolve_ref_unsafe("HEAD", 0, head_oid, NULL);
+	if (!*head_points_at) {
 		errors_found |= ERROR_REFS;
 		return error("Invalid HEAD");
 	}
-	if (!strcmp(head_points_at, "HEAD"))
+	if (!strcmp(*head_points_at, "HEAD"))
 		/* detached HEAD */
 		null_is_error = 1;
-	else if (!starts_with(head_points_at, "refs/heads/")) {
+	else if (!starts_with(*head_points_at, "refs/heads/")) {
 		errors_found |= ERROR_REFS;
 		return error("HEAD points to something strange (%s)",
-			     head_points_at);
+			     *head_points_at);
 	}
-	if (is_null_oid(&head_oid)) {
+	if (is_null_oid(head_oid)) {
 		if (null_is_error) {
 			errors_found |= ERROR_REFS;
 			return error("HEAD: detached HEAD points at nothing");
 		}
 		fprintf(stderr, "notice: HEAD points to an unborn branch (%s)\n",
-			head_points_at + 11);
+			*head_points_at + 11);
 	}
 	return 0;
 }
@@ -720,7 +726,6 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 
 	git_config(fsck_config, NULL);
 
-	fsck_head_link();
 	if (connectivity_only) {
 		for_each_loose_object(mark_loose_for_connectivity, NULL, 0);
 		for_each_packed_object(mark_packed_for_connectivity, NULL, 0);
-- 
2.19.1.647.g708186aaf9

[PATCH v3 7/8] fsck: check HEAD and reflog from other worktrees

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

fsck is a repo-wide operation and should check all references no
matter which worktree they are associated to.

Reported-by: Jeff King <peff@peff.net>
Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/fsck.c  | 55 ++++++++++++++++++++++++++++++++++---------------
 t/t1450-fsck.sh | 35 +++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 17 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 0d39462058..3c3e0f06e7 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -19,6 +19,7 @@
 #include "packfile.h"
 #include "object-store.h"
 #include "run-command.h"
+#include "worktree.h"
 
 #define REACHABLE 0x0001
 #define SEEN      0x0002
@@ -444,7 +445,11 @@ static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noid
 static int fsck_handle_reflog(const char *logname, const struct object_id *oid,
 			      int flag, void *cb_data)
 {
-	for_each_reflog_ent(logname, fsck_handle_reflog_ent, (void *)logname);
+	struct strbuf refname = STRBUF_INIT;
+
+	strbuf_worktree_ref(cb_data, &refname, logname);
+	for_each_reflog_ent(refname.buf, fsck_handle_reflog_ent, refname.buf);
+	strbuf_release(&refname);
 	return 0;
 }
 
@@ -482,20 +487,34 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,
 	return 0;
 }
 
-static int fsck_head_link(const char **head_points_at,
+static int fsck_head_link(const char *head_ref_name,
+			  const char **head_points_at,
 			  struct object_id *head_oid);
 
 static void get_default_heads(void)
 {
+	struct worktree **worktrees, **p;
 	const char *head_points_at;
 	struct object_id head_oid;
 
-	fsck_head_link(&head_points_at, &head_oid);
-	if (head_points_at && !is_null_oid(&head_oid))
-		fsck_handle_ref("HEAD", &head_oid, 0, NULL);
 	for_each_rawref(fsck_handle_ref, NULL);
-	if (include_reflogs)
-		for_each_reflog(fsck_handle_reflog, NULL);
+
+	worktrees = get_worktrees(0);
+	for (p = worktrees; *p; p++) {
+		struct worktree *wt = *p;
+		struct strbuf ref = STRBUF_INIT;
+
+		strbuf_worktree_ref(wt, &ref, "HEAD");
+		fsck_head_link(ref.buf, &head_points_at, &head_oid);
+		if (head_points_at && !is_null_oid(&head_oid))
+			fsck_handle_ref(ref.buf, &head_oid, 0, NULL);
+		strbuf_release(&ref);
+
+		if (include_reflogs)
+			refs_for_each_reflog(get_worktree_ref_store(wt),
+					     fsck_handle_reflog, wt);
+	}
+	free_worktrees(worktrees);
 
 	/*
 	 * Not having any default heads isn't really fatal, but
@@ -584,34 +603,36 @@ static void fsck_object_dir(const char *path)
 	stop_progress(&progress);
 }
 
-static int fsck_head_link(const char **head_points_at,
+static int fsck_head_link(const char *head_ref_name,
+			  const char **head_points_at,
 			  struct object_id *head_oid)
 {
 	int null_is_error = 0;
 
 	if (verbose)
-		fprintf(stderr, "Checking HEAD link\n");
+		fprintf(stderr, "Checking %s link\n", head_ref_name);
 
-	*head_points_at = resolve_ref_unsafe("HEAD", 0, head_oid, NULL);
+	*head_points_at = resolve_ref_unsafe(head_ref_name, 0, head_oid, NULL);
 	if (!*head_points_at) {
 		errors_found |= ERROR_REFS;
-		return error("Invalid HEAD");
+		return error("Invalid %s", head_ref_name);
 	}
-	if (!strcmp(*head_points_at, "HEAD"))
+	if (!strcmp(*head_points_at, head_ref_name))
 		/* detached HEAD */
 		null_is_error = 1;
 	else if (!starts_with(*head_points_at, "refs/heads/")) {
 		errors_found |= ERROR_REFS;
-		return error("HEAD points to something strange (%s)",
-			     *head_points_at);
+		return error("%s points to something strange (%s)",
+			     head_ref_name, *head_points_at);
 	}
 	if (is_null_oid(head_oid)) {
 		if (null_is_error) {
 			errors_found |= ERROR_REFS;
-			return error("HEAD: detached HEAD points at nothing");
+			return error("%s: detached HEAD points at nothing",
+				     head_ref_name);
 		}
-		fprintf(stderr, "notice: HEAD points to an unborn branch (%s)\n",
-			*head_points_at + 11);
+		fprintf(stderr, "notice: %s points to an unborn branch (%s)\n",
+			head_ref_name, *head_points_at + 11);
 	}
 	return 0;
 }
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 0f2dd26f74..28201677d5 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -101,6 +101,41 @@ test_expect_success 'HEAD link pointing at a funny place' '
 	grep "HEAD points to something strange" out
 '
 
+test_expect_success 'HEAD link pointing at a funny object (from different wt)' '
+	test_when_finished "mv .git/SAVED_HEAD .git/HEAD" &&
+	test_when_finished "rm -rf .git/worktrees wt" &&
+	git worktree add wt &&
+	mv .git/HEAD .git/SAVED_HEAD &&
+	echo $ZERO_OID >.git/HEAD &&
+	# avoid corrupt/broken HEAD from interfering with repo discovery
+	test_must_fail git -C wt fsck 2>out &&
+	grep "main-worktree/HEAD: detached HEAD points" out
+'
+
+test_expect_success 'other worktree HEAD link pointing at a funny object' '
+	test_when_finished "rm -rf .git/worktrees other" &&
+	git worktree add other &&
+	echo $ZERO_OID >.git/worktrees/other/HEAD &&
+	test_must_fail git fsck 2>out &&
+	grep "worktrees/other/HEAD: detached HEAD points" out
+'
+
+test_expect_success 'other worktree HEAD link pointing at missing object' '
+	test_when_finished "rm -rf .git/worktrees other" &&
+	git worktree add other &&
+	echo "Contents missing from repo" | git hash-object --stdin >.git/worktrees/other/HEAD &&
+	test_must_fail git fsck 2>out &&
+	grep "worktrees/other/HEAD: invalid sha1 pointer" out
+'
+
+test_expect_success 'other worktree HEAD link pointing at a funny place' '
+	test_when_finished "rm -rf .git/worktrees other" &&
+	git worktree add other &&
+	echo "ref: refs/funny/place" >.git/worktrees/other/HEAD &&
+	test_must_fail git fsck 2>out &&
+	grep "worktrees/other/HEAD points to something strange" out
+'
+
 test_expect_success 'email without @ is okay' '
 	git cat-file commit HEAD >basis &&
 	sed "s/@/AT/" basis >okay &&
-- 
2.19.1.647.g708186aaf9

[PATCH v3 8/8] reflog expire: cover reflog from all worktrees

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

Reported-by: Jeff King <peff@peff.net>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-reflog.txt |  7 +++++-
 builtin/reflog.c             | 46 ++++++++++++++++++++++++++++++++----
 t/t1410-reflog.sh            | 15 ++++++++++++
 3 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt
index 472a6808cd..ff487ff77d 100644
--- a/Documentation/git-reflog.txt
+++ b/Documentation/git-reflog.txt
@@ -20,7 +20,7 @@ depending on the subcommand:
 'git reflog' ['show'] [log-options] [<ref>]
 'git reflog expire' [--expire=<time>] [--expire-unreachable=<time>]
 	[--rewrite] [--updateref] [--stale-fix]
-	[--dry-run | -n] [--verbose] [--all | <refs>...]
+	[--dry-run | -n] [--verbose] [--all [--single-worktree] | <refs>...]
 'git reflog delete' [--rewrite] [--updateref]
 	[--dry-run | -n] [--verbose] ref@\{specifier\}...
 'git reflog exists' <ref>
@@ -72,6 +72,11 @@ Options for `expire`
 --all::
 	Process the reflogs of all references.
 
+--single-worktree::
+	By default when `--all` is specified, reflogs from all working
+	trees are processed. This option limits the processing to reflogs
+	from the current working tree only.
+
 --expire=<time>::
 	Prune entries older than the specified time. If this option is
 	not specified, the expiration time is taken from the
diff --git a/builtin/reflog.c b/builtin/reflog.c
index b5941c1ff3..7a85e4b164 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -10,6 +10,7 @@
 #include "diff.h"
 #include "revision.h"
 #include "reachable.h"
+#include "worktree.h"
 
 /* NEEDSWORK: switch to using parse_options */
 static const char reflog_expire_usage[] =
@@ -52,6 +53,7 @@ struct collect_reflog_cb {
 	struct collected_reflog **e;
 	int alloc;
 	int nr;
+	struct worktree *wt;
 };
 
 /* Remember to update object flag allocation in object.h */
@@ -330,13 +332,27 @@ static int push_tip_to_list(const char *refname, const struct object_id *oid,
 	return 0;
 }
 
+static int is_head(const char *refname)
+{
+	switch (ref_type(refname)) {
+	case REF_TYPE_OTHER_PSEUDOREF:
+	case REF_TYPE_MAIN_PSEUDOREF:
+		if (parse_worktree_ref(refname, NULL, NULL, &refname))
+			BUG("not a worktree ref: %s", refname);
+		break;
+	default:
+		break;
+	}
+	return !strcmp(refname, "HEAD");
+}
+
 static void reflog_expiry_prepare(const char *refname,
 				  const struct object_id *oid,
 				  void *cb_data)
 {
 	struct expire_reflog_policy_cb *cb = cb_data;
 
-	if (!cb->cmd.expire_unreachable || !strcmp(refname, "HEAD")) {
+	if (!cb->cmd.expire_unreachable || is_head(refname)) {
 		cb->tip_commit = NULL;
 		cb->unreachable_expire_kind = UE_HEAD;
 	} else {
@@ -388,8 +404,19 @@ static int collect_reflog(const char *ref, const struct object_id *oid, int unus
 {
 	struct collected_reflog *e;
 	struct collect_reflog_cb *cb = cb_data;
+	struct strbuf newref = STRBUF_INIT;
+
+	/*
+	 * Avoid collecting the same shared ref multiple times because
+	 * they are available via all worktrees.
+	 */
+	if (!cb->wt->is_current && ref_type(ref) == REF_TYPE_NORMAL)
+		return 0;
+
+	strbuf_worktree_ref(cb->wt, &newref, ref);
+	FLEX_ALLOC_STR(e, reflog, newref.buf);
+	strbuf_release(&newref);
 
-	FLEX_ALLOC_STR(e, reflog, ref);
 	oidcpy(&e->oid, oid);
 	ALLOC_GROW(cb->e, cb->nr + 1, cb->alloc);
 	cb->e[cb->nr++] = e;
@@ -512,7 +539,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 {
 	struct expire_reflog_policy_cb cb;
 	timestamp_t now = time(NULL);
-	int i, status, do_all;
+	int i, status, do_all, all_worktrees = 1;
 	int explicit_expiry = 0;
 	unsigned int flags = 0;
 
@@ -549,6 +576,8 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 			flags |= EXPIRE_REFLOGS_UPDATE_REF;
 		else if (!strcmp(arg, "--all"))
 			do_all = 1;
+		else if (!strcmp(arg, "--single-worktree"))
+			all_worktrees = 0;
 		else if (!strcmp(arg, "--verbose"))
 			flags |= EXPIRE_REFLOGS_VERBOSE;
 		else if (!strcmp(arg, "--")) {
@@ -577,10 +606,19 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 
 	if (do_all) {
 		struct collect_reflog_cb collected;
+		struct worktree **worktrees, **p;
 		int i;
 
 		memset(&collected, 0, sizeof(collected));
-		for_each_reflog(collect_reflog, &collected);
+		worktrees = get_worktrees(0);
+		for (p = worktrees; *p; p++) {
+			if (!all_worktrees && !(*p)->is_current)
+				continue;
+			collected.wt = *p;
+			refs_for_each_reflog(get_worktree_ref_store(*p),
+					     collect_reflog, &collected);
+		}
+		free_worktrees(worktrees);
 		for (i = 0; i < collected.nr; i++) {
 			struct collected_reflog *e = collected.e[i];
 			set_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog);
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 388b0611d8..3e053532eb 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -368,4 +368,19 @@ test_expect_success 'continue walking past root commits' '
 	)
 '
 
+test_expect_success 'expire with multiple worktrees' '
+	git init main-wt &&
+	(
+		cd main-wt &&
+		test_tick &&
+		test_commit foo &&
+		git  worktree add link-wt &&
+		test_tick &&
+		test_commit -C link-wt foobar &&
+		test_tick &&
+		git reflog expire --verbose --all --expire=$test_tick &&
+		test_must_be_empty .git/worktrees/link-wt/logs/HEAD
+	)
+'
+
 test_done
-- 
2.19.1.647.g708186aaf9

Re: [PATCH v3 2/8] Add a place for (not) sharing stuff between worktrees

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> Subject: Re: [PATCH v3 2/8] Add a place for (not) sharing stuff between worktrees

"a place"?  Missing "in $GIR_DIR" in the descrition made me read the
above three times before getting what it wanted to say.

My attempt to improve it, which admittedly is not great, came up with:

worktree: convention to make per-worktree things identifiable in $GIT_DIR

> When multiple worktrees are used, we need rules to determine if
> something belongs to one worktree or all of them. Instead of keeping
> adding rules when new stuff comes (*), have a generic rule:
>
> - Inside $GIT_DIR, which is per-worktree by default, add
>   $GIT_DIR/common which is always shared. New features that want to
>   share stuff should put stuff under this directory.
>
> - Inside refs/, which is shared by default except refs/bisect, add
>   refs/worktree/ which is per-worktree. We may eventually move
>   refs/bisect to this new location and remove the exception in refs
>   code.
>
> (*) And it may also include stuff from external commands which will
>     have no way to modify common/per-worktree rules.

OK.  Establishing such a convention is a good role for the core-git
should play to help third-party tools.

Should this play well with the per-worktree configuration as well?
Is it better to carve out a configuration variable namespace so that
certain keys are never read from common ones (or per-worktree ones),
so that people can tell which ones are what?  I know your current
design says "this is just another new layer, and the users can hang
themselves with this new rope".  I am wondering if there is a need
to do something a bit more structured.

Re: [PATCH v3 2/8] Add a place for (not) sharing stuff between worktrees

[SZEDER Gábor]

On Sun, Oct 21, 2018 at 10:08:53AM +0200, Nguyễn Thái Ngọc Duy wrote:
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index e2ee9fc21b..a50fbf8094 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -204,6 +204,22 @@ working trees, it can be used to identify worktrees. For example if
>  you only have two working trees, at "/abc/def/ghi" and "/abc/def/ggg",
>  then "ghi" or "def/ghi" is enough to point to the former working tree.
>  
> +REFS
> +----
> +In multiple working trees, some refs may be shared between all working
> +trees, some refs are local. One example is HEAD is different for all
> +working trees. This section is about the sharing rules.
> +
> +In general, all pseudo refs are per working tree and all refs starting
> +with "refs/" are shared. Pseudo refs are ones like HEAD which are
> +directly under GIT_DIR instead of inside GIT_DIR/refs. There are one
> +exception to this: refs inside refs/bisect and refs/worktree is not
> +shared.
> +
> +To access refs, it's best not to look inside GIT_DIR directly. Instead
> +use commands such as linkgit:git-revparse[1] or linkgit:git-update-ref[1]

s/revparse/rev-parse/

> +which will handle refs correctly.
> +

Re: [PATCH v3 2/8] Add a place for (not) sharing stuff between worktrees

[Duy Nguyen]

On Mon, Oct 22, 2018 at 6:28 AM Junio C Hamano <gitster@pobox.com> wrote:
> > When multiple worktrees are used, we need rules to determine if
> > something belongs to one worktree or all of them. Instead of keeping
> > adding rules when new stuff comes (*), have a generic rule:
> >
> > - Inside $GIT_DIR, which is per-worktree by default, add
> >   $GIT_DIR/common which is always shared. New features that want to
> >   share stuff should put stuff under this directory.
> >
> > - Inside refs/, which is shared by default except refs/bisect, add
> >   refs/worktree/ which is per-worktree. We may eventually move
> >   refs/bisect to this new location and remove the exception in refs
> >   code.
> >
> > (*) And it may also include stuff from external commands which will
> >     have no way to modify common/per-worktree rules.
>
> OK.  Establishing such a convention is a good role for the core-git
> should play to help third-party tools.
>
> Should this play well with the per-worktree configuration as well?
> Is it better to carve out a configuration variable namespace so that
> certain keys are never read from common ones (or per-worktree ones),
> so that people can tell which ones are what?  I know your current
> design says "this is just another new layer, and the users can hang
> themselves with this new rope".  I am wondering if there is a need
> to do something a bit more structured.

I actually find the layered design of config files more elegant. Even
before config.worktree is added, the user has system/per-user/per-repo
places to put stuff in. "git config" gives control to read or write
from a specific layer. We have to add layers to $GIT_DIR/refs to
handle multiple worktrees, but since the "API" for those don't have
the concept of layers at all, we need a single view where items from
different worktrees are accessible via the same path/ref name.

Adding variable namespace in config files does not look easy either.
Suppose we have perWorktree.* section just for per-workree stuff, what
do we do when the user just do "git config -f not-per-worktree-file
perWorktree.something foo" (or --global instead of -f)? What we could
do (and already do for some config keys) is ignore perWorktree.*
unless they come from config.worktree. Which does help a bit, but not
really perfect.

And a namespace like this means the user cannot have subsections
anymore (e.g. perWorktree.group.<name>.var is not supported if I
remember correctly). So yeah... perhaps we should wait a bit and see
what the user (or third party tool makers) comes up with.
-- 
Duy

Re: [PATCH v3 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

[Ævar Arnfjörð Bjarmason]

On Sun, Oct 21 2018, Nguyễn Thái Ngọc Duy wrote:

This change has a regression in 2.20:

> [...]
>  static void files_reflog_path(struct files_ref_store *refs,
>  			      struct strbuf *sb,
>  			      const char *refname)
> @@ -158,6 +178,9 @@ static void files_reflog_path(struct files_ref_store *refs,
>  	case REF_TYPE_PSEUDOREF:
>  		strbuf_addf(sb, "%s/logs/%s", refs->gitdir, refname);
>  		break;
> +	case REF_TYPE_OTHER_PSEUDOREF:
> +	case REF_TYPE_MAIN_PSEUDOREF:
> +		return files_reflog_path_other_worktrees(refs, sb, refname);
>  	case REF_TYPE_NORMAL:
>  		strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir, refname);
>  		break;

SunCC on Solaris hard errors on this:

    "refs/files-backend.c", line 183: void function cannot return value

Needs to be files...(); break; instead.

Re: [PATCH v3 3/8] refs: new ref types to make per-worktree refs visible to all worktrees

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Sun, Oct 21 2018, Nguyễn Thái Ngọc Duy wrote:
>
> This change has a regression in 2.20:
>
>> [...]
>>  static void files_reflog_path(struct files_ref_store *refs,
>>  			      struct strbuf *sb,
>>  			      const char *refname)
>> @@ -158,6 +178,9 @@ static void files_reflog_path(struct files_ref_store *refs,
>>  	case REF_TYPE_PSEUDOREF:
>>  		strbuf_addf(sb, "%s/logs/%s", refs->gitdir, refname);
>>  		break;
>> +	case REF_TYPE_OTHER_PSEUDOREF:
>> +	case REF_TYPE_MAIN_PSEUDOREF:
>> +		return files_reflog_path_other_worktrees(refs, sb, refname);
>>  	case REF_TYPE_NORMAL:
>>  		strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir, refname);
>>  		break;
>
> SunCC on Solaris hard errors on this:
>
>     "refs/files-backend.c", line 183: void function cannot return value
>
> Needs to be files...(); break; instead.

True.

The caller itself returns "void", so it would be nice if this were a
mere warning() from practical usabliity's point of view, though ;-)

[PATCH] files-backend.c: fix build error on Solaris

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

This function files_reflog_path returns void, which usually means
"return;" not returning "void value" from another function.

Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 refs/files-backend.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 9183875dad..dd8abe9185 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -180,7 +180,8 @@ static void files_reflog_path(struct files_ref_store *refs,
 		break;
 	case REF_TYPE_OTHER_PSEUDOREF:
 	case REF_TYPE_MAIN_PSEUDOREF:
-		return files_reflog_path_other_worktrees(refs, sb, refname);
+		files_reflog_path_other_worktrees(refs, sb, refname);
+		break;
 	case REF_TYPE_NORMAL:
 		strbuf_addf(sb, "%s/logs/%s", refs->gitcommondir, refname);
 		break;
-- 
2.19.1.1327.g328c130451.dirty

Re: [PATCH] files-backend.c: fix build error on Solaris

[Carlo Arenas]

Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>

clang with -Wpedantic also catch this (at least with Apple LLVM
version 10.0.0); recent versions of gcc also include that flag and at
least 8.2.0 shows a warning for it, so it might be worth adding it to
developer mode (maybe under the pedantic DEVOPTS), would this be
something I should be adding?, what are the expectations around
warnings and compilers?

Carlo

Re: [PATCH] files-backend.c: fix build error on Solaris

[Duy Nguyen]

On Sun, Nov 25, 2018 at 11:19 AM Carlo Arenas <carenas@gmail.com> wrote:
>
> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
>
> clang with -Wpedantic also catch this (at least with Apple LLVM
> version 10.0.0); recent versions of gcc also include that flag and at
> least 8.2.0 shows a warning for it, so it might be worth adding it to
> developer mode (maybe under the pedantic DEVOPTS), would this be
> something I should be adding?, what are the expectations around
> warnings and compilers?

make DEVELOPER=1 DEVOPTS=pedantic (with either gcc 7.3.0 or 8.2.0) can
already catch this. I have no comment if this pedantic should be part
of default DEVELOPER=1. But at least in controlled environments like
Travis, we could turn it on to catch more.
-- 
Duy

Re: [PATCH] files-backend.c: fix build error on Solaris

[Junio C Hamano]

Carlo Arenas <carenas@gmail.com> writes:

> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>

Do you mean Tested-by: (meaning, you actually saw the breakage with
SunCC without the patch and also saw the patch fixed the breakage)?

> clang with -Wpedantic also catch this (at least with Apple LLVM
> version 10.0.0); recent versions of gcc also include that flag and at
> least 8.2.0 shows a warning for it, so it might be worth adding it to
> developer mode (maybe under the pedantic DEVOPTS).

Nice to know.

> ..., would this be something I should be adding?, what are the expectations around
> warnings and compilers?

It may be a worthwhile thing to discuss, and as a discussion
starter, a patch would help ;-).