git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Ensure unique worktree ids across repositories
@ 2024-11-29  2:44 Caleb White
  2024-11-29  2:44 ` [PATCH 1/2] worktree: add worktree with unique suffix Caleb White
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Caleb White @ 2024-11-29  2:44 UTC (permalink / raw)
  To: git; +Cc: Caleb White

The `es/worktree-repair-copied` topic added support for repairing a
worktree from a copy scenario. I noted[1,2] that the topic added the
ability for a repository to "take over" a worktree from another
repository if the worktree_id matched a worktree inside the current
repository which can happen if two repositories use the same worktree name.

This series teaches Git to create worktrees with a unique suffix so
that the worktree_id is unique across all repositories even if they have
the same name. For example creating a worktree `develop` would look like:

    foo/
    ├── .git/worktrees/develop-5445874156/
    └── develop/
    bar/
    ├── .git/worktrees/develop-1549518426/
    └── develop/

The actual worktree directory name is still `develop`, but the
worktree_id is unique and prevents the "take over" scenario. The suffix
is given by the `git_rand()` function, but I'm open to suggestions if
there's a better random or hashing function to use.

[1]: https://lore.kernel.org/git/20241008153035.71178-1-cdwhite3@pm.me/
[2]: https://lore.kernel.org/git/r4zmcET41Skr_FMop47AKd7cms9E8bKPSvHuAUpnYavzKEY6JybJta0_7GfuYB0q-gD-XNcvh5VDTfiT3qthGKjqhS1sbT4M2lUABynOz2Q=@pm.me/

Signed-off-by: Caleb White <cdwhite3@pm.me>
---
Caleb White (2):
      worktree: add worktree with unique suffix
      worktree: rename worktree id during worktree move

 Documentation/git-worktree.txt     |  5 +-
 builtin/worktree.c                 | 30 ++++++++++++
 t/t0035-safe-bare-repository.sh    |  4 +-
 t/t0600-reffiles-backend.sh        | 10 ++--
 t/t0601-reffiles-pack-refs.sh      |  4 +-
 t/t0610-reftable-basics.sh         | 54 +++++++++++-----------
 t/t1407-worktree-ref-store.sh      |  4 +-
 t/t1410-reflog.sh                  | 10 ++--
 t/t1415-worktree-refs.sh           | 26 +++++------
 t/t1450-fsck.sh                    | 14 +++---
 t/t1500-rev-parse.sh               |  6 +--
 t/t2400-worktree-add.sh            | 51 +++++++++++----------
 t/t2401-worktree-prune.sh          | 20 ++++----
 t/t2403-worktree-move.sh           | 38 ++++++++--------
 t/t2405-worktree-submodule.sh      | 10 ++--
 t/t2406-worktree-repair.sh         | 93 ++++++++++++++++++++++++--------------
 t/t2407-worktree-heads.sh          | 27 +++++------
 t/t3200-branch.sh                  | 10 ++--
 t/t5304-prune.sh                   |  2 +-
 t/t7412-submodule-absorbgitdirs.sh |  4 +-
 20 files changed, 239 insertions(+), 183 deletions(-)
---
base-commit: 090d24e9af6e9f59c3f7bee97c42bb1ae3c7f559
change-id: 20241127-wt_unique_ids-1ffd7ea0bb19
prerequisite-change-id: 20241025-wt_relative_options-afa41987bc32:v5
prerequisite-patch-id: 179410e257e8eedf100f4f9faa9467cbbba4d61b
prerequisite-patch-id: 56ffe0afeadd511c9eef5f548a371659b040acab
prerequisite-patch-id: 809c1314e5dfa966f4f3d73b52f286f8aa89370f
prerequisite-patch-id: cf5f9491c8f8e58d1e0e103a5f8c64c55f2896e3
prerequisite-patch-id: 3d3bb3cc81d3030b1d27c39fdb4cf0e383937f89
prerequisite-patch-id: 62a09496d98d78a6bd1f9150ba887ee72359c7ee
prerequisite-patch-id: 5527e4b745963dd4fa08029491fcbfe3d91d5104
prerequisite-patch-id: bf433443e90939a493fa586de30938f78cb77020

Best regards,
-- 
Caleb White <cdwhite3@pm.me>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] worktree: add worktree with unique suffix
  2024-11-29  2:44 [PATCH 0/2] Ensure unique worktree ids across repositories Caleb White
@ 2024-11-29  2:44 ` Caleb White
  2024-11-29  2:44 ` [PATCH 2/2] worktree: rename worktree id during worktree move Caleb White
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Caleb White @ 2024-11-29  2:44 UTC (permalink / raw)
  To: git; +Cc: Caleb White

The `es/worktree-repair-copied` topic added support for repairing a
worktree from a copy scenario. However, the topic added the
ability for a repository to "take over" a worktree from another
repository if the worktree_id matched a worktree inside the current
repository which can happen if two repositories use the same worktree name.

This teaches Git to create worktrees with a unique suffix so the
worktree_id is unique across all repositories even if they have the
same name. For example creating a worktree `develop` would look like:

    foo/
    ├── .git/worktrees/develop-5445874156/
    └── develop/
    bar/
    ├── .git/worktrees/develop-1549518426/
    └── develop/

The actual worktree directory name is still `develop`, but the
worktree_id is unique and prevents the "take over" scenario. The suffix
is given by the `git_rand()` function. Worktree ids can already differ
from the actual directory name (appended with a number like `develop1`)
if the worktree name was already taken, so this should not be a
drastic change.

Signed-off-by: Caleb White <cdwhite3@pm.me>
---
 Documentation/git-worktree.txt     |  5 +-
 builtin/worktree.c                 |  6 +++
 t/t0035-safe-bare-repository.sh    |  4 +-
 t/t0600-reffiles-backend.sh        | 10 ++--
 t/t0601-reffiles-pack-refs.sh      |  4 +-
 t/t0610-reftable-basics.sh         | 54 +++++++++++-----------
 t/t1407-worktree-ref-store.sh      |  4 +-
 t/t1410-reflog.sh                  | 10 ++--
 t/t1415-worktree-refs.sh           | 26 +++++------
 t/t1450-fsck.sh                    | 14 +++---
 t/t1500-rev-parse.sh               |  6 +--
 t/t2400-worktree-add.sh            | 51 +++++++++++----------
 t/t2401-worktree-prune.sh          | 20 ++++----
 t/t2403-worktree-move.sh           | 32 ++++++-------
 t/t2405-worktree-submodule.sh      | 10 ++--
 t/t2406-worktree-repair.sh         | 93 ++++++++++++++++++++++++--------------
 t/t2407-worktree-heads.sh          | 27 +++++------
 t/t3200-branch.sh                  | 10 ++--
 t/t5304-prune.sh                   |  2 +-
 t/t7412-submodule-absorbgitdirs.sh |  4 +-
 20 files changed, 212 insertions(+), 180 deletions(-)

diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index 8340b7f028e6c1c3bae3de0879e9754098466d14..e0604b043361828f94b58f676a5ed4f15b116348 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -352,12 +352,11 @@ DETAILS
 -------
 Each linked worktree has a private sub-directory in the repository's
 `$GIT_DIR/worktrees` directory.  The private sub-directory's name is usually
-the base name of the linked worktree's path, possibly appended with a
+the base name of the linked worktree's path, appended with a random
 number to make it unique.  For example, when `$GIT_DIR=/path/main/.git` the
 command `git worktree add /path/other/test-next next` creates the linked
 worktree in `/path/other/test-next` and also creates a
-`$GIT_DIR/worktrees/test-next` directory (or `$GIT_DIR/worktrees/test-next1`
-if `test-next` is already taken).
+`$GIT_DIR/worktrees/test-next-#######` directory.
 
 Within a linked worktree, `$GIT_DIR` is set to point to this private
 directory (e.g. `/path/main/.git/worktrees/test-next` in the example) and
