diff mbox series

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

Message ID 20181021080859.3203-3-pclouds@gmail.com (mailing list archive)
State New, archived
Headers show
Series fix per-worktree ref iteration in fsck/reflog expire | expand

Commit Message

Duy Nguyen Oct. 21, 2018, 8:08 a.m. UTC
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

Comments

Junio C Hamano Oct. 22, 2018, 4:28 a.m. UTC | #1
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.
SZEDER Gábor Oct. 22, 2018, 10:25 a.m. UTC | #2
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.
> +
Duy Nguyen Oct. 29, 2018, 5:18 p.m. UTC | #3
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.
diff mbox series

Patch

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