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

2018-10-06 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   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/", ))
> + 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/", ))
  ^
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/", ))
  ^
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


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

2018-09-29 Thread Eric Sunshine
On Sat, Sep 29, 2018 at 3:10 PM Nguyễn Thái Ngọc Duy  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 
> ---
> 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
> +'


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

2018-09-29 Thread 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 
---
 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 &&
+   is_pseudoref_syntax(refname);
+}
+
+static int is_other_pseudoref_syntax(const char *refname)
+{
+   if (!skip_prefix(refname, "worktrees/", ))
+   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,