diff --git a/builtin/worktree.c b/builtin/worktree.c
index fde9ff4dc9a734c655e95ccd62774282950cbba6..3ad355ca762729401fc0c8625f4fd05b154a84ec 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -421,6 +421,7 @@ static int add_worktree(const char *path, const char *refname,
 	struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;
 	struct strbuf sb = STRBUF_INIT;
 	const char *name;
+	const char *suffix;
 	struct strvec child_env = STRVEC_INIT;
 	unsigned int counter = 0;
 	int len, ret;
@@ -455,6 +456,11 @@ static int add_worktree(const char *path, const char *refname,
 	strbuf_reset(&sb);
 	name = sb_name.buf;
 	git_path_buf(&sb_repo, "worktrees/%s", name);
+	suffix = getenv("GIT_TEST_WORKTREE_SUFFIX");
+	if (suffix)
+		strbuf_addf(&sb_repo, "-%s", suffix);
+	else
+		strbuf_addf(&sb_repo, "-%u", git_rand());
 	len = sb_repo.len;
 	if (safe_create_leading_directories_const(sb_repo.buf))
 		die_errno(_("could not create leading directories of '%s'"),
diff --git a/t/t0035-safe-bare-repository.sh b/t/t0035-safe-bare-repository.sh
index d3cb2a1cb9edb8f9ad7480be6ec3e02b464046bd..30cbf7fd32e2e4afd6d680e0328ee18b791778da 100755
--- a/t/t0035-safe-bare-repository.sh
+++ b/t/t0035-safe-bare-repository.sh
@@ -41,7 +41,7 @@ test_expect_success 'setup an embedded bare repo, secondary worktree and submodu
 		git -c protocol.file.allow=always \
 			submodule add --name subn -- ./bare-repo subd
 	) &&
-	test_path_is_dir outer-repo/.git/worktrees/outer-secondary &&
+	test_path_is_dir outer-repo/.git/worktrees/outer-secondary-* &&
 	test_path_is_dir outer-repo/.git/modules/subn
 '
 
@@ -97,7 +97,7 @@ test_expect_success 'no trace when "bare repository" is a subdir of .git' '
 '
 
 test_expect_success 'no trace in $GIT_DIR of secondary worktree' '
-	expect_accepted_implicit -C outer-repo/.git/worktrees/outer-secondary
+	expect_accepted_implicit -C outer-repo/.git/worktrees/outer-secondary-*
 '
 
 test_expect_success 'no trace in $GIT_DIR of a submodule' '
diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
index bef2b70871364931ab8b3ead950f59d11b6fe216..8da81b1ae34f5f095ba9406e8f031fd5e43ac789 100755
--- a/t/t0600-reffiles-backend.sh
+++ b/t/t0600-reffiles-backend.sh
@@ -256,12 +256,12 @@ test_expect_success 'delete fails cleanly if packed-refs.new write fails' '
 	test_cmp unchanged actual
 '
 
-RWT="test-tool ref-store worktree:wt"
+RWT="test-tool ref-store worktree:wt-123"
 RMAIN="test-tool ref-store worktree:main"
 
 test_expect_success 'setup worktree' '
 	test_commit first &&
-	git worktree add -b wt-main wt &&
+	GIT_TEST_WORKTREE_SUFFIX=123 git worktree add -b wt-main wt &&
 	(
 		cd wt &&
 		test_commit second
@@ -279,9 +279,9 @@ test_expect_success 'for_each_reflog()' '
 	mkdir -p     .git/logs/refs/bisect &&
 	echo $ZERO_OID >.git/logs/refs/bisect/random &&
 
-	echo $ZERO_OID >.git/worktrees/wt/logs/PSEUDO_WT_HEAD &&
-	mkdir -p     .git/worktrees/wt/logs/refs/bisect &&
-	echo $ZERO_OID >.git/worktrees/wt/logs/refs/bisect/wt-random &&
+	echo $ZERO_OID >.git/worktrees/wt-123/logs/PSEUDO_WT_HEAD &&
+	mkdir -p     .git/worktrees/wt-123/logs/refs/bisect &&
+	echo $ZERO_OID >.git/worktrees/wt-123/logs/refs/bisect/wt-random &&
 
 	$RWT for-each-reflog >actual &&
 	cat >expected <<-\EOF &&
diff --git a/t/t0601-reffiles-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh
index d8cbd3f202b5f00c9a94c5a2ea2dced6606b6f4d..d6597938caca5e6c3a6e86b860f3c0d309aa2140 100755
--- a/t/t0601-reffiles-pack-refs.sh
+++ b/t/t0601-reffiles-pack-refs.sh
@@ -326,8 +326,8 @@ 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_path_is_file .git/worktrees/wt1-*/refs/worktree/foo &&
+	test_path_is_file .git/worktrees/wt2-*/refs/worktree/foo
 '
 
 # we do not want to count on running pack-refs to
diff --git a/t/t0610-reftable-basics.sh b/t/t0610-reftable-basics.sh
index eaf6fab6d29f01430abae3c7abf5a750d4271a36..f21fd9dbba2b77636045b297073e340209d61a2d 100755
--- a/t/t0610-reftable-basics.sh
+++ b/t/t0610-reftable-basics.sh
@@ -965,11 +965,11 @@ test_expect_success 'worktree: adding worktree creates separate stack' '
 	test_commit -C repo A &&
 
 	git -C repo worktree add ../worktree &&
-	test_path_is_file repo/.git/worktrees/worktree/refs/heads &&
+	test_path_is_file repo/.git/worktrees/worktree-*/refs/heads &&
 	echo "ref: refs/heads/.invalid" >expect &&
-	test_cmp expect repo/.git/worktrees/worktree/HEAD &&
-	test_path_is_dir repo/.git/worktrees/worktree/reftable &&
-	test_path_is_file repo/.git/worktrees/worktree/reftable/tables.list
+	test_cmp expect repo/.git/worktrees/worktree-*/HEAD &&
+	test_path_is_dir repo/.git/worktrees/worktree-*/reftable &&
+	test_path_is_file repo/.git/worktrees/worktree-*/reftable/tables.list
 '
 
 test_expect_success 'worktree: pack-refs in main repo packs main refs' '
@@ -982,10 +982,10 @@ test_expect_success 'worktree: pack-refs in main repo packs main refs' '
 	GIT_TEST_REFTABLE_AUTOCOMPACTION=false \
 	git -C worktree update-ref refs/worktree/per-worktree HEAD &&
 
-	test_line_count = 4 repo/.git/worktrees/worktree/reftable/tables.list &&
+	test_line_count = 4 repo/.git/worktrees/worktree-*/reftable/tables.list &&
 	test_line_count = 3 repo/.git/reftable/tables.list &&
 	git -C repo pack-refs &&
-	test_line_count = 4 repo/.git/worktrees/worktree/reftable/tables.list &&
+	test_line_count = 4 repo/.git/worktrees/worktree-*/reftable/tables.list &&
 	test_line_count = 1 repo/.git/reftable/tables.list
 '
 
@@ -999,10 +999,10 @@ test_expect_success 'worktree: pack-refs in worktree packs worktree refs' '
 	GIT_TEST_REFTABLE_AUTOCOMPACTION=false \
 	git -C worktree update-ref refs/worktree/per-worktree HEAD &&
 
-	test_line_count = 4 repo/.git/worktrees/worktree/reftable/tables.list &&
+	test_line_count = 4 repo/.git/worktrees/worktree-*/reftable/tables.list &&
 	test_line_count = 3 repo/.git/reftable/tables.list &&
 	git -C worktree pack-refs &&
-	test_line_count = 1 repo/.git/worktrees/worktree/reftable/tables.list &&
+	test_line_count = 1 repo/.git/worktrees/worktree-*/reftable/tables.list &&
 	test_line_count = 3 repo/.git/reftable/tables.list
 '
 
@@ -1014,12 +1014,12 @@ test_expect_success 'worktree: creating shared ref updates main stack' '
 	git -C repo worktree add ../worktree &&
 	git -C repo pack-refs &&
 	git -C worktree pack-refs &&
-	test_line_count = 1 repo/.git/worktrees/worktree/reftable/tables.list &&
+	test_line_count = 1 repo/.git/worktrees/worktree-*/reftable/tables.list &&
 	test_line_count = 1 repo/.git/reftable/tables.list &&
 
 	GIT_TEST_REFTABLE_AUTOCOMPACTION=false \
 	git -C worktree update-ref refs/heads/shared HEAD &&
-	test_line_count = 1 repo/.git/worktrees/worktree/reftable/tables.list &&
+	test_line_count = 1 repo/.git/worktrees/worktree-*/reftable/tables.list &&
 	test_line_count = 2 repo/.git/reftable/tables.list
 '
 
@@ -1031,11 +1031,11 @@ test_expect_success 'worktree: creating per-worktree ref updates worktree stack'
 	git -C repo worktree add ../worktree &&
 	git -C repo pack-refs &&
 	git -C worktree pack-refs &&
-	test_line_count = 1 repo/.git/worktrees/worktree/reftable/tables.list &&
+	test_line_count = 1 repo/.git/worktrees/worktree-*/reftable/tables.list &&
 	test_line_count = 1 repo/.git/reftable/tables.list &&
 
 	git -C worktree update-ref refs/bisect/per-worktree HEAD &&
-	test_line_count = 2 repo/.git/worktrees/worktree/reftable/tables.list &&
+	test_line_count = 2 repo/.git/worktrees/worktree-*/reftable/tables.list &&
 	test_line_count = 1 repo/.git/reftable/tables.list
 '
 
@@ -1044,14 +1044,14 @@ test_expect_success 'worktree: creating per-worktree ref from main repo' '
 	git init repo &&
 	test_commit -C repo A &&
 
-	git -C repo worktree add ../worktree &&
+	GIT_TEST_WORKTREE_SUFFIX=456 git -C repo worktree add ../worktree &&
 	git -C repo pack-refs &&
 	git -C worktree pack-refs &&
-	test_line_count = 1 repo/.git/worktrees/worktree/reftable/tables.list &&
+	test_line_count = 1 repo/.git/worktrees/worktree-456/reftable/tables.list &&
 	test_line_count = 1 repo/.git/reftable/tables.list &&
 
-	git -C repo update-ref worktrees/worktree/refs/bisect/per-worktree HEAD &&
-	test_line_count = 2 repo/.git/worktrees/worktree/reftable/tables.list &&
+	git -C repo update-ref worktrees/worktree-456/refs/bisect/per-worktree HEAD &&
+	test_line_count = 2 repo/.git/worktrees/worktree-456/reftable/tables.list &&
 	test_line_count = 1 repo/.git/reftable/tables.list
 '
 
@@ -1060,18 +1060,18 @@ test_expect_success 'worktree: creating per-worktree ref from second worktree' '
 	git init repo &&
 	test_commit -C repo A &&
 
-	git -C repo worktree add ../wt1 &&
-	git -C repo worktree add ../wt2 &&
+	GIT_TEST_WORKTREE_SUFFIX=123 git -C repo worktree add ../wt1 &&
+	GIT_TEST_WORKTREE_SUFFIX=456 git -C repo worktree add ../wt2 &&
 	git -C repo pack-refs &&
 	git -C wt1 pack-refs &&
 	git -C wt2 pack-refs &&
-	test_line_count = 1 repo/.git/worktrees/wt1/reftable/tables.list &&
-	test_line_count = 1 repo/.git/worktrees/wt2/reftable/tables.list &&
+	test_line_count = 1 repo/.git/worktrees/wt1-123/reftable/tables.list &&
+	test_line_count = 1 repo/.git/worktrees/wt2-456/reftable/tables.list &&
 	test_line_count = 1 repo/.git/reftable/tables.list &&
 
-	git -C wt1 update-ref worktrees/wt2/refs/bisect/per-worktree HEAD &&
-	test_line_count = 1 repo/.git/worktrees/wt1/reftable/tables.list &&
-	test_line_count = 2 repo/.git/worktrees/wt2/reftable/tables.list &&
+	git -C wt1 update-ref worktrees/wt2-456/refs/bisect/per-worktree HEAD &&
+	test_line_count = 1 repo/.git/worktrees/wt1-123/reftable/tables.list &&
+	test_line_count = 2 repo/.git/worktrees/wt2-456/reftable/tables.list &&
 	test_line_count = 1 repo/.git/reftable/tables.list
 '
 
@@ -1080,18 +1080,18 @@ test_expect_success 'worktree: can create shared and per-worktree ref in one tra
 	git init repo &&
 	test_commit -C repo A &&
 
-	git -C repo worktree add ../worktree &&
+	GIT_TEST_WORKTREE_SUFFIX=123 git -C repo worktree add ../worktree &&
 	git -C repo pack-refs &&
 	git -C worktree pack-refs &&
-	test_line_count = 1 repo/.git/worktrees/worktree/reftable/tables.list &&
+	test_line_count = 1 repo/.git/worktrees/worktree-123/reftable/tables.list &&
 	test_line_count = 1 repo/.git/reftable/tables.list &&
 
 	cat >stdin <<-EOF &&
-	create worktrees/worktree/refs/bisect/per-worktree HEAD
+	create worktrees/worktree-123/refs/bisect/per-worktree HEAD
 	create refs/branches/shared HEAD
 	EOF
 	git -C repo update-ref --stdin <stdin &&
-	test_line_count = 2 repo/.git/worktrees/worktree/reftable/tables.list &&
+	test_line_count = 2 repo/.git/worktrees/worktree-123/reftable/tables.list &&
 	test_line_count = 2 repo/.git/reftable/tables.list
 '
 
diff --git a/t/t1407-worktree-ref-store.sh b/t/t1407-worktree-ref-store.sh
index 48b1c92a41450b25645d6cd782aa86c8e630164b..4d081627a1e6b55936cd4051ed760fa92e7cdc02 100755
--- a/t/t1407-worktree-ref-store.sh
+++ b/t/t1407-worktree-ref-store.sh
@@ -8,12 +8,12 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
-RWT="test-tool ref-store worktree:wt"
+RWT="test-tool ref-store worktree:wt-456"
 RMAIN="test-tool ref-store worktree:main"
 
 test_expect_success 'setup' '
 	test_commit first &&
-	git worktree add -b wt-main wt &&
+	GIT_TEST_WORKTREE_SUFFIX=456 git worktree add -b wt-main wt &&
 	(
 		cd wt &&
 		test_commit second
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 246a3f46abafdf0e24528be59b33a4987ff791c1..c1b6b4d8fab8d261713a192478af12179f9bc917 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -402,12 +402,12 @@ test_expect_success 'expire with multiple worktrees' '
 		cd main-wt &&
 		test_tick &&
 		test_commit foo &&
-		git  worktree add link-wt &&
+		GIT_TEST_WORKTREE_SUFFIX=123 git  worktree add link-wt &&
 		test_tick &&
 		test_commit -C link-wt foobar &&
 		test_tick &&
 		git reflog expire --verbose --all --expire=$test_tick &&
-		test-tool ref-store worktree:link-wt for-each-reflog-ent HEAD >actual &&
+		test-tool ref-store worktree:link-wt-123 for-each-reflog-ent HEAD >actual &&
 		test_must_be_empty actual
 	)
 '
@@ -418,17 +418,17 @@ test_expect_success 'expire one of multiple worktrees' '
 		cd main-wt2 &&
 		test_tick &&
 		test_commit foo &&
-		git worktree add link-wt &&
+		GIT_TEST_WORKTREE_SUFFIX=456 git worktree add link-wt &&
 		test_tick &&
 		test_commit -C link-wt foobar &&
 		test_tick &&
-		test-tool ref-store worktree:link-wt for-each-reflog-ent HEAD \
+		test-tool ref-store worktree:link-wt-456 for-each-reflog-ent HEAD \
 			>expect-link-wt &&
 		git reflog expire --verbose --all --expire=$test_tick \
 			--single-worktree &&
 		test-tool ref-store worktree:main for-each-reflog-ent HEAD \
 			>actual-main &&
-		test-tool ref-store worktree:link-wt for-each-reflog-ent HEAD \
+		test-tool ref-store worktree:link-wt-456 for-each-reflog-ent HEAD \
 			>actual-link-wt &&
 		test_must_be_empty actual-main &&
 		test_cmp expect-link-wt actual-link-wt
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
index eb4eec8becbfa64efcde4e866334363a866c01a2..c46bf29aa4d9ceac85147d685a1d23144ec23b2b 100755
--- a/t/t1415-worktree-refs.sh
+++ b/t/t1415-worktree-refs.sh
@@ -9,8 +9,8 @@ test_expect_success 'setup' '
 	test_commit initial &&
 	test_commit wt1 &&
 	test_commit wt2 &&
-	git worktree add wt1 wt1 &&
-	git worktree add wt2 wt2 &&
+	GIT_TEST_WORKTREE_SUFFIX=123 git worktree add wt1 wt1 &&
+	GIT_TEST_WORKTREE_SUFFIX=456 git worktree add wt2 wt2 &&
 	git checkout initial &&
 	git update-ref refs/worktree/foo HEAD &&
 	git -C wt1 update-ref refs/worktree/foo HEAD &&
@@ -37,16 +37,16 @@ test_expect_success 'ambiguous main-worktree/HEAD' '
 '
 
 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_cmp_rev worktrees/wt1-123/HEAD wt1 &&
+	( cd wt1 && test_cmp_rev worktrees/wt1-123/HEAD wt1 ) &&
+	( cd wt2 && test_cmp_rev worktrees/wt1-123/HEAD wt1 )
 '
 
 test_expect_success 'ambiguous worktrees/xx/HEAD' '
-	git update-ref refs/heads/worktrees/wt1/HEAD $(git rev-parse HEAD) &&
-	test_when_finished git update-ref -d refs/heads/worktrees/wt1/HEAD &&
-	git rev-parse worktrees/wt1/HEAD 2>warn &&
-	grep "worktrees/wt1/HEAD.*ambiguous" warn
+	git update-ref refs/heads/worktrees/wt1-123/HEAD $(git rev-parse HEAD) &&
+	test_when_finished git update-ref -d refs/heads/worktrees/wt1-123/HEAD &&
+	git rev-parse worktrees/wt1-123/HEAD 2>warn &&
+	grep "worktrees/wt1-123/HEAD.*ambiguous" warn
 '
 
 test_expect_success 'reflog of main-worktree/HEAD' '
@@ -58,12 +58,12 @@ test_expect_success 'reflog of main-worktree/HEAD' '
 '
 
 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 &&
+	git -C wt2 reflog HEAD | sed "s/HEAD/worktrees\/wt2-456\/HEAD/" >expected &&
+	git reflog worktrees/wt2-456/HEAD >actual &&
 	test_cmp expected actual &&
-	git -C wt1 reflog worktrees/wt2/HEAD >actual.wt1 &&
+	git -C wt1 reflog worktrees/wt2-456/HEAD >actual.wt1 &&
 	test_cmp expected actual.wt1 &&
-	git -C wt2 reflog worktrees/wt2/HEAD >actual.wt2 &&
+	git -C wt2 reflog worktrees/wt2-456/HEAD >actual.wt2 &&
 	test_cmp expected actual.wt2
 '
 
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index 280cbf3e031e1ab67ed28aa2af4c3b105b7d254e..de25ab24fd8924e288969650a658908a5e9afd22 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -152,10 +152,10 @@ test_expect_success REFFILES 'HEAD link pointing at a funny object (from differe
 
 test_expect_success REFFILES 'other worktree HEAD link pointing at a funny object' '
 	test_when_finished "git worktree remove -f other" &&
-	git worktree add other &&
-	echo $ZERO_OID >.git/worktrees/other/HEAD &&
+	GIT_TEST_WORKTREE_SUFFIX=123 git worktree add other &&
+	echo $ZERO_OID >.git/worktrees/other-123/HEAD &&
 	test_must_fail git fsck 2>out &&
-	test_grep "worktrees/other/HEAD: detached HEAD points" out
+	test_grep "worktrees/other-123/HEAD: detached HEAD points" out
 '
 
 test_expect_success 'other worktree HEAD link pointing at missing object' '
@@ -164,7 +164,7 @@ test_expect_success 'other worktree HEAD link pointing at missing object' '
 	object_id=$(echo "Contents missing from repo" | git hash-object --stdin) &&
 	test-tool -C other ref-store main update-ref msg HEAD $object_id "" REF_NO_DEREF,REF_SKIP_OID_VERIFICATION &&
 	test_must_fail git fsck 2>out &&
-	test_grep "worktrees/other/HEAD: invalid sha1 pointer" out
+	test_grep "worktrees/other-.*/HEAD: invalid sha1 pointer" out
 '
 
 test_expect_success 'other worktree HEAD link pointing at a funny place' '
@@ -172,7 +172,7 @@ test_expect_success 'other worktree HEAD link pointing at a funny place' '
 	git worktree add other &&
 	git -C other symbolic-ref HEAD refs/funny/place &&
 	test_must_fail git fsck 2>out &&
-	test_grep "worktrees/other/HEAD points to something strange" out
+	test_grep "worktrees/other-.*/HEAD points to something strange" out
 '
 
 test_expect_success 'commit with multiple signatures is okay' '
@@ -1033,7 +1033,7 @@ test_expect_success 'fsck error on gitattributes with excessive size' '
 
 test_expect_success 'fsck detects problems in worktree index' '
 	test_when_finished "git worktree remove -f wt" &&
-	git worktree add wt &&
+	GIT_TEST_WORKTREE_SUFFIX=123 git worktree add wt &&
 
 	echo "this will be removed to break the worktree index" >wt/file &&
 	git -C wt add file &&
@@ -1042,7 +1042,7 @@ test_expect_success 'fsck detects problems in worktree index' '
 
 	test_must_fail git fsck --name-objects >actual 2>&1 &&
 	cat >expect <<-EOF &&
-	missing blob $blob (.git/worktrees/wt/index:file)
+	missing blob $blob (.git/worktrees/wt-123/index:file)
 	EOF
 	test_cmp expect actual
 '
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 30c31918fde6539d52800e18dfbb3423b5b73491..49e0ac68858d922336a498caaa743f2f4011d28a 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -80,7 +80,7 @@ test_expect_success 'setup' '
 	git checkout -b side &&
 	test_commit def &&
 	git checkout main &&
-	git worktree add worktree side
+	GIT_TEST_WORKTREE_SUFFIX=123 git worktree add worktree side
 '
 
 test_rev_parse toplevel false false true '' .git "$ROOT/.git"
@@ -113,7 +113,7 @@ test_expect_success 'rev-parse --path-format=absolute' '
 	test_one "." "$ROOT/.git" --path-format=absolute --git-common-dir &&
 	test_one "sub/dir" "$ROOT/.git" --path-format=absolute --git-dir &&
 	test_one "sub/dir" "$ROOT/.git" --path-format=absolute --git-common-dir &&
-	test_one "worktree" "$ROOT/.git/worktrees/worktree" --path-format=absolute --git-dir &&
+	test_one "worktree" "$ROOT/.git/worktrees/worktree-123" --path-format=absolute --git-dir &&
 	test_one "worktree" "$ROOT/.git" --path-format=absolute --git-common-dir &&
 	test_one "." "$ROOT" --path-format=absolute --show-toplevel &&
 	test_one "." "$ROOT/.git/objects" --path-format=absolute --git-path objects &&
@@ -125,7 +125,7 @@ test_expect_success 'rev-parse --path-format=relative' '
 	test_one "." ".git" --path-format=relative --git-common-dir &&
 	test_one "sub/dir" "../../.git" --path-format=relative --git-dir &&
 	test_one "sub/dir" "../../.git" --path-format=relative --git-common-dir &&
-	test_one "worktree" "../.git/worktrees/worktree" --path-format=relative --git-dir &&
+	test_one "worktree" "../.git/worktrees/worktree-123" --path-format=relative --git-dir &&
 	test_one "worktree" "../.git" --path-format=relative --git-common-dir &&
 	test_one "." "./" --path-format=relative --show-toplevel &&
 	test_one "." ".git/objects" --path-format=relative --git-path objects &&
diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
index bc4f4e90d6ecfedbde9082bda6f9e4eec3e3575d..33262b49f18521c805f188a10f944dbfa9f285ba 100755
--- a/t/t2400-worktree-add.sh
+++ b/t/t2400-worktree-add.sh
@@ -71,21 +71,21 @@ test_expect_success '"add" worktree' '
 test_expect_success '"add" worktree with lock' '
 	git worktree add --detach --lock here-with-lock main &&
 	test_when_finished "git worktree unlock here-with-lock || :" &&
-	test -f .git/worktrees/here-with-lock/locked
+	test -f .git/worktrees/here-with-lock-*/locked
 '
 
 test_expect_success '"add" worktree with lock and reason' '
 	lock_reason="why not" &&
 	git worktree add --detach --lock --reason "$lock_reason" here-with-lock-reason main &&
 	test_when_finished "git worktree unlock here-with-lock-reason || :" &&
-	test -f .git/worktrees/here-with-lock-reason/locked &&
+	test -f .git/worktrees/here-with-lock-reason-*/locked &&
 	echo "$lock_reason" >expect &&
-	test_cmp expect .git/worktrees/here-with-lock-reason/locked
+	test_cmp expect .git/worktrees/here-with-lock-reason-*/locked
 '
 
 test_expect_success '"add" worktree with reason but no lock' '
 	test_must_fail git worktree add --detach --reason "why not" here-with-reason-only main &&
-	test_path_is_missing .git/worktrees/here-with-reason-only/locked
+	test_path_is_missing .git/worktrees/here-with-reason-only-*/locked
 '
 
 test_expect_success '"add" worktree from a subdir' '
@@ -413,16 +413,16 @@ test_expect_success '"add --orphan" with empty repository' '
 test_expect_success '"add" worktree with orphan branch and lock' '
 	git worktree add --lock --orphan -b orphanbr orphan-with-lock &&
 	test_when_finished "git worktree unlock orphan-with-lock || :" &&
-	test -f .git/worktrees/orphan-with-lock/locked
+	test -f .git/worktrees/orphan-with-lock-*/locked
 '
 
 test_expect_success '"add" worktree with orphan branch, lock, and reason' '
 	lock_reason="why not" &&
 	git worktree add --detach --lock --reason "$lock_reason" orphan-with-lock-reason main &&
 	test_when_finished "git worktree unlock orphan-with-lock-reason || :" &&
-	test -f .git/worktrees/orphan-with-lock-reason/locked &&
+	test -f .git/worktrees/orphan-with-lock-reason-*/locked &&
 	echo "$lock_reason" >expect &&
-	test_cmp expect .git/worktrees/orphan-with-lock-reason/locked
+	test_cmp expect .git/worktrees/orphan-with-lock-reason-*/locked
 '
 
 # Note: Quoted arguments containing spaces are not supported.
@@ -1088,10 +1088,10 @@ test_expect_success '"add" invokes post-checkout hook (branch)' '
 	post_checkout_hook &&
 	{
 		echo $ZERO_OID $(git rev-parse HEAD) 1 &&
-		echo $(pwd)/.git/worktrees/gumby &&
+		echo $(pwd)/.git/worktrees/gumby-123 &&
 		echo $(pwd)/gumby
 	} >hook.expect &&
-	git worktree add gumby &&
+	GIT_TEST_WORKTREE_SUFFIX="123" git worktree add gumby &&
 	test_cmp hook.expect gumby/hook.actual
 '
 
@@ -1099,10 +1099,10 @@ test_expect_success '"add" invokes post-checkout hook (detached)' '
 	post_checkout_hook &&
 	{
 		echo $ZERO_OID $(git rev-parse HEAD) 1 &&
-		echo $(pwd)/.git/worktrees/grumpy &&
+		echo $(pwd)/.git/worktrees/grumpy-456 &&
 		echo $(pwd)/grumpy
 	} >hook.expect &&
-	git worktree add --detach grumpy &&
+	GIT_TEST_WORKTREE_SUFFIX="456" git worktree add --detach grumpy &&
 	test_cmp hook.expect grumpy/hook.actual
 '
 
@@ -1117,10 +1117,10 @@ test_expect_success '"add" in other worktree invokes post-checkout hook' '
 	post_checkout_hook &&
 	{
 		echo $ZERO_OID $(git rev-parse HEAD) 1 &&
-		echo $(pwd)/.git/worktrees/guppy &&
+		echo $(pwd)/.git/worktrees/guppy-789 &&
 		echo $(pwd)/guppy
 	} >hook.expect &&
-	git -C gloopy worktree add --detach ../guppy &&
+	GIT_TEST_WORKTREE_SUFFIX="789" git -C gloopy worktree add --detach ../guppy &&
 	test_cmp hook.expect guppy/hook.actual
 '
 
@@ -1129,11 +1129,11 @@ test_expect_success '"add" in bare repo invokes post-checkout hook' '
 	git clone --bare . bare &&
 	{
 		echo $ZERO_OID $(git --git-dir=bare rev-parse HEAD) 1 &&
-		echo $(pwd)/bare/worktrees/goozy &&
+		echo $(pwd)/bare/worktrees/goozy-651 &&
 		echo $(pwd)/goozy
 	} >hook.expect &&
 	post_checkout_hook bare &&
-	git -C bare worktree add --detach ../goozy &&
+	GIT_TEST_WORKTREE_SUFFIX="651" git -C bare worktree add --detach ../goozy &&
 	test_cmp hook.expect goozy/hook.actual
 '
 
@@ -1165,8 +1165,9 @@ test_expect_success '"add" not tripped up by magic worktree matching"' '
 '
 
 test_expect_success FUNNYNAMES 'sanitize generated worktree name' '
-	git worktree add --detach ".  weird*..?.lock.lock" &&
-	test -d .git/worktrees/---weird-.-
+	GIT_TEST_WORKTREE_SUFFIX="1234" \
+		git worktree add --detach ".  weird*..?.lock.lock" &&
+	test -d .git/worktrees/---weird-.--1234
 '
 
 test_expect_success '"add" should not fail because of another bad worktree' '
@@ -1210,23 +1211,23 @@ test_expect_success '"add" with initialized submodule, with submodule.recurse se
 test_expect_success 'can create worktrees with relative paths' '
 	test_when_finished "git worktree remove relative" &&
 	test_config worktree.useRelativePaths false &&
-	git worktree add --relative-paths ./relative &&
-	echo "gitdir: ../.git/worktrees/relative" >expect &&
+	GIT_TEST_WORKTREE_SUFFIX=123 git worktree add --relative-paths ./relative &&
+	echo "gitdir: ../.git/worktrees/relative-123" >expect &&
 	test_cmp expect relative/.git &&
 	echo "../../../relative/.git" >expect &&
-	test_cmp expect .git/worktrees/relative/gitdir
+	test_cmp expect .git/worktrees/relative-123/gitdir
 '
 
 test_expect_success 'can create worktrees with absolute paths' '
 	test_config worktree.useRelativePaths true &&
-	git worktree add ./relative &&
-	echo "gitdir: ../.git/worktrees/relative" >expect &&
+	GIT_TEST_WORKTREE_SUFFIX=123 git worktree add ./relative &&
+	echo "gitdir: ../.git/worktrees/relative-123" >expect &&
 	test_cmp expect relative/.git &&
-	git worktree add --no-relative-paths ./absolute &&
-	echo "gitdir: $(pwd)/.git/worktrees/absolute" >expect &&
+	GIT_TEST_WORKTREE_SUFFIX=456 git worktree add --no-relative-paths ./absolute &&
+	echo "gitdir: $(pwd)/.git/worktrees/absolute-456" >expect &&
 	test_cmp expect absolute/.git &&
 	echo "$(pwd)/absolute/.git" >expect &&
-	test_cmp expect .git/worktrees/absolute/gitdir
+	test_cmp expect .git/worktrees/absolute-456/gitdir
 '
 
 test_expect_success 'move repo without breaking relative internal links' '
diff --git a/t/t2401-worktree-prune.sh b/t/t2401-worktree-prune.sh
index 5eb52b9abbf29514dc082c260ebb7a5e8e63aae0..856fcdd19d376d3448ee0be46592fe68d44617c4 100755
--- a/t/t2401-worktree-prune.sh
+++ b/t/t2401-worktree-prune.sh
@@ -83,29 +83,29 @@ test_expect_success 'not prune locked checkout' '
 test_expect_success 'not prune recent checkouts' '
 	test_when_finished rm -r .git/worktrees &&
 	git worktree add jlm HEAD &&
-	test -d .git/worktrees/jlm &&
+	test -d .git/worktrees/jlm-* &&
 	rm -rf jlm &&
 	git worktree prune --verbose --expire=2.days.ago &&
-	test -d .git/worktrees/jlm
+	test -d .git/worktrees/jlm-*
 '
 
 test_expect_success 'not prune proper checkouts' '
 	test_when_finished rm -r .git/worktrees &&
 	git worktree add --detach "$PWD/nop" main &&
 	git worktree prune &&
-	test -d .git/worktrees/nop
+	test -d .git/worktrees/nop-*
 '
 
 test_expect_success 'prune duplicate (linked/linked)' '
 	test_when_finished rm -fr .git/worktrees w1 w2 &&
-	git worktree add --detach w1 &&
-	git worktree add --detach w2 &&
-	sed "s/w2/w1/" .git/worktrees/w2/gitdir >.git/worktrees/w2/gitdir.new &&
-	mv .git/worktrees/w2/gitdir.new .git/worktrees/w2/gitdir &&
+	GIT_TEST_WORKTREE_SUFFIX=1 git worktree add --detach w1 &&
+	GIT_TEST_WORKTREE_SUFFIX=2 git worktree add --detach w2 &&
+	sed "s/w2/w1/" .git/worktrees/w2-2/gitdir >.git/worktrees/w2-2/gitdir.new &&
+	mv .git/worktrees/w2-2/gitdir.new .git/worktrees/w2-2/gitdir &&
 	git worktree prune --verbose 2>actual &&
 	test_grep "duplicate entry" actual &&
-	test -d .git/worktrees/w1 &&
-	! test -d .git/worktrees/w2
+	test -d .git/worktrees/w1-1 &&
+	! test -d .git/worktrees/w2-2
 '
 
 test_expect_success 'prune duplicate (main/linked)' '
@@ -117,7 +117,7 @@ test_expect_success 'prune duplicate (main/linked)' '
 	mv repo wt &&
 	git -C wt worktree prune --verbose 2>actual &&
 	test_grep "duplicate entry" actual &&
-	! test -d .git/worktrees/wt
+	! test -d .git/worktrees/wt-*
 '
 
 test_expect_success 'not prune proper worktrees inside linked worktree with relative paths' '
diff --git a/t/t2403-worktree-move.sh b/t/t2403-worktree-move.sh
index 422c1a05580057b18ab8bfdfe38da4d723749493..ba3f05c16a4969fb84d98052ae375ef162f3e73a 100755
--- a/t/t2403-worktree-move.sh
+++ b/t/t2403-worktree-move.sh
@@ -24,27 +24,27 @@ test_expect_success 'lock main worktree' '
 test_expect_success 'lock linked worktree' '
 	git worktree lock --reason hahaha source &&
 	echo hahaha >expected &&
-	test_cmp expected .git/worktrees/source/locked
+	test_cmp expected .git/worktrees/source-*/locked
 '
 
 test_expect_success 'lock linked worktree from another worktree' '
-	rm .git/worktrees/source/locked &&
+	rm .git/worktrees/source-*/locked &&
 	git worktree add elsewhere &&
 	git -C elsewhere worktree lock --reason hahaha ../source &&
 	echo hahaha >expected &&
-	test_cmp expected .git/worktrees/source/locked
+	test_cmp expected .git/worktrees/source-*/locked
 '
 
 test_expect_success 'lock worktree twice' '
 	test_must_fail git worktree lock source &&
 	echo hahaha >expected &&
-	test_cmp expected .git/worktrees/source/locked
+	test_cmp expected .git/worktrees/source-*/locked
 '
 
 test_expect_success 'lock worktree twice (from the locked worktree)' '
 	test_must_fail git -C source worktree lock . &&
 	echo hahaha >expected &&
-	test_cmp expected .git/worktrees/source/locked
+	test_cmp expected .git/worktrees/source-*/locked
 '
 
 test_expect_success 'unlock main worktree' '
@@ -183,19 +183,19 @@ test_expect_success 'force remove worktree with untracked file' '
 
 test_expect_success 'remove missing worktree' '
 	git worktree add to-be-gone &&
-	test -d .git/worktrees/to-be-gone &&
+	test -d .git/worktrees/to-be-gone-* &&
 	mv to-be-gone gone &&
 	git worktree remove to-be-gone &&
-	test_path_is_missing .git/worktrees/to-be-gone
+	test_path_is_missing .git/worktrees/to-be-gone-*
 '
 
 test_expect_success 'NOT remove missing-but-locked worktree' '
 	git worktree add gone-but-locked &&
 	git worktree lock gone-but-locked &&
-	test -d .git/worktrees/gone-but-locked &&
+	test -d .git/worktrees/gone-but-locked-* &&
 	mv gone-but-locked really-gone-now &&
 	test_must_fail git worktree remove gone-but-locked &&
-	test_path_is_dir .git/worktrees/gone-but-locked
+	test_path_is_dir .git/worktrees/gone-but-locked-*
 '
 
 test_expect_success 'proper error when worktree not found' '
@@ -249,27 +249,27 @@ test_expect_success 'not remove a repo with initialized submodule' '
 
 test_expect_success 'move worktree with absolute path to relative path' '
 	test_config worktree.useRelativePaths false &&
-	git worktree add ./absolute &&
+	GIT_TEST_WORKTREE_SUFFIX=123 git worktree add ./absolute &&
 	git worktree move --relative-paths absolute relative &&
-	echo "gitdir: ../.git/worktrees/absolute" >expect &&
+	echo "gitdir: ../.git/worktrees/absolute-123" >expect &&
 	test_cmp expect relative/.git &&
 	echo "../../../relative/.git" >expect &&
-	test_cmp expect .git/worktrees/absolute/gitdir &&
+	test_cmp expect .git/worktrees/absolute-123/gitdir &&
 	test_config worktree.useRelativePaths true &&
 	git worktree move relative relative2 &&
-	echo "gitdir: ../.git/worktrees/absolute" >expect &&
+	echo "gitdir: ../.git/worktrees/absolute-123" >expect &&
 	test_cmp expect relative2/.git &&
 	echo "../../../relative2/.git" >expect &&
-	test_cmp expect .git/worktrees/absolute/gitdir
+	test_cmp expect .git/worktrees/absolute-123/gitdir
 '
 
 test_expect_success 'move worktree with relative path to absolute path' '
 	test_config worktree.useRelativePaths true &&
 	git worktree move --no-relative-paths relative2 absolute &&
-	echo "gitdir: $(pwd)/.git/worktrees/absolute" >expect &&
+	echo "gitdir: $(pwd)/.git/worktrees/absolute-123" >expect &&
 	test_cmp expect absolute/.git &&
 	echo "$(pwd)/absolute/.git" >expect &&
-	test_cmp expect .git/worktrees/absolute/gitdir
+	test_cmp expect .git/worktrees/absolute-123/gitdir
 '
 
 test_done
diff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh
index 1d7f60563387f9c2f53dfc3a79ac0289afe57611..5479b2a74aa0d9b1e8880ed7c038307ffa1d0c54 100755
--- a/t/t2405-worktree-submodule.sh
+++ b/t/t2405-worktree-submodule.sh
@@ -9,6 +9,7 @@ TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 base_path=$(pwd -P)
+suffix=4567
 
 test_expect_success 'setup: create origin repos'  '
 	git config --global protocol.file.allow always &&
@@ -61,9 +62,10 @@ test_expect_success 'submodule is checked out after manually adding submodule wo
 '
 
 test_expect_success 'checkout --recurse-submodules uses $GIT_DIR for submodules in a linked worktree' '
-	git -C main worktree add "$base_path/checkout-recurse" --detach  &&
+	GIT_TEST_WORKTREE_SUFFIX=$suffix \
+		git -C main worktree add "$base_path/checkout-recurse" --detach  &&
 	git -C checkout-recurse submodule update --init &&
-	echo "gitdir: ../../main/.git/worktrees/checkout-recurse/modules/sub" >expect-gitfile &&
+	echo "gitdir: ../../main/.git/worktrees/checkout-recurse-$suffix/modules/sub" >expect-gitfile &&
 	cat checkout-recurse/sub/.git >actual-gitfile &&
 	test_cmp expect-gitfile actual-gitfile &&
 	git -C main/sub rev-parse HEAD >expect-head-main &&
@@ -82,14 +84,14 @@ test_expect_success 'core.worktree is removed in $GIT_DIR/modules/<name>/config,
 	git -C checkout-recurse/sub config --get core.worktree >actual-linked &&
 	test_cmp expect-linked actual-linked &&
 	git -C checkout-recurse checkout --recurse-submodules first &&
-	test_expect_code 1 git -C main/.git/worktrees/checkout-recurse/modules/sub config --get core.worktree >linked-config &&
+	test_expect_code 1 git -C main/.git/worktrees/checkout-recurse-$suffix/modules/sub config --get core.worktree >linked-config &&
 	test_must_be_empty linked-config &&
 	git -C main/sub config --get core.worktree >actual-main &&
 	test_cmp expect-main actual-main
 '
 
 test_expect_success 'unsetting core.worktree does not prevent running commands directly against the submodule repository' '
-	git -C main/.git/worktrees/checkout-recurse/modules/sub log
+	git -C main/.git/worktrees/checkout-recurse-$suffix/modules/sub log
 '
 
 test_done
diff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh
index 49b70b999518d47e1edd72a61a847b427f4c67a1..49d020f5fe786014ddc428bcb74cb706f8cef3d1 100755
--- a/t/t2406-worktree-repair.sh
+++ b/t/t2406-worktree-repair.sh
@@ -106,8 +106,8 @@ test_expect_success 'repo not found; .git not file' '
 
 test_expect_success 'repo not found; .git not referencing repo' '
 	test_when_finished "rm -rf side not-a-repo && git worktree prune" &&
-	git worktree add --detach side &&
-	sed s,\.git/worktrees/side$,not-a-repo, side/.git >side/.newgit &&
+	GIT_TEST_WORKTREE_SUFFIX=1234 git worktree add --detach side &&
+	sed s,\.git/worktrees/side-1234$,not-a-repo, side/.git >side/.newgit &&
 	mv side/.newgit side/.git &&
 	mkdir not-a-repo &&
 	test_must_fail git worktree repair side 2>err &&
@@ -127,41 +127,41 @@ test_expect_success 'repo not found; .git file broken' '
 test_expect_success 'repair broken gitdir' '
 	test_when_finished "rm -rf orig moved && git worktree prune" &&
 	git worktree add --detach orig &&
-	sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&
-	rm .git/worktrees/orig/gitdir &&
+	sed s,orig/\.git$,moved/.git, .git/worktrees/orig-*/gitdir >expect &&
+	rm .git/worktrees/orig-*/gitdir &&
 	mv orig moved &&
 	git worktree repair moved 2>err &&
-	test_cmp expect .git/worktrees/orig/gitdir &&
+	test_cmp expect .git/worktrees/orig-*/gitdir &&
 	test_grep "gitdir unreadable" err
 '
 
 test_expect_success 'repair incorrect gitdir' '
 	test_when_finished "rm -rf orig moved && git worktree prune" &&
 	git worktree add --detach orig &&
-	sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&
+	sed s,orig/\.git$,moved/.git, .git/worktrees/orig-*/gitdir >expect &&
 	mv orig moved &&
 	git worktree repair moved 2>err &&
-	test_cmp expect .git/worktrees/orig/gitdir &&
+	test_cmp expect .git/worktrees/orig-*/gitdir &&
 	test_grep "gitdir incorrect" err
 '
 
 test_expect_success 'repair gitdir (implicit) from linked worktree' '
 	test_when_finished "rm -rf orig moved && git worktree prune" &&
 	git worktree add --detach orig &&
-	sed s,orig/\.git$,moved/.git, .git/worktrees/orig/gitdir >expect &&
+	sed s,orig/\.git$,moved/.git, .git/worktrees/orig-*/gitdir >expect &&
 	mv orig moved &&
 	git -C moved worktree repair 2>err &&
-	test_cmp expect .git/worktrees/orig/gitdir &&
+	test_cmp expect .git/worktrees/orig-*/gitdir &&
 	test_grep "gitdir incorrect" err
 '
 
 test_expect_success 'unable to repair gitdir (implicit) from main worktree' '
 	test_when_finished "rm -rf orig moved && git worktree prune" &&
 	git worktree add --detach orig &&
-	cat .git/worktrees/orig/gitdir >expect &&
+	cat .git/worktrees/orig-*/gitdir >expect &&
 	mv orig moved &&
 	git worktree repair 2>err &&
-	test_cmp expect .git/worktrees/orig/gitdir &&
+	test_cmp expect .git/worktrees/orig-*/gitdir &&
 	test_must_be_empty err
 '
 
@@ -170,15 +170,15 @@ test_expect_success 'repair multiple gitdir files' '
 		git worktree prune" &&
 	git worktree add --detach orig1 &&
 	git worktree add --detach orig2 &&
-	sed s,orig1/\.git$,moved1/.git, .git/worktrees/orig1/gitdir >expect1 &&
-	sed s,orig2/\.git$,moved2/.git, .git/worktrees/orig2/gitdir >expect2 &&
+	sed s,orig1/\.git$,moved1/.git, .git/worktrees/orig1-*/gitdir >expect1 &&
+	sed s,orig2/\.git$,moved2/.git, .git/worktrees/orig2-*/gitdir >expect2 &&
 	mv orig1 moved1 &&
 	mv orig2 moved2 &&
 	git worktree repair moved1 moved2 2>err &&
-	test_cmp expect1 .git/worktrees/orig1/gitdir &&
-	test_cmp expect2 .git/worktrees/orig2/gitdir &&
-	test_grep "gitdir incorrect:.*orig1/gitdir$" err &&
-	test_grep "gitdir incorrect:.*orig2/gitdir$" err
+	test_cmp expect1 .git/worktrees/orig1-*/gitdir &&
+	test_cmp expect2 .git/worktrees/orig2-*/gitdir &&
+	test_grep "gitdir incorrect:.*orig1-.*/gitdir$" err &&
+	test_grep "gitdir incorrect:.*orig2-.*/gitdir$" err
 '
 
 test_expect_success 'repair moved main and linked worktrees' '
@@ -186,14 +186,12 @@ test_expect_success 'repair moved main and linked worktrees' '
 	test_create_repo main &&
 	test_commit -C main init &&
 	git -C main worktree add --detach ../side &&
-	sed "s,side/\.git$,sidemoved/.git," \
-		main/.git/worktrees/side/gitdir >expect-gitdir &&
-	sed "s,main/.git/worktrees/side$,mainmoved/.git/worktrees/side," \
-		side/.git >expect-gitfile &&
+	sed "s,side,sidemoved," main/.git/worktrees/side-*/gitdir >expect-gitdir &&
+	sed "s,main,mainmoved," side/.git >expect-gitfile &&
 	mv main mainmoved &&
 	mv side sidemoved &&
 	git -C mainmoved worktree repair ../sidemoved &&
-	test_cmp expect-gitdir mainmoved/.git/worktrees/side/gitdir &&
+	test_cmp expect-gitdir mainmoved/.git/worktrees/side-*/gitdir &&
 	test_cmp expect-gitfile sidemoved/.git
 '
 
@@ -203,16 +201,15 @@ test_expect_success 'repair copied main and linked worktrees' '
 	git -C orig init main &&
 	test_commit -C orig/main nothing &&
 	git -C orig/main worktree add ../linked &&
-	cp orig/main/.git/worktrees/linked/gitdir orig/main.expect &&
+	cp orig/main/.git/worktrees/linked-*/gitdir orig/main.expect &&
 	cp orig/linked/.git orig/linked.expect &&
 	cp -R orig dup &&
 	sed "s,orig/linked/\.git$,dup/linked/.git," orig/main.expect >dup/main.expect &&
-	sed "s,orig/main/\.git/worktrees/linked$,dup/main/.git/worktrees/linked," \
-		orig/linked.expect >dup/linked.expect &&
+	sed "s,orig,dup," orig/linked.expect >dup/linked.expect &&
 	git -C dup/main worktree repair ../linked &&
-	test_cmp orig/main.expect orig/main/.git/worktrees/linked/gitdir &&
+	test_cmp orig/main.expect orig/main/.git/worktrees/linked-*/gitdir &&
 	test_cmp orig/linked.expect orig/linked/.git &&
-	test_cmp dup/main.expect dup/main/.git/worktrees/linked/gitdir &&
+	test_cmp dup/main.expect dup/main/.git/worktrees/linked-*/gitdir &&
 	test_cmp dup/linked.expect dup/linked/.git
 '
 
@@ -221,11 +218,11 @@ test_expect_success 'repair worktree with relative path with missing gitfile' '
 	test_create_repo main &&
 	git -C main config worktree.useRelativePaths true &&
 	test_commit -C main init &&
-	git -C main worktree add --detach ../wt &&
+	GIT_TEST_WORKTREE_SUFFIX=123 git -C main worktree add --detach ../wt &&
 	rm wt/.git &&
 	test_path_is_missing wt/.git &&
 	git -C main worktree repair &&
-	echo "gitdir: ../main/.git/worktrees/wt" >expect &&
+	echo "gitdir: ../main/.git/worktrees/wt-123" >expect &&
 	test_cmp expect wt/.git
 '
 
@@ -233,12 +230,12 @@ test_expect_success 'repair absolute worktree to use relative paths' '
 	test_when_finished "rm -rf main side sidemoved" &&
 	test_create_repo main &&
 	test_commit -C main init &&
-	git -C main worktree add --detach ../side &&
+	GIT_TEST_WORKTREE_SUFFIX=456 git -C main worktree add --detach ../side &&
 	echo "../../../../sidemoved/.git" >expect-gitdir &&
-	echo "gitdir: ../main/.git/worktrees/side" >expect-gitfile &&
+	echo "gitdir: ../main/.git/worktrees/side-456" >expect-gitfile &&
 	mv side sidemoved &&
 	git -C main worktree repair --relative-paths ../sidemoved &&
-	test_cmp expect-gitdir main/.git/worktrees/side/gitdir &&
+	test_cmp expect-gitdir main/.git/worktrees/side-456/gitdir &&
 	test_cmp expect-gitfile sidemoved/.git
 '
 
@@ -246,13 +243,39 @@ test_expect_success 'repair relative worktree to use absolute paths' '
 	test_when_finished "rm -rf main side sidemoved" &&
 	test_create_repo main &&
 	test_commit -C main init &&
-	git -C main worktree add --relative-paths --detach ../side &&
+	GIT_TEST_WORKTREE_SUFFIX=789 git -C main worktree add --relative-paths --detach ../side &&
 	echo "$(pwd)/sidemoved/.git" >expect-gitdir &&
-	echo "gitdir: $(pwd)/main/.git/worktrees/side" >expect-gitfile &&
+	echo "gitdir: $(pwd)/main/.git/worktrees/side-789" >expect-gitfile &&
 	mv side sidemoved &&
 	git -C main worktree repair ../sidemoved &&
-	test_cmp expect-gitdir main/.git/worktrees/side/gitdir &&
+	test_cmp expect-gitdir main/.git/worktrees/side-789/gitdir &&
 	test_cmp expect-gitfile sidemoved/.git
 '
 
+test_expect_success 'does not repair worktrees from another repo' '
+	test_when_finished "rm -rf repo1 repo2" &&
+	mkdir -p repo1 &&
+	git -C repo1 init main &&
+	test_commit -C repo1/main nothing &&
+	git -C repo1/main worktree add ../linked &&
+	cp repo1/main/.git/worktrees/linked-*/gitdir repo1/main.expect &&
+	cp repo1/linked/.git repo1/linked.expect &&
+	mkdir -p repo2 &&
+	git -C repo2 init main &&
+	test_commit -C repo2/main nothing &&
+	git -C repo2/main worktree add ../linked &&
+	cp repo2/main/.git/worktrees/linked-*/gitdir repo2/main.expect &&
+	cp repo2/linked/.git repo2/linked.expect &&
+	git -C repo1/main worktree repair ../../repo2/linked &&
+	test_cmp repo1/main.expect repo1/main/.git/worktrees/linked-*/gitdir &&
+	test_cmp repo1/linked.expect repo1/linked/.git &&
+	test_cmp repo2/main.expect repo2/main/.git/worktrees/linked-*/gitdir &&
+	test_cmp repo2/linked.expect repo2/linked/.git &&
+	git -C repo2/main worktree repair ../../repo1/linked &&
+	test_cmp repo1/main.expect repo1/main/.git/worktrees/linked-*/gitdir &&
+	test_cmp repo1/linked.expect repo1/linked/.git &&
+	test_cmp repo2/main.expect repo2/main/.git/worktrees/linked-*/gitdir &&
+	test_cmp repo2/linked.expect repo2/linked/.git
+'
+
 test_done
diff --git a/t/t2407-worktree-heads.sh b/t/t2407-worktree-heads.sh
index f6835c91dcc49cfeb23881fe0ef7a96629bfb2e6..1587dadfd1e1fa122edccc62ffd9aa4c20f0ec80 100755
--- a/t/t2407-worktree-heads.sh
+++ b/t/t2407-worktree-heads.sh
@@ -19,7 +19,8 @@ test_expect_success 'setup' '
 		test_commit $i &&
 		git branch wt-$i &&
 		git branch fake-$i &&
-		git worktree add wt-$i wt-$i || return 1
+		GIT_TEST_WORKTREE_SUFFIX=$i \
+			git worktree add wt-$i wt-$i || return 1
 	done &&
 
 	# Create a server that updates each branch by one commit
@@ -132,20 +133,20 @@ test_expect_success 'refuse to overwrite when in error states' '
 	test_when_finished rm -rf .git/worktrees/wt-*/BISECT_* &&
 
 	# Both branches are currently under rebase.
-	mkdir -p .git/worktrees/wt-3/rebase-merge &&
-	touch .git/worktrees/wt-3/rebase-merge/interactive &&
-	echo refs/heads/fake-1 >.git/worktrees/wt-3/rebase-merge/head-name &&
-	echo refs/heads/fake-2 >.git/worktrees/wt-3/rebase-merge/onto &&
-	mkdir -p .git/worktrees/wt-4/rebase-merge &&
-	touch .git/worktrees/wt-4/rebase-merge/interactive &&
-	echo refs/heads/fake-2 >.git/worktrees/wt-4/rebase-merge/head-name &&
-	echo refs/heads/fake-1 >.git/worktrees/wt-4/rebase-merge/onto &&
+	mkdir -p .git/worktrees/wt-3-3/rebase-merge &&
+	touch .git/worktrees/wt-3-3/rebase-merge/interactive &&
+	echo refs/heads/fake-1 >.git/worktrees/wt-3-3/rebase-merge/head-name &&
+	echo refs/heads/fake-2 >.git/worktrees/wt-3-3/rebase-merge/onto &&
+	mkdir -p .git/worktrees/wt-4-4/rebase-merge &&
+	touch .git/worktrees/wt-4-4/rebase-merge/interactive &&
+	echo refs/heads/fake-2 >.git/worktrees/wt-4-4/rebase-merge/head-name &&
+	echo refs/heads/fake-1 >.git/worktrees/wt-4-4/rebase-merge/onto &&
 
 	# Both branches are currently under bisect.
-	touch .git/worktrees/wt-4/BISECT_LOG &&
-	echo refs/heads/fake-2 >.git/worktrees/wt-4/BISECT_START &&
-	touch .git/worktrees/wt-1/BISECT_LOG &&
-	echo refs/heads/fake-1 >.git/worktrees/wt-1/BISECT_START &&
+	touch .git/worktrees/wt-4-4/BISECT_LOG &&
+	echo refs/heads/fake-2 >.git/worktrees/wt-4-4/BISECT_START &&
+	touch .git/worktrees/wt-1-1/BISECT_LOG &&
+	echo refs/heads/fake-1 >.git/worktrees/wt-1-1/BISECT_START &&
 
 	for i in 1 2
 	do
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index ccfa6a720d090c2f7f2a085f60065bdcfaf8d1d9..e44497ac94394119662115b1f6aa035c7f0565d2 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -246,14 +246,14 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou
 '
 
 test_expect_success REFFILES 'git branch -M fails if updating any linked working tree fails' '
-	git worktree add -b baz bazdir1 &&
-	git worktree add -f bazdir2 baz &&
-	touch .git/worktrees/bazdir1/HEAD.lock &&
+	GIT_TEST_WORKTREE_SUFFIX=123 git worktree add -b baz bazdir1 &&
+	GIT_TEST_WORKTREE_SUFFIX=456 git worktree add -f bazdir2 baz &&
+	touch .git/worktrees/bazdir1-123/HEAD.lock &&
 	test_must_fail git branch -M baz bam &&
 	test $(git -C bazdir2 rev-parse --abbrev-ref HEAD) = bam &&
 	git branch -M bam baz &&
-	rm .git/worktrees/bazdir1/HEAD.lock &&
-	touch .git/worktrees/bazdir2/HEAD.lock &&
+	rm .git/worktrees/bazdir1-123/HEAD.lock &&
+	touch .git/worktrees/bazdir2-456/HEAD.lock &&
 	test_must_fail git branch -M baz bam &&
 	test $(git -C bazdir1 rev-parse --abbrev-ref HEAD) = bam &&
 	rm -rf bazdir1 bazdir2 &&
diff --git a/t/t5304-prune.sh b/t/t5304-prune.sh
index e641df0116c24404e4892a0e30af4ef4bf8db493..0e98c6627a98ed197d7ab1ff41e8dd41eeaff3ac 100755
--- a/t/t5304-prune.sh
+++ b/t/t5304-prune.sh
@@ -293,7 +293,7 @@ test_expect_success 'prune: handle HEAD in multiple worktrees' '
 	echo "new blob for third-worktree" >third-worktree/blob &&
 	git -C third-worktree add blob &&
 	git -C third-worktree commit -m "third" &&
-	rm .git/worktrees/third-worktree/index &&
+	rm .git/worktrees/third-worktree-*/index &&
 	test_must_fail git -C third-worktree show :blob &&
 	git prune --expire=now &&
 	git -C third-worktree show HEAD:blob >actual &&
diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
index f77832185765585e2bda1677f8cbbe13841127f7..acf9544e35966f054cbc90c37e84377c9f461f2b 100755
--- a/t/t7412-submodule-absorbgitdirs.sh
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -123,7 +123,7 @@ test_expect_success 'absorb the git dir outside of primary worktree' '
 	test_when_finished "rm -rf repo-bare.git" &&
 	git clone --bare . repo-bare.git &&
 	test_when_finished "rm -rf repo-wt" &&
-	git -C repo-bare.git worktree add ../repo-wt &&
+	GIT_TEST_WORKTREE_SUFFIX=123 git -C repo-bare.git worktree add ../repo-wt &&
 
 	test_when_finished "rm -f .gitconfig" &&
 	test_config_global protocol.file.allow always &&
@@ -134,7 +134,7 @@ test_expect_success 'absorb the git dir outside of primary worktree' '
 	cat >expect <<-EOF &&
 	Migrating git directory of '\''sub2'\'' from
 	'\''$cwd/repo-wt/sub2/.git'\'' to
-	'\''$cwd/repo-bare.git/worktrees/repo-wt/modules/sub2'\''
+	'\''$cwd/repo-bare.git/worktrees/repo-wt-123/modules/sub2'\''
 	EOF
 	git -C repo-wt submodule absorbgitdirs 2>actual &&
 	test_cmp expect actual

-- 
2.47.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] worktree: rename worktree id during worktree move
  2024-11-29  2:44 [PATCH 0/2] Ensure unique worktree ids across repositories Caleb White
  2024-11-29  2:44 ` [PATCH 1/2] worktree: add worktree with unique suffix Caleb White
@ 2024-11-29  2:44 ` Caleb White
  2024-11-29  2:49 ` [PATCH 0/2] Ensure unique worktree ids across repositories Caleb White
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Caleb White @ 2024-11-29  2:44 UTC (permalink / raw)
  To: git; +Cc: Caleb White

During a `worktree move` the worktree directory is moved/renamed but the
repository under `worktrees/<id>` is not updated. For example, given the
following structure:

    foo/
    ├── .git/worktrees/develop-5445874156/
    └── develop/

moving `develop` to `master` results in

    foo/
    ├── .git/worktrees/develop-5445874156/
    └── master/

This works because the linking files still point to the correct
repository, but this is a little weird. This teaches Git to also
move/rename the repository / worktree id during a `move` so that the
structure now looks like:

    foo/
    ├── .git/worktrees/master-1565465986/
    └── master/

Note that a new unique suffix is assigned to reduce the complexity of
trying to parse and reuse the existing suffix.

Signed-off-by: Caleb White <cdwhite3@pm.me>
---
 builtin/worktree.c       | 24 ++++++++++++++++++++++++
 t/t2403-worktree-move.sh | 18 +++++++++---------
 2 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 3ad355ca762729401fc0c8625f4fd05b154a84ec..36235546b492803707707ff208b13fe777bff1b4 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -1202,9 +1202,14 @@ static int move_worktree(int ac, const char **av, const char *prefix)
 	};
 	struct worktree **worktrees, *wt;
 	struct strbuf dst = STRBUF_INIT;
+	struct strbuf repo = STRBUF_INIT;
+	struct strbuf repo_dst = STRBUF_INIT;
 	struct strbuf errmsg = STRBUF_INIT;
 	const char *reason = NULL;
+	const char *new_id;
+	const char *suffix;
 	char *path;
+	int len;
 
 	ac = parse_options(ac, av, prefix, options, git_worktree_move_usage,
 			   0);
@@ -1250,9 +1255,28 @@ static int move_worktree(int ac, const char **av, const char *prefix)
 	if (rename(wt->path, dst.buf) == -1)
 		die_errno(_("failed to move '%s' to '%s'"), wt->path, dst.buf);
 
+	strbuf_realpath(&repo, git_common_path("worktrees/%s", wt->id), 1);
+	new_id = worktree_basename(dst.buf, &len);
+	strbuf_add(&repo_dst, new_id, dst.buf + len - new_id);
+	strbuf_realpath(&repo_dst, git_common_path("worktrees/%s", repo_dst.buf), 1);
+	suffix = getenv("GIT_TEST_WORKTREE_SUFFIX");
+	if (suffix)
+		strbuf_addf(&repo_dst, "-%s", suffix);
+	else
+		strbuf_addf(&repo_dst, "-%u", git_rand());
+	new_id = strrchr(repo_dst.buf, '/') + 1;
+	if (rename(repo.buf, repo_dst.buf) == -1)
+		die_errno(_("failed to move '%s' to '%s'"), repo.buf, repo_dst.buf);
+	else {
+		free(wt->id);
+		wt->id = xstrdup(new_id);
+	}
+
 	update_worktree_location(wt, dst.buf, use_relative_paths);
 
 	strbuf_release(&dst);
+	strbuf_release(&repo);
+	strbuf_release(&repo_dst);
 	free_worktrees(worktrees);
 	return 0;
 }
diff --git a/t/t2403-worktree-move.sh b/t/t2403-worktree-move.sh
index ba3f05c16a4969fb84d98052ae375ef162f3e73a..703aa58d10643e99ffaf803aa38dabfd4af68a10 100755
--- a/t/t2403-worktree-move.sh
+++ b/t/t2403-worktree-move.sh
@@ -250,26 +250,26 @@ test_expect_success 'not remove a repo with initialized submodule' '
 test_expect_success 'move worktree with absolute path to relative path' '
 	test_config worktree.useRelativePaths false &&
 	GIT_TEST_WORKTREE_SUFFIX=123 git worktree add ./absolute &&
-	git worktree move --relative-paths absolute relative &&
-	echo "gitdir: ../.git/worktrees/absolute-123" >expect &&
+	GIT_TEST_WORKTREE_SUFFIX=456 git worktree move --relative-paths absolute relative &&
+	echo "gitdir: ../.git/worktrees/relative-456" >expect &&
 	test_cmp expect relative/.git &&
 	echo "../../../relative/.git" >expect &&
-	test_cmp expect .git/worktrees/absolute-123/gitdir &&
+	test_cmp expect .git/worktrees/relative-456/gitdir &&
 	test_config worktree.useRelativePaths true &&
-	git worktree move relative relative2 &&
-	echo "gitdir: ../.git/worktrees/absolute-123" >expect &&
+	GIT_TEST_WORKTREE_SUFFIX=789 git worktree move relative relative2 &&
+	echo "gitdir: ../.git/worktrees/relative2-789" >expect &&
 	test_cmp expect relative2/.git &&
 	echo "../../../relative2/.git" >expect &&
-	test_cmp expect .git/worktrees/absolute-123/gitdir
+	test_cmp expect .git/worktrees/relative2-789/gitdir
 '
 
 test_expect_success 'move worktree with relative path to absolute path' '
 	test_config worktree.useRelativePaths true &&
-	git worktree move --no-relative-paths relative2 absolute &&
-	echo "gitdir: $(pwd)/.git/worktrees/absolute-123" >expect &&
+	GIT_TEST_WORKTREE_SUFFIX=851 git worktree move --no-relative-paths relative2 absolute &&
+	echo "gitdir: $(pwd)/.git/worktrees/absolute-851" >expect &&
 	test_cmp expect absolute/.git &&
 	echo "$(pwd)/absolute/.git" >expect &&
-	test_cmp expect .git/worktrees/absolute-123/gitdir
+	test_cmp expect .git/worktrees/absolute-851/gitdir
 '
 
 test_done

-- 
2.47.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Ensure unique worktree ids across repositories
  2024-11-29  2:44 [PATCH 0/2] Ensure unique worktree ids across repositories Caleb White
  2024-11-29  2:44 ` [PATCH 1/2] worktree: add worktree with unique suffix Caleb White
  2024-11-29  2:44 ` [PATCH 2/2] worktree: rename worktree id during worktree move Caleb White
@ 2024-11-29  2:49 ` Caleb White
  2024-11-29  3:14 ` Junio C Hamano
  2024-11-29 11:05 ` shejialuo
  4 siblings, 0 replies; 11+ messages in thread
From: Caleb White @ 2024-11-29  2:49 UTC (permalink / raw)
  To: Caleb White, git

On Thu Nov 28, 2024 at 8:44 PM CST, Caleb White wrote:
> The `es/worktree-repair-copied` topic added support for repairing a
> worktree from a copy scenario. I noted[1,2] that the topic added the
> ability for a repository to "take over" a worktree from another
> repository if the worktree_id matched a worktree inside the current
> repository which can happen if two repositories use the same worktree name.
>
> This series teaches Git to create worktrees with a unique suffix so
> that the worktree_id is unique across all repositories even if they have
> the same name. For example creating a worktree `develop` would look like:
>
>     foo/
>     ├── .git/worktrees/develop-5445874156/
>     └── develop/
>     bar/
>     ├── .git/worktrees/develop-1549518426/
>     └── develop/
>
> The actual worktree directory name is still `develop`, but the
> worktree_id is unique and prevents the "take over" scenario. The suffix
> is given by the `git_rand()` function, but I'm open to suggestions if
> there's a better random or hashing function to use.
>
> [1]: https://lore.kernel.org/git/20241008153035.71178-1-cdwhite3@pm.me/
> [2]: https://lore.kernel.org/git/r4zmcET41Skr_FMop47AKd7cms9E8bKPSvHuAUpnYavzKEY6JybJta0_7GfuYB0q-gD-XNcvh5VDTfiT3qthGKjqhS1sbT4M2lUABynOz2Q=@pm.me/

I forgot to mention, but the base for this series is obtained by merging
the `cw/worktree-extension` topic (2024-11-26, 20241125-wt_relative_options-v5-0-356d122ff3db@pm.me)
onto 090d24e9af.

Best,

Caleb


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Ensure unique worktree ids across repositories
  2024-11-29  2:44 [PATCH 0/2] Ensure unique worktree ids across repositories Caleb White
                   ` (2 preceding siblings ...)
  2024-11-29  2:49 ` [PATCH 0/2] Ensure unique worktree ids across repositories Caleb White
@ 2024-11-29  3:14 ` Junio C Hamano
  2024-11-29  3:31   ` Caleb White
  2024-11-29 11:05 ` shejialuo
  4 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2024-11-29  3:14 UTC (permalink / raw)
  To: Caleb White; +Cc: git

Caleb White <cdwhite3@pm.me> writes:

> The `es/worktree-repair-copied` topic added support for repairing a
> worktree from a copy scenario. I noted[1,2] that the topic added the
> ability for a repository to "take over" a worktree from another
> repository if the worktree_id matched a worktree inside the current
> repository which can happen if two repositories use the same worktree name.

Problem worth solving.  Another would be to fail if the worktree ID
proposed to be used is already in use, but the ID is supposed to be
almost invisible (unless the user is doing some adiministrative work
on the repository), generating a unique ID is a good approach.

> This series teaches Git to create worktrees with a unique suffix so
> that the worktree_id is unique across all repositories even if they have
> the same name. For example creating a worktree `develop` would look like:
>
>     foo/
>     ├── .git/worktrees/develop-5445874156/
>     └── develop/
>     bar/
>     ├── .git/worktrees/develop-1549518426/
>     └── develop/
>
> The actual worktree directory name is still `develop`, but the
> worktree_id is unique and prevents the "take over" scenario. The suffix
> is given by the `git_rand()` function, but I'm open to suggestions if
> there's a better random or hashing function to use.

I do not think it matters much what hash/rand algorithm is chosen.
What is important is what you do when the suffix suggested by that
chosen algorithm collides with an existing worktree ID.  IOW, there
is no way a "random" can guarantee uniqueness.  Attempt to create and
if you find a collision, retry from the generation of another suffix,
or something like that, is necessary.

And as long as that "make sure it is unique" part is done right, it
does not even have to be random.  Just generating a sequence number
and using the first one that is available would work as well.

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Ensure unique worktree ids across repositories
  2024-11-29  3:14 ` Junio C Hamano
@ 2024-11-29  3:31   ` Caleb White
  2024-12-01  4:38     ` Junio C Hamano
  0 siblings, 1 reply; 11+ messages in thread
From: Caleb White @ 2024-11-29  3:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thu Nov 28, 2024 at 9:14 PM CST, Junio C Hamano wrote:
> Caleb White <cdwhite3@pm.me> writes:
>
>> The `es/worktree-repair-copied` topic added support for repairing a
>> worktree from a copy scenario. I noted[1,2] that the topic added the
>> ability for a repository to "take over" a worktree from another
>> repository if the worktree_id matched a worktree inside the current
>> repository which can happen if two repositories use the same worktree name.
>
> Problem worth solving.  Another would be to fail if the worktree ID
> proposed to be used is already in use, but the ID is supposed to be
> almost invisible (unless the user is doing some adiministrative work
> on the repository), generating a unique ID is a good approach.

There's already a `while` loop that tries incrementing the proposed id
(e.g., `develop1` if `develop` is taken). However, this is on
a per-repository basis, so there's no way to know what's already in use
in another repository. The problem arises when trying to run `worktree
repair` on a worktree that has a matching id (like `develop`) from
another repository. The goal here is that the same worktree name can be
created in different repositories and the id will be (effectively)
unique.

>> This series teaches Git to create worktrees with a unique suffix so
>> that the worktree_id is unique across all repositories even if they have
>> the same name. For example creating a worktree `develop` would look like:
>>
>>     foo/
>>     ├── .git/worktrees/develop-5445874156/
>>     └── develop/
>>     bar/
>>     ├── .git/worktrees/develop-1549518426/
>>     └── develop/
>>
>> The actual worktree directory name is still `develop`, but the
>> worktree_id is unique and prevents the "take over" scenario. The suffix
>> is given by the `git_rand()` function, but I'm open to suggestions if
>> there's a better random or hashing function to use.
>
> I do not think it matters much what hash/rand algorithm is chosen.
> What is important is what you do when the suffix suggested by that
> chosen algorithm collides with an existing worktree ID.  IOW, there
> is no way a "random" can guarantee uniqueness.  Attempt to create and
> if you find a collision, retry from the generation of another suffix,
> or something like that, is necessary.
>
> And as long as that "make sure it is unique" part is done right, it
> does not even have to be random.  Just generating a sequence number
> and using the first one that is available would work as well.

The `while` loop mentioned earlier still exists, so in the (unlikely)
event that the suffix collides with an existing worktree_id, it will
increment the suffix and try again.

Best,

Caleb


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Ensure unique worktree ids across repositories
  2024-11-29  2:44 [PATCH 0/2] Ensure unique worktree ids across repositories Caleb White
                   ` (3 preceding siblings ...)
  2024-11-29  3:14 ` Junio C Hamano
@ 2024-11-29 11:05 ` shejialuo
  2024-11-29 15:58   ` Caleb White
  4 siblings, 1 reply; 11+ messages in thread
From: shejialuo @ 2024-11-29 11:05 UTC (permalink / raw)
  To: Caleb White; +Cc: git

On Fri, Nov 29, 2024 at 02:44:24AM +0000, Caleb White wrote:
> The `es/worktree-repair-copied` topic added support for repairing a
> worktree from a copy scenario. I noted[1,2] that the topic added the
> ability for a repository to "take over" a worktree from another
> repository if the worktree_id matched a worktree inside the current
> repository which can happen if two repositories use the same worktree name.
> 

I somehow understand why we need to append a hash or a random number
into the current "id" field of the "struct worktree *". But I don't see
a _strong_ reason.

I think we need to figure out the following things:

    1. In what situation, there is a possibility that the user will
    repair the worktree from another repository.
    2. Why we need to hash to make sure the worktree is unique? From the
    expression, my intuitive way is that we need to distinguish whether
    the repository is the same.

> This series teaches Git to create worktrees with a unique suffix so
> that the worktree_id is unique across all repositories even if they have
> the same name. For example creating a worktree `develop` would look like:
> 
>     foo/
>     ├── .git/worktrees/develop-5445874156/
>     └── develop/
>     bar/
>     ├── .git/worktrees/develop-1549518426/
>     └── develop/
> 
> The actual worktree directory name is still `develop`, but the
> worktree_id is unique and prevents the "take over" scenario. The suffix
> is given by the `git_rand()` function, but I'm open to suggestions if
> there's a better random or hashing function to use.
> 

The actual worktree directory name is unchanged. But we have changed the
"worktree->id" and the git filesystem. Now, we will encounter much
trouble. The main reason is that we make the worktree name and worktree
id inconsistent. There are many tools which assume that worktree id is
the worktree name. In other words, there is no difference between the
worktree id and worktree name at current.

Let me give you an example.

The user could use "git update-ref" to update a ref from another ref.
So, a situation is that the user want to update(create) the
main-worktree ref from linked-worktree ref.

    ```sh
    git init repo && cd repo
    git commit --allow-empty -m initial
    git branch branch-1
    git worktree add ./worktree-1 branch-1
    (cd worktree-1 && git update-ref refs/worktree/branch-2 HEAD)
    ```
By the above operations, we will create a worktree-specified ref under
the ".git/worktrees/<worktree_id>/refs/worktree".

What if we want to this in the main worktree:

    ```sh
    git update-ref refs/heads/branch-3 \
        worktrees/worktree-1/refs/worktree/branch-2
    ```

So, with this patch, we make worktree-id not the same as worktree name.
If we do this. "git update-ref" cannot find the
".git/worktrees/worktree-1/refs/worktree/branch-2". This is because the
filesystem is changed to ".git/worktrees/worktree-1-<hash>/...".

If we use hash / random number to distinguish. We also need to change
the ref-related code to ignore the "-<hash>". It's impossible to let the
user type the extra hash / random number. However, this requires a lot
of effort.

So, I think we need a _strong_ reason to indicate that we must append
some chars into worktree id to do this.

Thanks,
Jialuo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Ensure unique worktree ids across repositories
  2024-11-29 11:05 ` shejialuo
@ 2024-11-29 15:58   ` Caleb White
  2024-11-29 17:32     ` shejialuo
  0 siblings, 1 reply; 11+ messages in thread
From: Caleb White @ 2024-11-29 15:58 UTC (permalink / raw)
  To: shejialuo; +Cc: git

On Fri Nov 29, 2024 at 5:05 AM CST, shejialuo wrote:
> On Fri, Nov 29, 2024 at 02:44:24AM +0000, Caleb White wrote:
>> The `es/worktree-repair-copied` topic added support for repairing a
>> worktree from a copy scenario. I noted[1,2] that the topic added the
>> ability for a repository to "take over" a worktree from another
>> repository if the worktree_id matched a worktree inside the current
>> repository which can happen if two repositories use the same worktree name.
>
> I somehow understand why we need to append a hash or a random number
> into the current "id" field of the "struct worktree *". But I don't see
> a _strong_ reason.
>
> I think we need to figure out the following things:
>
>     1. In what situation, there is a possibility that the user will
>     repair the worktree from another repository.

This can happen if a user accidentally mistypes a directory name when
executing `git worktree repair`. Or if a user copies a worktree from one
repository and the executes `git worktree repair` in a different repository.

The point is to prevent this from happening before it becomes a problem.

>     2. Why we need to hash to make sure the worktree is unique? From the
>     expression, my intuitive way is that we need to distinguish whether
>     the repository is the same.

During `worktree repair`, an "inferred backlink" is established by
parsing the worktree id from the `.git` file and checking if that
matches an existing worktree id in the current repository. If so, then
the link is established even if that worktree belonged to another
repository. The easiest way I thought to prevent this from happening is
to ensure that no "inferred backlink" can be established by using an
effectively unique worktree_id. So two repositories can have the same
worktree name (e.g., `develop`) but the actual ids would be different.
Additionally, if the ids **do match**, then this would be indicative of
a copy situation like the original topic addressed.

How do you propose to distinguish whether the repository is the same?

>> This series teaches Git to create worktrees with a unique suffix so
>> that the worktree_id is unique across all repositories even if they have
>> the same name. For example creating a worktree `develop` would look like:
>>
>>     foo/
>>     ├── .git/worktrees/develop-5445874156/
>>     └── develop/
>>     bar/
>>     ├── .git/worktrees/develop-1549518426/
>>     └── develop/
>>
>> The actual worktree directory name is still `develop`, but the
>> worktree_id is unique and prevents the "take over" scenario. The suffix
>> is given by the `git_rand()` function, but I'm open to suggestions if
>> there's a better random or hashing function to use.
>
> The actual worktree directory name is unchanged. But we have changed the
> "worktree->id" and the git filesystem. Now, we will encounter much
> trouble. The main reason is that we make the worktree name and worktree
> id inconsistent. There are many tools which assume that worktree id is
> the worktree name.

Do you have any sources for these tools? Because I'm not aware of any.
Any tool that needs the actual worktree id should be extracting the id
from the `.git` file and not using the worktree directory name.

> In other words, there is no difference between the worktree id and
> worktree name at current.

This is NOT true, there are several scenarios where they can currently differ:

1. git currently will append a number to the worktree name if it already
   exists in the repository. This means that you can create a `develop`
   worktree and wind up with an id of `develop2`.
2. git does not currently rename the id during a `worktree move`. This
   means that I can create a worktree with a name of `develop` and then
   execute `git worktree move develop master` and the id will still be
   `develop` while the directory is now `master`.
3. a user can manually move/rename the directory and then repair the
   worktree and wind up in the same situation as 2).

The suffix is not a new concept, I've just changed it from occasionally
adding a suffix to always adding a (unique) suffix. The worktree id has
never been guaranteed to be the same as the worktree name, and as
mentioned above, any tools/scripts/intelligence that need the id should
always be extracting it from the `.git` file.

One thing we can do is to add the worktree id to the `git worktree list`
output so that users can see the id and the name together. This would
make it easier for users/tools obtain the id if they need it without
having to parse the `.git` file.

> Let me give you an example.
>
> So, with this patch, we make worktree-id not the same as worktree name.
> If we do this. "git update-ref" cannot find the
> ".git/worktrees/worktree-1/refs/worktree/branch-2". This is because the
> filesystem is changed to ".git/worktrees/worktree-1-<hash>/...".

Yes this is expected because the worktree id is not the same as the
name.

> If we use hash / random number to distinguish. We also need to change
> the ref-related code to ignore the "-<hash>". It's impossible to let the
> user type the extra hash / random number. However, this requires a lot
> of effort.

This would be possible as long as the given worktree slug is unambiguous.
However, I think this is more trouble than it's worth.

> So, I think we need a _strong_ reason to indicate that we must append
> some chars into worktree id to do this.

As stated above, git already appends a number to the worktree name if it
collides with an existing directory. Always appending a unique suffix
should actually make things simpler / more consistent in the long run
because the worktree id will always be different from the name instead
of occasionally being different.

Best,

Caleb


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Ensure unique worktree ids across repositories
  2024-11-29 15:58   ` Caleb White
@ 2024-11-29 17:32     ` shejialuo
  2024-11-29 20:39       ` Caleb White
  0 siblings, 1 reply; 11+ messages in thread
From: shejialuo @ 2024-11-29 17:32 UTC (permalink / raw)
  To: Caleb White; +Cc: git, Junio C Hamano, Eric Sunshine

On Fri, Nov 29, 2024 at 03:58:16PM +0000, Caleb White wrote:
> On Fri Nov 29, 2024 at 5:05 AM CST, shejialuo wrote:
> > On Fri, Nov 29, 2024 at 02:44:24AM +0000, Caleb White wrote:
> >> The `es/worktree-repair-copied` topic added support for repairing a
> >> worktree from a copy scenario. I noted[1,2] that the topic added the
> >> ability for a repository to "take over" a worktree from another
> >> repository if the worktree_id matched a worktree inside the current
> >> repository which can happen if two repositories use the same worktree name.
> >
> > I somehow understand why we need to append a hash or a random number
> > into the current "id" field of the "struct worktree *". But I don't see
> > a _strong_ reason.
> >
> > I think we need to figure out the following things:
> >
> >     1. In what situation, there is a possibility that the user will
> >     repair the worktree from another repository.
> 
> This can happen if a user accidentally mistypes a directory name when
> executing `git worktree repair`. Or if a user copies a worktree from one
> repository and the executes `git worktree repair` in a different repository.
> 
> The point is to prevent this from happening before it becomes a problem.
> 

If we could prevent this, this is great. But the current implementation
will cause much burden for the refs-related code. In other words, my
concern is that if we use this way, we may put a lot of efforts to
change the ref-related codes to adapt into this new design. So, we
should think carefully whether we should put so many efforts to solve
this corner case by this way.

I somehow know the background, because we have allowed both the absolute
and relative paths for worktree, we need to handle this problem.

I agree with your motivation, but we need to consider the burden
introduced. This is my point.

> >     2. Why we need to hash to make sure the worktree is unique? From the
> >     expression, my intuitive way is that we need to distinguish whether
> >     the repository is the same.
> 
> During `worktree repair`, an "inferred backlink" is established by
> parsing the worktree id from the `.git` file and checking if that
> matches an existing worktree id in the current repository. If so, then
> the link is established even if that worktree belonged to another
> repository. The easiest way I thought to prevent this from happening is
> to ensure that no "inferred backlink" can be established by using an
> effectively unique worktree_id. So two repositories can have the same
> worktree name (e.g., `develop`) but the actual ids would be different.
> Additionally, if the ids **do match**, then this would be indicative of
> a copy situation like the original topic addressed.
> 

OK.

> How do you propose to distinguish whether the repository is the same?
> 

I am sorry that I cannot tell you the answer. I haven't dived into the
worktree. I just gave my thoughts above.

> >> This series teaches Git to create worktrees with a unique suffix so
> >> that the worktree_id is unique across all repositories even if they have
> >> the same name. For example creating a worktree `develop` would look like:
> >>
> >>     foo/
> >>     ├── .git/worktrees/develop-5445874156/
> >>     └── develop/
> >>     bar/
> >>     ├── .git/worktrees/develop-1549518426/
> >>     └── develop/
> >>
> >> The actual worktree directory name is still `develop`, but the
> >> worktree_id is unique and prevents the "take over" scenario. The suffix
> >> is given by the `git_rand()` function, but I'm open to suggestions if
> >> there's a better random or hashing function to use.
> >
> > The actual worktree directory name is unchanged. But we have changed the
> > "worktree->id" and the git filesystem. Now, we will encounter much
> > trouble. The main reason is that we make the worktree name and worktree
> > id inconsistent. There are many tools which assume that worktree id is
> > the worktree name.
> 
> Do you have any sources for these tools? Because I'm not aware of any.
> Any tool that needs the actual worktree id should be extracting the id
> from the `.git` file and not using the worktree directory name.
> 

I am sorry that my words may confuse you here. These tools are just the
git builtins. Such as "git update-ref" and "git symbolic-ref".

> > In other words, there is no difference between the worktree id and
> > worktree name at current.
> 
> This is NOT true, there are several scenarios where they can currently differ:
> 
> 1. git currently will append a number to the worktree name if it already
>    exists in the repository. This means that you can create a `develop`
>    worktree and wind up with an id of `develop2`.
> 2. git does not currently rename the id during a `worktree move`. This
>    means that I can create a worktree with a name of `develop` and then
>    execute `git worktree move develop master` and the id will still be
>    `develop` while the directory is now `master`.
> 3. a user can manually move/rename the directory and then repair the
>    worktree and wind up in the same situation as 2).
> 

Thanks for this information. I am not familiar with the worktree. I just
have learned the worktree when doing something related to the refs. So,
it is true that worktree id and worktree name are not the same.

But that's wired. For any situation above, it won't cause any trouble
when the user is in the worktree. Because the user could just use
"refs/worktree/foo" to indicate the worktree specified ref without
knowing the worktree id.

So if a user moves the path from "worktree_1" to "worktree_2" and wants
to do the following operation in the main worktree:

    ```sh
    git update-ref refs/heads/master \
        worktrees/worktree_2/refs/worktree/foo
    ```
It will encounter error, because the worktree id is still the
"worktree_1".

But when the user create the worktree using the following command:

    ```sh
    git worktree add ./worktree_1 branch-1
    ```

The name `worktree_1`(path) will be the worktree id. So, when moving the
path "worktree_1" to "worktree_2". The user won't know the detail about
the worktree id. The user (like me) will just think that "worktree_2"
will be the new worktree id.

That does not make sense. It's impossible for the user know the mapping
between the worktree name and worktree id.

Hi Eric and Junio, I am wondering what the purpose of worktree is. When
I implemented the consistent check for ref contents, Junio has told me
that the main-worktree symref could point to the linked-worktree ref.
The linked-worktree symref could also point to another linked-worktree
ref.

But from above, it gives a feeling that the purpose of the worktree is
to make the environment totally independent. And we shouldn't allow any
ref interactions between the main-worktree and linked-worktree. But we
DO allow at now by using "git symbolic-ref" and "git update-ref".

> The suffix is not a new concept, I've just changed it from occasionally
> adding a suffix to always adding a (unique) suffix. The worktree id has
> never been guaranteed to be the same as the worktree name, and as
> mentioned above, any tools/scripts/intelligence that need the id should
> always be extracting it from the `.git` file.
> 

Yes, I agree. Thanks.

> One thing we can do is to add the worktree id to the `git worktree list`
> output so that users can see the id and the name together. This would
> make it easier for users/tools obtain the id if they need it without
> having to parse the `.git` file.
> 

This is a good idea, if we need to use this way. And we may also need to
add documentation.

> > Let me give you an example.
> >
> > So, with this patch, we make worktree-id not the same as worktree name.
> > If we do this. "git update-ref" cannot find the
> > ".git/worktrees/worktree-1/refs/worktree/branch-2". This is because the
> > filesystem is changed to ".git/worktrees/worktree-1-<hash>/...".
> 
> Yes this is expected because the worktree id is not the same as the
> name.
> 
> > If we use hash / random number to distinguish. We also need to change
> > the ref-related code to ignore the "-<hash>". It's impossible to let the
> > user type the extra hash / random number. However, this requires a lot
> > of effort.
> 
> This would be possible as long as the given worktree slug is unambiguous.
> However, I think this is more trouble than it's worth.
> 

As you can see from above, the main problem is that we allow some ref
interactions between the main-worktree and linked-worktrees.

The reason why I use this example is that I am afraid that the user will
create a new symbolic ref in the main worktree which points to the
linked-worktree ref.

When the user create a worktree, it is natural to think that there is no
difference between the worktree id and the worktree name. And will use
"worktrees/<worktree_id>/refs/worktree/foo" to access the ref in the
worktree.

And It's OK to add hash / random number to the worktree id if worktree
is totally independent. However, at now, we allow some interactions
between the main-worktree and linked worktrees (even the linked
worktree and another linked worktree).

When doing above, the user must know the worktree id. But at now
worktree name is not the same as worktree id.

So, I don't know...

> > So, I think we need a _strong_ reason to indicate that we must append
> > some chars into worktree id to do this.
> 
> As stated above, git already appends a number to the worktree name if it
> collides with an existing directory. Always appending a unique suffix
> should actually make things simpler / more consistent in the long run
> because the worktree id will always be different from the name instead
> of occasionally being different.
> 

In general, I agree with your way now after knowing the truth that
worktree name is not the same as the worktree id. However, it seems that
the situation is a little complicated.

> Best,
> 
> Caleb

Thanks,
Jialuo

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Ensure unique worktree ids across repositories
  2024-11-29 17:32     ` shejialuo
@ 2024-11-29 20:39       ` Caleb White
  0 siblings, 0 replies; 11+ messages in thread
From: Caleb White @ 2024-11-29 20:39 UTC (permalink / raw)
  To: shejialuo; +Cc: git, Junio C Hamano, Eric Sunshine

On Fri Nov 29, 2024 at 11:32 AM CST, shejialuo wrote:
> On Fri, Nov 29, 2024 at 03:58:16PM +0000, Caleb White wrote:
>> On Fri Nov 29, 2024 at 5:05 AM CST, shejialuo wrote:
>> > I somehow understand why we need to append a hash or a random number
>> > into the current "id" field of the "struct worktree *". But I don't see
>> > a _strong_ reason.
>> >
>> > I think we need to figure out the following things:
>> >
>> >     1. In what situation, there is a possibility that the user will
>> >     repair the worktree from another repository.
>>
>> This can happen if a user accidentally mistypes a directory name when
>> executing `git worktree repair`. Or if a user copies a worktree from one
>> repository and the executes `git worktree repair` in a different repository.
>>
>> The point is to prevent this from happening before it becomes a problem.
>
> If we could prevent this, this is great. But the current implementation
> will cause much burden for the refs-related code. In other words, my
> concern is that if we use this way, we may put a lot of efforts to
> change the ref-related codes to adapt into this new design. So, we
> should think carefully whether we should put so many efforts to solve
> this corner case by this way.

I'm not sure that any of the ref related code would need to change,
all the tests are passing and the ref code uses the worktree id
for anything worktree related.

> I somehow know the background, because we have allowed both the absolute
> and relative paths for worktree, we need to handle this problem.

This edge case exists when using absolute paths, so this is not
a consequence of now allowing relative paths.

>> >     2. Why we need to hash to make sure the worktree is unique? From the
>> >     expression, my intuitive way is that we need to distinguish whether
>> >     the repository is the same.
>> 
>> How do you propose to distinguish whether the repository is the same?
>
> I am sorry that I cannot tell you the answer. I haven't dived into the
> worktree. I just gave my thoughts above.

That's fine, I'm just not really sure about how such a thing would be
done. To me, the unique id is the easiest and most intuitive. The
repository generally has no knowledge of other repositories on the
system (as far as I am aware).

>> Do you have any sources for these tools? Because I'm not aware of any.
>> Any tool that needs the actual worktree id should be extracting the id
>> from the `.git` file and not using the worktree directory name.
>
> I am sorry that my words may confuse you here. These tools are just the
> git builtins. Such as "git update-ref" and "git symbolic-ref".

Ah, I understand now---I thought you were talking about external tools.
All internal git builtins use the worktree id, the actual directory name
of the worktree is inconsequential. So nothing should need to change.

>> > In other words, there is no difference between the worktree id and
>> > worktree name at current.
>>
>> This is NOT true, there are several scenarios where they can currently differ:
>>
>> 1. git currently will append a number to the worktree name if it already
>>    exists in the repository. This means that you can create a `develop`
>>    worktree and wind up with an id of `develop2`.
>> 2. git does not currently rename the id during a `worktree move`. This
>>    means that I can create a worktree with a name of `develop` and then
>>    execute `git worktree move develop master` and the id will still be
>>    `develop` while the directory is now `master`.
>> 3. a user can manually move/rename the directory and then repair the
>>    worktree and wind up in the same situation as 2).
>>
>
> Thanks for this information. I am not familiar with the worktree. I just
> have learned the worktree when doing something related to the refs. So,
> it is true that worktree id and worktree name are not the same.
>
> But that's wired. For any situation above, it won't cause any trouble
> when the user is in the worktree. Because the user could just use
> "refs/worktree/foo" to indicate the worktree specified ref without
> knowing the worktree id.
>
> So if a user moves the path from "worktree_1" to "worktree_2" and wants
> to do the following operation in the main worktree:
>
>     ```sh
>     git update-ref refs/heads/master \
>         worktrees/worktree_2/refs/worktree/foo
>     ```
> It will encounter error, because the worktree id is still the
> "worktree_1".
>
> But when the user create the worktree using the following command:
>
>     ```sh
>     git worktree add ./worktree_1 branch-1
>     ```
>
> The name `worktree_1`(path) will be the worktree id. So, when moving the
> path "worktree_1" to "worktree_2". The user won't know the detail about
> the worktree id. The user (like me) will just think that "worktree_2"
> will be the new worktree id.
>
> That does not make sense. It's impossible for the user know the mapping
> between the worktree name and worktree id.

It's not impossible, the user can always look in the `.git` file.
However, I have added the id to the `worktree list` output to more easily
associate the id with the worktree.

>> One thing we can do is to add the worktree id to the `git worktree list`
>> output so that users can see the id and the name together. This would
>> make it easier for users/tools obtain the id if they need it without
>> having to parse the `.git` file.
>>
> This is a good idea, if we need to use this way. And we may also need to
> add documentation.

I have implemented this and added documentation.

> As you can see from above, the main problem is that we allow some ref
> interactions between the main-worktree and linked-worktrees.
>
> The reason why I use this example is that I am afraid that the user will
> create a new symbolic ref in the main worktree which points to the
> linked-worktree ref.
>
> When the user create a worktree, it is natural to think that there is no
> difference between the worktree id and the worktree name. And will use
> "worktrees/<worktree_id>/refs/worktree/foo" to access the ref in the
> worktree.
>
> And It's OK to add hash / random number to the worktree id if worktree
> is totally independent. However, at now, we allow some interactions
> between the main-worktree and linked worktrees (even the linked
> worktree and another linked worktree).
>
> When doing above, the user must know the worktree id. But at now
> worktree name is not the same as worktree id.
>
> So, I don't know...

Interactions between the main and linked (and linked with other linked)
worktrees is not a limiting factor here.

>> As stated above, git already appends a number to the worktree name if it
>> collides with an existing directory. Always appending a unique suffix
>> should actually make things simpler / more consistent in the long run
>> because the worktree id will always be different from the name instead
>> of occasionally being different.
>
> In general, I agree with your way now after knowing the truth that
> worktree name is not the same as the worktree id. However, it seems that
> the situation is a little complicated.

I see where you're coming from, but I do not believe that the situation
is any more complicated than it already is. All internal git code uses
the worktree id which is already not guaranteed to be the same as the
worktree directory name, so nothing is changing on that front.

Best,

Caleb


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/2] Ensure unique worktree ids across repositories
  2024-11-29  3:31   ` Caleb White
@ 2024-12-01  4:38     ` Junio C Hamano
  0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2024-12-01  4:38 UTC (permalink / raw)
  To: Caleb White; +Cc: git

Caleb White <cdwhite3@pm.me> writes:

> On Thu Nov 28, 2024 at 9:14 PM CST, Junio C Hamano wrote:
>> Caleb White <cdwhite3@pm.me> writes:
>>
>>> The `es/worktree-repair-copied` topic added support for repairing a
>>> worktree from a copy scenario. I noted[1,2] that the topic added the
>>> ability for a repository to "take over" a worktree from another
>>> repository if the worktree_id matched a worktree inside the current
>>> repository which can happen if two repositories use the same worktree name.
>>
>> Problem worth solving.  Another would be to fail if the worktree ID
>> proposed to be used is already in use, but the ID is supposed to be
>> almost invisible (unless the user is doing some adiministrative work
>> on the repository), generating a unique ID is a good approach.
>
> There's already a `while` loop that tries incrementing the proposed id
> (e.g., `develop1` if `develop` is taken). However, this is on
> a per-repository basis, so there's no way to know what's already in use
> in another repository.

Usually repositories on a single host are not aware of each other,
so I am not sure if it is a sensible goal to begin with, to try to
ensure a worktree ID is unique "across repositories".

> The problem arises when trying to run `worktree
> repair` on a worktree that has a matching id (like `develop`) from
> another repository. The goal here is that the same worktree name can be
> created in different repositories and the id will be (effectively)
> unique.

OK, but wouldn't that change the problem we need to solve greatly?

The problem is very much simplified, in fact.  When we are adding to
.git/worktrees/ of a single repository, we need a unique worktree ID
given to the new worktree.  And the ID like `develop` taken from
another repository may or may not be already in use here.

In a repository, a directory ../foo/develop may already be
registered as its worktree, and the user may try to add yet another
directory ../bar/develop as a new worktree.  The first one has been
using 'develop' as its ID, and the "worktree add" command to create
the new one needs to tweak the basename 'develop' to make it unique
within this single repository.  Shouldn't `repair` that tries to
bring in an orphaned worktree that used to be given a worktree ID by
a potentially different repository (or it could be initially created
in this repository and then forgotten, and in the meantime there
could have been many iterations of `develop` worktrees created for
the repository and while it was missing, its ID plus serial number
may have already taken) follow the same pattern?  Whatever worktree
ID that the other repository gave it is invalid in the context of
this repository, anyway.

So I am not sure why we need to complicate the system by adding
random number, which does not help in ensuring uniqueness (it may
make it less likely to collide, but that is different from
guaranteeing uniqueness), while misleading readers that somehow
these numbers after the worktree IDs are serving some purpose.

Am I missing something?

Thanks.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-12-01  4:38 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-29  2:44 [PATCH 0/2] Ensure unique worktree ids across repositories Caleb White
2024-11-29  2:44 ` [PATCH 1/2] worktree: add worktree with unique suffix Caleb White
2024-11-29  2:44 ` [PATCH 2/2] worktree: rename worktree id during worktree move Caleb White
2024-11-29  2:49 ` [PATCH 0/2] Ensure unique worktree ids across repositories Caleb White
2024-11-29  3:14 ` Junio C Hamano
2024-11-29  3:31   ` Caleb White
2024-12-01  4:38     ` Junio C Hamano
2024-11-29 11:05 ` shejialuo
2024-11-29 15:58   ` Caleb White
2024-11-29 17:32     ` shejialuo
2024-11-29 20:39       ` Caleb White

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).