worktree repair: avoid breaking unrelated .git file and gitdir

PATCH Superseded Yoichi NAKAYAMA via GitGitGadget 3 participants 23 messages Sep 13 – Oct 17

Share & tools

Yoichi NAKAYAMA via GitGitGadget

author

2026-09-13 03:20 UTC #0 lore raw

cover worktree repair: avoid breaking unrelated .git file and gitdir

'git worktree repair' does not sufficiently validate the cross-references between a linked working tree and its administrative data before repairing them. This can cause the repair to modify the wrong .git file or gitdir in certain situations.

This series first refactors the code to read the .git file once and extract the worktree ID, then uses that information to validate the repair target before modifying the cross-references.

* [1/2] Refactor the code without changing functionality before making the fix
* [2/2] Validate the worktree ID and inferred gitdir path before repairing

Yoichi NAKAYAMA (2):
worktree repair: refactor and reduce .git file reads worktree repair: avoid breaking unrelated .git file and gitdir

 t/t2406-worktree-repair.sh |  33 +++++++++---
 worktree.c                 | 105 ++++++++++++++++++++-----------------
 2 files changed, 83 insertions(+), 55 deletions(-)

base-commit: 47ce80527c56f462cb97db4ca8125342204d3783
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2225%2Fyoichi%2Fworktree-repair-keep-unrelated-gitfile-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2225/yoichi/worktree-repair-keep-unrelated-gitfile-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2225

Signature · 1 line

gitgitgadget

Yoichi NAKAYAMA via GitGitGadget

author

2026-09-13 03:20 UTC #1 lore raw

PATCH 1/2 worktree repair: refactor and reduce .git file reads

From: Yoichi NAKAYAMA <[email protected]>

Remove the file reading and trimming logic from infer_backlink(), and instead read the .git file once in its caller, repair_worktree_at_path(), using read_gitfile_raw(). Since read_gitfile_gently() is replaced with read_gitfile_raw(), restore the logic for constructing the absolute path and replace the READ_GITFILE_ERR_NOT_A_REPO handling with a check using is_git_directory(). Simplify the logic for prioritizing 'inferred_backlink' over 'backlink'.

Extract get_worktree_id() to get the worktree ID from the contents of the .git file. We are going to modify and use this function in subsequent commits.

Signed-off-by Yoichi NAKAYAMA <[email protected]>

---
 worktree.c | 89 ++++++++++++++++++++++++++----------------------------
 1 file changed, 43 insertions(+), 46 deletions(-)

worktree.c +43 ~~-46~~

index 8cb8637b18..7af13898d0 100644
@@ -637,6 +637,14 @@ int other_head_refs(struct repository *repo,
 	return ret;
 }
 
+static const char *get_worktree_id(const char *dotgit_contents)
+{
+	const char *slash = find_last_dir_sep(dotgit_contents);
+	if (!slash)
+		return "";
+	return slash + 1;
+}
+
 /*
  * Repair worktree's /path/to/worktree/.git file if missing, corrupt, or not
  * pointing at <repo>/worktrees/<id>.

@@ -798,30 +806,20 @@ static int is_main_worktree_path(struct repository *repo, const char *path)
  * Returns -1 on failure and strbuf.len on success.
  */
 static ssize_t infer_backlink(struct repository *repo,
-			      const char *gitfile,
+			      const char *dotgit_contents,
 			      struct strbuf *inferred)
 {
-	struct strbuf actual = STRBUF_INIT;
 	const char *id;
 
-	if (strbuf_read_file(&actual, gitfile, 0) < 0)
-		goto error;
-	if (!starts_with(actual.buf, "gitdir:"))
-		goto error;
-	if (!(id = find_last_dir_sep(actual.buf)))
-		goto error;
-	strbuf_trim(&actual);
-	id++; /* advance past '/' to point at <id> */
+	id = get_worktree_id(dotgit_contents);
 	if (!*id)
 		goto error;
 	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
 	if (!is_directory(inferred->buf))
 		goto error;
 
-	strbuf_release(&actual);
 	return inferred->len;
 error:
-	strbuf_release(&actual);
 	strbuf_reset(inferred); /* clear invalid path */
 	return -1;
 }

@@ -840,7 +838,8 @@ void repair_worktree_at_path(struct repository *repo,
 	struct strbuf inferred_backlink = STRBUF_INIT;
 	struct strbuf gitdir = STRBUF_INIT;
 	struct strbuf olddotgit = STRBUF_INIT;
-	char *dotgit_contents = NULL;
+	struct strbuf contents = STRBUF_INIT;
+	const char *dotgit_contents = NULL;
 	const char *repair = NULL;
 	int err;

@@ -856,51 +855,49 @@ void repair_worktree_at_path(struct repository *repo,
 		goto done;
 	}
 
-	infer_backlink(repo, dotgit.buf, &inferred_backlink);
-	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
-	dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));
-	if (dotgit_contents) {
-		strbuf_addstr(&backlink, dotgit_contents);
-	} else if (err == READ_GITFILE_ERR_NOT_A_FILE ||
-			err == READ_GITFILE_ERR_IS_A_DIR) {
+	err = read_gitfile_raw(&contents, dotgit.buf);
+	if (err == READ_GITFILE_ERR_NOT_A_FILE ||
+	    err == READ_GITFILE_ERR_IS_A_DIR) {
 		fn(1, dotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
 		goto done;
-	} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {
-		if (inferred_backlink.len) {
-			/*
-			 * Worktree's .git file does not point at a repository
-			 * but we found a .git/worktrees/<id> in this
-			 * repository with the same <id> as recorded in the
-			 * worktree's .git file so make the worktree point at
-			 * the discovered .git/worktrees/<id>.
-			 */
-			strbuf_swap(&backlink, &inferred_backlink);
-		} else {
-			fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
-			goto done;
-		}
-	} else {
+	} else if (err) {
 		fn(1, dotgit.buf, _("unable to locate repository; .git file broken"), cb_data);
 		goto done;
 	}
 
+	dotgit_contents = contents.buf;
+	infer_backlink(repo, dotgit_contents, &inferred_backlink);
+	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
+
+	if (is_absolute_path(dotgit_contents)) {
+		strbuf_addstr(&backlink, dotgit_contents);
+	} else {
+		strbuf_addbuf(&backlink, &dotgit);
+		strbuf_strip_suffix(&backlink, ".git");
+		strbuf_addstr(&backlink, dotgit_contents);
+		strbuf_realpath_forgiving(&backlink, backlink.buf, 0);
+	}
+
+	if (!is_git_directory(backlink.buf) && !inferred_backlink.len) {
+		fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
+		goto done;
+	}
+
 	/*
 	 * If we got this far, either the worktree's .git file pointed at a
-	 * valid repository (i.e. read_gitfile_gently() returned success) or
+	 * valid repository (i.e. is_git_directory() returned true) or
 	 * the .git file did not point at a repository but we were able to
 	 * infer a suitable new value for the .git file by locating a
 	 * .git/worktrees/<id> in *this* repository corresponding to the <id>
 	 * recorded in the worktree's .git file.
 	 *
-	 * However, if, at this point, inferred_backlink is non-NULL (i.e. we
-	 * found a suitable .git/worktrees/<id> in *this* repository) *and* the
-	 * worktree's .git file points at a valid repository *and* those two
-	 * paths differ, then that indicates that the user probably *copied*
-	 * the main and linked worktrees to a new location as a unit rather
-	 * than *moving* them. Thus, the copied worktree's .git file actually
-	 * points at the .git/worktrees/<id> in the *original* repository, not
-	 * in the "copy" repository. In this case, point the "copy" worktree's
-	 * .git file at the "copy" repository.
+	 * Even if the worktree's .git file pointed at a valid repository,
+	 * it doesn't always mean that the backlink is correct. For example,
+	 * the user might have *copied* the main and linked worktrees to a
+	 * new location as a unit rather than *moving* them (the copied
+	 * worktree's .git file actually points at the .git/worktrees/<id>
+	 * in the *original* repository, not in the "copy" repository).
+	 * Therefore, we prioritize inferred_backlink over backlink.
 	 */
 	if (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf))
 		strbuf_swap(&backlink, &inferred_backlink);

@@ -926,12 +923,12 @@ void repair_worktree_at_path(struct repository *repo,
 					     gitdir.buf, use_relative_paths);
 	}
 done:
-	free(dotgit_contents);
 	strbuf_release(&olddotgit);
 	strbuf_release(&backlink);
 	strbuf_release(&inferred_backlink);
 	strbuf_release(&gitdir);
 	strbuf_release(&dotgit);
+	strbuf_release(&contents);
 }
 
 int should_prune_worktree(struct repository *repo,

Signature · 1 line

gitgitgadget

Yoichi NAKAYAMA via GitGitGadget

author

2026-09-13 03:20 UTC #5 lore raw

PATCH 2/2 worktree repair: avoid breaking unrelated .git file and gitdir

In reply to Yoichi NAKAYAMA via GitGitGadget

From: Yoichi NAKAYAMA <[email protected]>

Currently, repair_gitfile() does not verify whether the worktree ID recorded in the .git file matches the worktree being repaired, which can result in an unrelated .git file being corrupted. For instance, if two worktree directories are swapped without using 'git worktree move', running 'git worktree repair' in the main worktree accidentally swaps the links between their .git files and gitdirs.

repair_worktree_at_path() proceeds even if it fails to infer the gitdir path. This can result in the corruption of an unrelated gitdir. For instance, if we copied a linked worktree to a new location X, running 'git worktree repair X' in a working tree which does not belong to the original repository can accidentally overwrite the gitdir in the original repository (the scope of impact should be limited to the repository where the command was executed).

Resolve these issues by validating the worktree ID and stopping the repair when the ID does not match or the gitdir path cannot be inferred.

Signed-off-by Yoichi NAKAYAMA <[email protected]>

---
 t/t2406-worktree-repair.sh | 33 +++++++++++++++++++++++++++------
 worktree.c                 | 18 ++++++++++++++----
 2 files changed, 41 insertions(+), 10 deletions(-)

t/t2406-worktree-repair.sh +27 -6

index d4e53d492b..2ffa123f42 100755
@@ -56,15 +56,12 @@ test_expect_success 'repair missing .git file' '
 '
 
 test_expect_success 'repair bogus .git file' '
-	test_corrupt_gitfile "echo \"gitdir: /nowhere\" >corrupt/.git" \
+	test_corrupt_gitfile "echo \"contents not started with gitdir:\" >corrupt/.git" \
 		".git file broken"
 '
 
-test_expect_success 'repair incorrect .git file' '
-	test_when_finished "rm -rf other && git worktree prune" &&
-	test_create_repo other &&
-	other=$(git -C other rev-parse --absolute-git-dir) &&
-	test_corrupt_gitfile "echo \"gitdir: $other\" >corrupt/.git" \
+test_expect_success 'repair unlinked .git file' '
+	test_corrupt_gitfile "echo \"gitdir: /nowhere/worktrees/corrupt\" >corrupt/.git" \
 		".git file incorrect"
 '

@@ -89,6 +86,18 @@ test_expect_success 'repair .git file from bare.git' '
 	test_cmp expect actual
 '
 
+test_expect_success 'skip unrelated .git file' '
+	test_when_finished "rm -rf corrupt other && git worktree prune" &&
+	git worktree add --detach corrupt &&
+	rm -rf corrupt &&
+	git worktree add --detach other &&
+	mv other corrupt &&
+	cat corrupt/.git >expect &&
+	test_must_fail git worktree repair 2>err &&
+	test_cmp expect corrupt/.git &&
+	test_grep "unrelated .git file" err
+'
+
 test_expect_success 'invalid worktree path' '
 	test_must_fail git worktree repair /notvalid >out 2>err &&
 	test_must_be_empty out &&

@@ -113,6 +122,18 @@ test_expect_success 'repo not found; .git not referencing repo' '
 	test_grep ".git file does not reference a repository" err
 '
 
+test_expect_success 'repo not found; .git not for worktree' '
+	test_when_finished "rm -rf side other-repo && git worktree prune" &&
+	test_create_repo other-repo &&
+	git worktree add --detach side &&
+	cat .git/worktrees/side/gitdir >expect &&
+	cp -R side other-repo/side &&
+	test_must_fail git -C other-repo worktree repair side >out 2>err &&
+	test_cmp expect .git/worktrees/side/gitdir &&
+	test_must_be_empty out &&
+	test_grep ".git file is not for a linked worktree" err
+'
+
 test_expect_success 'repo not found; .git file broken' '
 	test_when_finished "rm -rf orig moved && git worktree prune" &&
 	git worktree add --detach orig &&

worktree.c +14 -4

index 7af13898d0..88da599ab6 100644
@@ -640,7 +640,11 @@ int other_head_refs(struct repository *repo,
 static const char *get_worktree_id(const char *dotgit_contents)
 {
 	const char *slash = find_last_dir_sep(dotgit_contents);
-	if (!slash)
+	const char *prefix = "/worktrees";
+	int prefixlen = strlen(prefix);
+	if (!slash ||
+	    slash - dotgit_contents < prefixlen ||
+	    strncmp(slash - prefixlen, prefix, prefixlen))
 		return "";
 	return slash + 1;
 }

@@ -692,8 +696,10 @@ static void repair_gitfile(struct worktree *wt,
 	if (err == READ_GITFILE_ERR_NOT_A_FILE ||
 		err == READ_GITFILE_ERR_IS_A_DIR)
 		fn(1, wt->path, _(".git is not a file"), cb_data);
-	else if (err || !is_git_directory(backlink.buf))
+	else if (err)
 		repair = _(".git file broken");
+	else if (strcmp(get_worktree_id(dotgit_contents), wt->id))
+		fn(1, wt->path, _("unrelated .git file"), cb_data);
 	else if (fspathcmp(backlink.buf, repo.buf))
 		repair = _(".git file incorrect");
 	else if (use_relative_paths == is_absolute_path(dotgit_contents))

@@ -815,7 +821,7 @@ static ssize_t infer_backlink(struct repository *repo,
 	if (!*id)
 		goto error;
 	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
-	if (!is_directory(inferred->buf))
+	if (!is_git_directory(inferred->buf))
 		goto error;
 
 	return inferred->len;

@@ -882,6 +888,10 @@ void repair_worktree_at_path(struct repository *repo,
 		fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
 		goto done;
 	}
+	if (!inferred_backlink.len) {
+		fn(1, dotgit.buf, _("unable to locate repository; .git file is not for a linked worktree"), cb_data);
+		goto done;
+	}
 
 	/*
 	 * If we got this far, either the worktree's .git file pointed at a

@@ -899,7 +909,7 @@ void repair_worktree_at_path(struct repository *repo,
 	 * in the *original* repository, not in the "copy" repository).
 	 * Therefore, we prioritize inferred_backlink over backlink.
 	 */
-	if (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf))
+	if (fspathcmp(backlink.buf, inferred_backlink.buf))
 		strbuf_swap(&backlink, &inferred_backlink);
 
 	strbuf_addf(&gitdir, "%s/gitdir", backlink.buf);

Signature · 1 line

gitgitgadget

Yoichi NAKAYAMA

2026-10-08 03:50 UTC #6 lore raw

In reply to Yoichi NAKAYAMA via GitGitGadget

Friendly ping on this :)

Junio C Hamano

2026-10-08 20:55 UTC #2 lore raw

Re: [PATCH 1/2] worktree repair: refactor and reduce .git file reads

In reply to Yoichi NAKAYAMA via GitGitGadget

worktree.c int other_head_refs(struct repository *repo, · line 646

Quoted 7 lines · Yoichi NAKAYAMA via GitGitGadget

worktree.c +7 -0

+static const char *get_worktree_id(const char *dotgit_contents)
+{
+	const char *slash = find_last_dir_sep(dotgit_contents);
+	if (!slash)
+		return "";
+	return slash + 1;
+}

This returns a pointer into dotgit_contents; it is the last component of a pathname, similar to what basename(3) gives us.

Review static int is_main_worktree_path(struct repository *repo, const char *path)

Quoted 1 line

(quoted diff) +0 -0

@@ -798,30 +806,20 @@ static int is_main_worktree_path(struct repository *repo, const char *path)

The unified diff is a bit hard to follow, so let's see if we can compare preimage and postimage more easily.

Quoted 17 lines

worktree.c +0 ~~-10~~

 static ssize_t infer_backlink(struct repository *repo,
-			      const char *gitfile,
 			      struct strbuf *inferred)
 {
-	struct strbuf actual = STRBUF_INIT;
 	const char *id;
 
-	if (strbuf_read_file(&actual, gitfile, 0) < 0)
-		goto error;
-	if (!starts_with(actual.buf, "gitdir:"))
-		goto error;
-	if (!(id = find_last_dir_sep(actual.buf)))
-		goto error;
-	strbuf_trim(&actual);
-	id++; /* advance past '/' to point at <id> */
 	if (!*id)
 		goto error;

worktree.c static int is_main_worktree_path(struct repository *repo, const char *path) · line 824

Quoted 10 lines

worktree.c +0 -2

 	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
 	if (!is_directory(inferred->buf))
 		goto error;
 
-	strbuf_release(&actual);
 	return inferred->len;
 error:
-	strbuf_release(&actual);
 	strbuf_reset(inferred); /* clear invalid path */
 	return -1;

We used to receive the filename of ".git", read it and made sure we have "gitdir:" prefix, and find the last component, but then trimmed the actual buffer. Which means a few things.

- If the contents of the gitfile were "gitdir:foo/bar/baz \n", our id pointer found the slash after "foo/bar", trimmed the buffer to have "gitdir:foo/bar/baz", and then incremented id, which now points at "baz".

- If the contents of the gitfile were "gitdir: foo/bar/ \n", then
after triming, the buffer would have "gitdir: foo/bar/" and id would be pointing at the NUL at the end, which would have lead us to error.

Now let's look at the new code.

Quoted 10 lines

worktree.c +2 -0

@@ -798,30 +806,20 @@ static int is_main_worktree_path(struct repository *repo, const char *path)
  * Returns -1 on failure and strbuf.len on success.
  */
 static ssize_t infer_backlink(struct repository *repo,
+			      const char *dotgit_contents,
 			      struct strbuf *inferred)
 {
 	const char *id;
 
+	id = get_worktree_id(dotgit_contents);

worktree.c static int is_main_worktree_path(struct repository *repo, const char *path) · line 824

Quoted 10 lines

worktree.c +0 -0

 	if (!*id)
 		goto error;
 	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
 	if (!is_directory(inferred->buf))
 		goto error;
 
 	return inferred->len;
 error:
 	strbuf_reset(inferred); /* clear invalid path */
 	return -1;

The caller is expected to give us the contents of gitfile read by setup.c:read_gitfile_raw(), which reads the file in full, validates that the file begins with "gitdir: " (notice the trailing space), removes arbitrary run of CR or LF from the end, and then returns the string after skipping "gitdir: " prefix (8 bytes).

In the normal case, read_gitfile_raw() would see "gitdir: foo/bar/baz\n" in the file and returns "foo/bar/baz" to our caller. In fishy cases we examined for the preimage above:

- If the contents of the gitfile were "gitdir:foo/bar/baz \n", our caller would have received an error from read_gitfile_raw() and wouldn't have called us.

- If the contents of the gitfile were "gitdir: foo/bar/ \n", our
caller would have given us "foo/bar/ ".

get_worktree_id() will give us "baz" in the normal case, and " "
in the last case. We fail to error out in the latter with "*id" check, but is_directory() check will catch us, as the inferred
directory is "worktrees/ " in that bad case.

So there are certain differences in error cases, but they behave the same in the most basic cases.

Now, this is the caller in the preimage (i.e., what we used to do).

worktree.c void repair_worktree_at_path(struct repository *repo, · line 861

Quoted 7 lines

worktree.c +0 -3

@@ -856,51 +855,49 @@ void repair_worktree_at_path(struct repository *repo,
 		goto done;
 	}
 
-	infer_backlink(repo, dotgit.buf, &inferred_backlink);
-	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
-	dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));

We used to have infer_backlink() read the .git file to compute "worktree/$id", then again called read_gitfile_gently() to read it again.

worktree.c void repair_worktree_at_path(struct repository *repo, · line 863

Quoted 2 lines

worktree.c +0 -2

-	if (dotgit_contents) {
-		strbuf_addstr(&backlink, dotgit_contents);

This is the happy path. We successfully read from .git and use it.

worktree.c void repair_worktree_at_path(struct repository *repo, · line 862

Quoted 4 lines

worktree.c +0 -2

-	} else if (err == READ_GITFILE_ERR_NOT_A_FILE ||
-			err == READ_GITFILE_ERR_IS_A_DIR) {
 		fn(1, dotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
 		goto done;

This is inherited badness, but overly long lines like this one needs to be fixed.

worktree.c void repair_worktree_at_path(struct repository *repo, · line 868

Quoted 1 line

worktree.c +0 -1

-	} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {

The _gently() did read something, but that does not point at a git directory.

worktree.c void repair_worktree_at_path(struct repository *repo, · line 877

Quoted 9 lines

worktree.c +0 -9

-		if (inferred_backlink.len) {
-			/*
-			 * Worktree's .git file does not point at a repository
-			 * but we found a .git/worktrees/<id> in this
-			 * repository with the same <id> as recorded in the
-			 * worktree's .git file so make the worktree point at
-			 * the discovered .git/worktrees/<id>.
-			 */
-			strbuf_swap(&backlink, &inferred_backlink);

If we had the "worktree/$id" thing, we use it.

worktree.c void repair_worktree_at_path(struct repository *repo, · line 866

Quoted 8 lines

worktree.c +0 -5

-		} else {
-			fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
-			goto done;
-		}
-	} else {
 		fn(1, dotgit.buf, _("unable to locate repository; .git file broken"), cb_data);
 		goto done;
 	}

These lines to show error messages should also be folded to avoid overly long lines.

So, what does the updated code in the postimage do?

worktree.c void repair_worktree_at_path(struct repository *repo, · line 858

Quoted 5 lines

worktree.c +1 -0

@@ -856,51 +855,49 @@ void repair_worktree_at_path(struct repository *repo,
 		goto done;
 	}
 
+	err = read_gitfile_raw(&contents, dotgit.buf);

We use read_gitfile_raw() just once.

worktree.c void repair_worktree_at_path(struct repository *repo, · line 866

Quoted 8 lines

worktree.c +3 -0

+	if (err == READ_GITFILE_ERR_NOT_A_FILE ||
+	    err == READ_GITFILE_ERR_IS_A_DIR) {
 		fn(1, dotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
 		goto done;
+	} else if (err) {
 		fn(1, dotgit.buf, _("unable to locate repository; .git file broken"), cb_data);
 		goto done;
 	}

The original code handled the happy case that read_gitfile_gently() successfully returned first. Underlying read_gitfile_raw() would not have given any of these errors when read_gitfile_gently() succeeded, so handling the error cases first would not affect the behaviour of the code in these cases. Again, these overlong lines are annoying.

Now the simplest error cases are behind us. How would we do in the happy case?

worktree.c void repair_worktree_at_path(struct repository *repo, · line 870

Quoted 3 lines

worktree.c +3 -0

+	dotgit_contents = contents.buf;
+	infer_backlink(repo, dotgit_contents, &inferred_backlink);
+	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);

We reuse what we already read with read_gitfile_raw(), which prepared "worktrees/$id", and do the same realpath_forgiving() the original used to do a bit earlier.

Review void repair_worktree_at_path(struct repository *repo, · line 871

Quoted 2 lines

(quoted diff) +2 -0

+	if (is_absolute_path(dotgit_contents)) {
+		strbuf_addstr(&backlink, dotgit_contents);

I am not sure which part of the original this logic corresponds to. If the result from read_gitfile_raw() is an absolute path, even if it later turns out not to be is_git_directory(), the inferred backlink is not given a chance to act as a fallback. The original made a call to read_gitfile_gently() which checked is_git_directory() to give us an error, and that is how it allowed inferred backlink to substitute for a bad contents stored in .git file. Now we do not allow that fallback if .git file has an absolute path?

Ah, outside the context of this patch, before we barf for "unable to locate repository" when we complain backlink.buf is not naming a git directory, there is the fallback logic, and in order to reach there, we have "if (!is_git_directory(backlink.buf) && !inferred_backlink.len)" there. OK, so this may be doing the same thing as the original, but it is rather hard to follow and convince readers that this is a no-op conversion.

worktree.c void repair_worktree_at_path(struct repository *repo, · line 878

Quoted 5 lines

worktree.c +5 -0

+	} else {
+		strbuf_addbuf(&backlink, &dotgit);
+		strbuf_strip_suffix(&backlink, ".git");
+		strbuf_addstr(&backlink, dotgit_contents);
+		strbuf_realpath_forgiving(&backlink, backlink.buf, 0);

This converts dotgit_contents relative to the computed backlink, which needs to be done here because read_gitfile_gently() used to do that for us, which we no longer use.

worktree.c void repair_worktree_at_path(struct repository *repo, · line 884

Quoted 6 lines

worktree.c +6 -0

+	}
+
+	if (!is_git_directory(backlink.buf) && !inferred_backlink.len) {
+		fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
+		goto done;
+	}

I'll stop here.

Yoichi NAKAYAMA

2026-10-11 16:16 UTC #3 lore raw

Re: [PATCH 1/2] worktree repair: refactor and reduce .git file reads

Quoted 101 lines · Junio C Hamano

worktree.c int other_head_refs(struct repository *repo, · line 646
Quoted 7 lines · Yoichi NAKAYAMA via GitGitGadget
worktree.c +7 -0
+static const char *get_worktree_id(const char *dotgit_contents)
+{
+     const char *slash = find_last_dir_sep(dotgit_contents);
+     if (!slash)
+             return "";
+     return slash + 1;
+}
This returns a pointer into dotgit_contents; it is the last component of a pathname, similar to what basename(3) gives us.
Review static int is_main_worktree_path(struct repository *repo, const char *path)
Quoted 1 line
(quoted diff) +0 -0
@@ -798,30 +806,20 @@ static int is_main_worktree_path(struct repository *repo, const char *path)
The unified diff is a bit hard to follow, so let's see if we can compare preimage and postimage more easily.
Quoted 17 lines
worktree.c +0 ~~-10~~
 static ssize_t infer_backlink(struct repository *repo,
-                           const char *gitfile,
                            struct strbuf *inferred)
 {
-     struct strbuf actual = STRBUF_INIT;
      const char *id;
 
-     if (strbuf_read_file(&actual, gitfile, 0) < 0)
-             goto error;
-     if (!starts_with(actual.buf, "gitdir:"))
-             goto error;
-     if (!(id = find_last_dir_sep(actual.buf)))
-             goto error;
-     strbuf_trim(&actual);
-     id++; /* advance past '/' to point at <id> */
      if (!*id)
              goto error;
worktree.c static int is_main_worktree_path(struct repository *repo, const char *path) · line 824
Quoted 10 lines
worktree.c +0 -2
      repo_common_path_replace(repo, inferred, "worktrees/%s", id);
      if (!is_directory(inferred->buf))
              goto error;
 
-     strbuf_release(&actual);
      return inferred->len;
 error:
-     strbuf_release(&actual);
      strbuf_reset(inferred); /* clear invalid path */
      return -1;
We used to receive the filename of ".git", read it and made sure we have "gitdir:" prefix, and find the last component, but then trimmed the actual buffer. Which means a few things.
- If the contents of the gitfile were "gitdir:foo/bar/baz \n", our id pointer found the slash after "foo/bar", trimmed the buffer to have "gitdir:foo/bar/baz", and then incremented id, which now points at "baz".
- If the contents of the gitfile were "gitdir: foo/bar/ \n", then
after triming, the buffer would have "gitdir: foo/bar/" and id would be pointing at the NUL at the end, which would have lead us to error.
Now let's look at the new code.
Quoted 10 lines
worktree.c +2 -0
@@ -798,30 +806,20 @@ static int is_main_worktree_path(struct repository *repo, const char *path)
  * Returns -1 on failure and strbuf.len on success.
  */
 static ssize_t infer_backlink(struct repository *repo,
+                           const char *dotgit_contents,
                            struct strbuf *inferred)
 {
      const char *id;
 
+     id = get_worktree_id(dotgit_contents);
worktree.c static int is_main_worktree_path(struct repository *repo, const char *path) · line 824
Quoted 10 lines
worktree.c +0 -0
      if (!*id)
              goto error;
      repo_common_path_replace(repo, inferred, "worktrees/%s", id);
      if (!is_directory(inferred->buf))
              goto error;
 
      return inferred->len;
 error:
      strbuf_reset(inferred); /* clear invalid path */
      return -1;
The caller is expected to give us the contents of gitfile read by setup.c:read_gitfile_raw(), which reads the file in full, validates that the file begins with "gitdir: " (notice the trailing space), removes arbitrary run of CR or LF from the end, and then returns the string after skipping "gitdir: " prefix (8 bytes).
In the normal case, read_gitfile_raw() would see "gitdir: foo/bar/baz\n" in the file and returns "foo/bar/baz" to our caller. In fishy cases we examined for the preimage above:
- If the contents of the gitfile were "gitdir:foo/bar/baz \n", our caller would have received an error from read_gitfile_raw() and wouldn't have called us.
- If the contents of the gitfile were "gitdir: foo/bar/ \n", our
caller would have given us "foo/bar/ ".

Quoted 7 lines

get_worktree_id() will give us "baz" in the normal case, and " "
in the last case. We fail to error out in the latter with "*id" check, but is_directory() check will catch us, as the inferred
directory is "worktrees/ " in that bad case.
So there are certain differences in error cases, but they behave the same in the most basic cases.

infer_backlink() in the preimage accepts the "gitdir:" prefix without requiring a following space and uses strbuf_trim(), which does more than just remove trailing newlines. This is more lenient than read_gitfile_gently(), which is called when other Git commands are used.
I believe the primary purpose of worktree repair is to fix common issues like worktree relocation, with repairing cases where the .git file's contents were manually modified being a secondary concern.
Therefore, I have adjusted the behavior for error cases: if the file deviates from the standard .git file format, it is treated as ".git file broken"; if there
are trailing whitespaces, they are interpreted as part of the path and treated as ".git file does not reference a repository."
I would like to split the commits and provide a detailed explanation for this.

Quoted 7 lines

worktree.c void repair_worktree_at_path(struct repository *repo, · line 862
Quoted 4 lines
worktree.c +0 -2
-     } else if (err == READ_GITFILE_ERR_NOT_A_FILE ||
-                     err == READ_GITFILE_ERR_IS_A_DIR) {
              fn(1, dotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
              goto done;
This is inherited badness, but overly long lines like this one needs to be fixed.

I'll fold long lines near the changes.

Quoted 7 lines

Ah, outside the context of this patch, before we barf for "unable to locate repository" when we complain backlink.buf is not naming a git directory, there is the fallback logic, and in order to reach there, we have "if (!is_git_directory(backlink.buf) && !inferred_backlink.len)" there. OK, so this may be doing the same thing as the original, but it is rather hard to follow and convince readers that this is a no-op conversion.

The first commit contained multiple changes, making it difficult to follow. I will split the commit to distinguish between the parts where the behavior is modified and those where it remains unchanged, making it easier to verify that no changes were made to the latter.

Thanks,

Signature · 1 line

Yoichi NAKAYAMA

Yoichi NAKAYAMA

2026-10-12 19:57 UTC #4 lore raw

Re: [PATCH 1/2] worktree repair: refactor and reduce .git file reads

Quoted 111 lines · Yoichi NAKAYAMA

Quoted 101 lines · Junio C Hamano
worktree.c int other_head_refs(struct repository *repo, · line 646
Quoted 7 lines · Yoichi NAKAYAMA via GitGitGadget
worktree.c +7 -0
+static const char *get_worktree_id(const char *dotgit_contents)
+{
+     const char *slash = find_last_dir_sep(dotgit_contents);
+     if (!slash)
+             return "";
+     return slash + 1;
+}
This returns a pointer into dotgit_contents; it is the last component of a pathname, similar to what basename(3) gives us.
Review static int is_main_worktree_path(struct repository *repo, const char *path)
Quoted 1 line
(quoted diff) +0 -0
@@ -798,30 +806,20 @@ static int is_main_worktree_path(struct repository *repo, const char *path)
The unified diff is a bit hard to follow, so let's see if we can compare preimage and postimage more easily.
Quoted 17 lines
worktree.c +0 ~~-10~~
 static ssize_t infer_backlink(struct repository *repo,
-                           const char *gitfile,
                            struct strbuf *inferred)
 {
-     struct strbuf actual = STRBUF_INIT;
      const char *id;
 
-     if (strbuf_read_file(&actual, gitfile, 0) < 0)
-             goto error;
-     if (!starts_with(actual.buf, "gitdir:"))
-             goto error;
-     if (!(id = find_last_dir_sep(actual.buf)))
-             goto error;
-     strbuf_trim(&actual);
-     id++; /* advance past '/' to point at <id> */
      if (!*id)
              goto error;
worktree.c static int is_main_worktree_path(struct repository *repo, const char *path) · line 824
Quoted 10 lines
worktree.c +0 -2
      repo_common_path_replace(repo, inferred, "worktrees/%s", id);
      if (!is_directory(inferred->buf))
              goto error;
 
-     strbuf_release(&actual);
      return inferred->len;
 error:
-     strbuf_release(&actual);
      strbuf_reset(inferred); /* clear invalid path */
      return -1;
We used to receive the filename of ".git", read it and made sure we have "gitdir:" prefix, and find the last component, but then trimmed the actual buffer. Which means a few things.
- If the contents of the gitfile were "gitdir:foo/bar/baz \n", our id pointer found the slash after "foo/bar", trimmed the buffer to have "gitdir:foo/bar/baz", and then incremented id, which now points at "baz".
- If the contents of the gitfile were "gitdir: foo/bar/ \n", then
after triming, the buffer would have "gitdir: foo/bar/" and id would be pointing at the NUL at the end, which would have lead us to error.
Now let's look at the new code.
Quoted 10 lines
worktree.c +2 -0
@@ -798,30 +806,20 @@ static int is_main_worktree_path(struct repository *repo, const char *path)
  * Returns -1 on failure and strbuf.len on success.
  */
 static ssize_t infer_backlink(struct repository *repo,
+                           const char *dotgit_contents,
                            struct strbuf *inferred)
 {
      const char *id;
 
+     id = get_worktree_id(dotgit_contents);
worktree.c static int is_main_worktree_path(struct repository *repo, const char *path) · line 824
Quoted 10 lines
worktree.c +0 -0
      if (!*id)
              goto error;
      repo_common_path_replace(repo, inferred, "worktrees/%s", id);
      if (!is_directory(inferred->buf))
              goto error;
 
      return inferred->len;
 error:
      strbuf_reset(inferred); /* clear invalid path */
      return -1;
The caller is expected to give us the contents of gitfile read by setup.c:read_gitfile_raw(), which reads the file in full, validates that the file begins with "gitdir: " (notice the trailing space), removes arbitrary run of CR or LF from the end, and then returns the string after skipping "gitdir: " prefix (8 bytes).
In the normal case, read_gitfile_raw() would see "gitdir: foo/bar/baz\n" in the file and returns "foo/bar/baz" to our caller. In fishy cases we examined for the preimage above:
- If the contents of the gitfile were "gitdir:foo/bar/baz \n", our caller would have received an error from read_gitfile_raw() and wouldn't have called us.
- If the contents of the gitfile were "gitdir: foo/bar/ \n", our
caller would have given us "foo/bar/ ".
Quoted 7 lines
get_worktree_id() will give us "baz" in the normal case, and " "
in the last case. We fail to error out in the latter with "*id" check, but is_directory() check will catch us, as the inferred
directory is "worktrees/ " in that bad case.
So there are certain differences in error cases, but they behave the same in the most basic cases.

Quoted 12 lines

infer_backlink() in the preimage accepts the "gitdir:" prefix without requiring a following space and uses strbuf_trim(), which does more than just remove trailing newlines. This is more lenient than read_gitfile_gently(), which is called when other Git commands are used.
I believe the primary purpose of worktree repair is to fix common issues like worktree relocation, with repairing cases where the .git file's contents were manually modified being a secondary concern.
Therefore, I have adjusted the behavior for error cases: if the file deviates from the standard .git file format, it is treated as ".git file broken"; if there
are trailing whitespaces, they are interpreted as part of the path and treated as ".git file does not reference a repository."

The former case (where there is no space after "gitdir:") behaved this way even before the change. repair_worktree_at_path() calls read_gitfile_gently() and returns err = READ_GITFILE_ERR_INVALID_FORMAT; this behavior remains unchanged.

Thanks,

Signature · 1 line

Yoichi NAKAYAMA

Yoichi NAKAYAMA via GitGitGadget

author

2026-10-14 21:20 UTC #7 lore raw

v2 cover worktree repair: avoid breaking unrelated .git file and gitdir

In reply to Yoichi NAKAYAMA via GitGitGadget

v2 (0156021)
============

Split the first commit of v1 to make the changes easier to understand. Fold long lines.

* [1/5] align .git file handling with repair_gitfile()
* [2/5] centralize inferred backlink selection
* [3/5] use read_gitfile_raw() in infer_backlink()
* [4/5] reduce .git file reads
* [5/5] avoid breaking unrelated .git file and gitdir

v1 (99aa341)
============

This series first refactors the code to read the .git file once and extract the worktree ID, then uses that information to validate the repair target before modifying the cross-references.

* [1/2] Refactor the code without changing functionality before making the fix
* [2/2] Validate the worktree ID and inferred gitdir path before repairing

Yoichi NAKAYAMA (5):
worktree repair: align .git file handling with repair_gitfile() worktree repair: centralize inferred backlink selection worktree repair: use read_gitfile_raw() in infer_backlink() worktree repair: reduce .git file reads
worktree repair: avoid breaking unrelated .git file and gitdir

 t/t2406-worktree-repair.sh |  33 +++++++++--
 worktree.c                 | 113 +++++++++++++++++++++----------------
 2 files changed, 91 insertions(+), 55 deletions(-)

base-commit: 6de20f6092dcf9bdb1c8efe03db4b70c82b423dd
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2225%2Fyoichi%2Fworktree-repair-keep-unrelated-gitfile-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2225/yoichi/worktree-repair-keep-unrelated-gitfile-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2225

Range-diff vs v1:

 -:  ---------- > 1:  21b2878aa5 worktree repair: align .git file handling with repair_gitfile()
 1:  dc7ebb427b ! 2:  30ac7acad3 worktree repair: refactor and reduce .git file reads
     @@ Metadata
      Author: Yoichi NAKAYAMA <[email protected]>
      
       ## Commit message ##
     -    worktree repair: refactor and reduce .git file reads
     +    worktree repair: centralize inferred backlink selection
      
     -    Remove the file reading and trimming logic from `infer_backlink()`,
     -    and instead read the .git file once in its caller,
     -    `repair_worktree_at_path()`, using `read_gitfile_raw()`. Since
     -    `read_gitfile_gently()` is replaced with `read_gitfile_raw()`, restore
     -    the logic for constructing the absolute path and replace the
     -    READ_GITFILE_ERR_NOT_A_REPO handling with a check using
     -    `is_git_directory()`. Simplify the logic for prioritizing
     -    'inferred_backlink' over 'backlink'.
     -
     -    Extract `get_worktree_id()` to get the worktree ID from the contents
     -    of the .git file. We are going to modify and use this function in
     -    subsequent commits.
     +    Reorder the processing steps to centralize the logic for preferring
     +    the inferred_backlink.
      
          Signed-off-by: Yoichi NAKAYAMA <[email protected]>
      
       ## worktree.c ##
     -@@ worktree.c: int other_head_refs(struct repository *repo,
     - 	return ret;
     - }
     - 
     -+static const char *get_worktree_id(const char *dotgit_contents)
     -+{
     -+	const char *slash = find_last_dir_sep(dotgit_contents);
     -+	if (!slash)
     -+		return "";
     -+	return slash + 1;
     -+}
     -+
     - /*
     -  * Repair worktree's /path/to/worktree/.git file if missing, corrupt, or not
     -  * pointing at <repo>/worktrees/<id>.
     -@@ worktree.c: static int is_main_worktree_path(struct repository *repo, const char *path)
     -  * Returns -1 on failure and strbuf.len on success.
     -  */
     - static ssize_t infer_backlink(struct repository *repo,
     --			      const char *gitfile,
     -+			      const char *dotgit_contents,
     - 			      struct strbuf *inferred)
     - {
     --	struct strbuf actual = STRBUF_INIT;
     - 	const char *id;
     - 
     --	if (strbuf_read_file(&actual, gitfile, 0) < 0)
     --		goto error;
     --	if (!starts_with(actual.buf, "gitdir:"))
     --		goto error;
     --	if (!(id = find_last_dir_sep(actual.buf)))
     --		goto error;
     --	strbuf_trim(&actual);
     --	id++; /* advance past '/' to point at <id> */
     -+	id = get_worktree_id(dotgit_contents);
     - 	if (!*id)
     - 		goto error;
     - 	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
     - 	if (!is_directory(inferred->buf))
     - 		goto error;
     - 
     --	strbuf_release(&actual);
     - 	return inferred->len;
     - error:
     --	strbuf_release(&actual);
     - 	strbuf_reset(inferred); /* clear invalid path */
     - 	return -1;
     - }
      @@ worktree.c: void repair_worktree_at_path(struct repository *repo,
     - 	struct strbuf inferred_backlink = STRBUF_INIT;
       	struct strbuf gitdir = STRBUF_INIT;
       	struct strbuf olddotgit = STRBUF_INIT;
     --	char *dotgit_contents = NULL;
     -+	struct strbuf contents = STRBUF_INIT;
     + 	struct strbuf contents = STRBUF_INIT;
      +	const char *dotgit_contents = NULL;
       	const char *repair = NULL;
       	int err;
     @@ worktree.c: void repair_worktree_at_path(struct repository *repo,
       
      -	infer_backlink(repo, dotgit.buf, &inferred_backlink);
      -	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
     --	dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));
     --	if (dotgit_contents) {
     --		strbuf_addstr(&backlink, dotgit_contents);
     --	} else if (err == READ_GITFILE_ERR_NOT_A_FILE ||
     --			err == READ_GITFILE_ERR_IS_A_DIR) {
     -+	err = read_gitfile_raw(&contents, dotgit.buf);
     -+	if (err == READ_GITFILE_ERR_NOT_A_FILE ||
     -+	    err == READ_GITFILE_ERR_IS_A_DIR) {
     - 		fn(1, dotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
     + 	err = read_gitfile_raw(&contents, dotgit.buf);
     +-	if (!err) {
     +-		const char *dotgit_contents = contents.buf;
     +-		if (is_absolute_path(dotgit_contents)) {
     +-			strbuf_addstr(&backlink, dotgit_contents);
     +-		} else {
     +-			strbuf_addbuf(&backlink, &dotgit);
     +-			strbuf_strip_suffix(&backlink, ".git");
     +-			strbuf_addstr(&backlink, dotgit_contents);
     +-			strbuf_realpath_forgiving(&backlink, backlink.buf, 0);
     +-		}
     +-	}
     + 	if (err == READ_GITFILE_ERR_NOT_A_FILE ||
     + 	    err == READ_GITFILE_ERR_IS_A_DIR) {
     + 		fn(1, dotgit.buf,
     + 		   _("unable to locate repository; .git is not a file"),
     + 		   cb_data);
       		goto done;
     --	} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {
     +-	} else if (!err && !is_git_directory(backlink.buf)) {
     +-		strbuf_release(&backlink);
      -		if (inferred_backlink.len) {
      -			/*
      -			 * Worktree's .git file does not point at a repository
     @@ worktree.c: void repair_worktree_at_path(struct repository *repo,
      -			 */
      -			strbuf_swap(&backlink, &inferred_backlink);
      -		} else {
     --			fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
     +-			fn(1, dotgit.buf,
     +-			   _("unable to locate repository; .git file does not reference a repository"),
     +-			   cb_data);
      -			goto done;
      -		}
     --	} else {
     -+	} else if (err) {
     - 		fn(1, dotgit.buf, _("unable to locate repository; .git file broken"), cb_data);
     + 	} else if (err) {
     + 		fn(1, dotgit.buf,
     + 		   _("unable to locate repository; .git file broken"),
     +@@ worktree.c: void repair_worktree_at_path(struct repository *repo,
       		goto done;
       	}
       
      +	dotgit_contents = contents.buf;
     -+	infer_backlink(repo, dotgit_contents, &inferred_backlink);
     ++	infer_backlink(repo, dotgit.buf, &inferred_backlink);
      +	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
      +
      +	if (is_absolute_path(dotgit_contents)) {
     @@ worktree.c: void repair_worktree_at_path(struct repository *repo,
      +	}
      +
      +	if (!is_git_directory(backlink.buf) && !inferred_backlink.len) {
     -+		fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
     ++		fn(1, dotgit.buf,
     ++		   _("unable to locate repository; .git file does not reference a repository"),
     ++		   cb_data);
      +		goto done;
      +	}
      +
     @@ worktree.c: void repair_worktree_at_path(struct repository *repo,
       	 */
       	if (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf))
       		strbuf_swap(&backlink, &inferred_backlink);
     -@@ worktree.c: void repair_worktree_at_path(struct repository *repo,
     - 					     gitdir.buf, use_relative_paths);
     - 	}
     - done:
     --	free(dotgit_contents);
     - 	strbuf_release(&olddotgit);
     - 	strbuf_release(&backlink);
     - 	strbuf_release(&inferred_backlink);
     - 	strbuf_release(&gitdir);
     - 	strbuf_release(&dotgit);
     -+	strbuf_release(&contents);
     - }
     - 
     - int should_prune_worktree(struct repository *repo,
 -:  ---------- > 3:  2cc7b817ee worktree repair: use read_gitfile_raw() in infer_backlink()
 -:  ---------- > 4:  8c80b9ae05 worktree repair: reduce .git file reads
 2:  99aa34135c ! 5:  015602168c worktree repair: avoid breaking unrelated .git file and gitdir
     @@ worktree.c: static ssize_t infer_backlink(struct repository *repo,
       
       	return inferred->len;
      @@ worktree.c: void repair_worktree_at_path(struct repository *repo,
     - 		fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
     + 		   cb_data);
       		goto done;
       	}
      +	if (!inferred_backlink.len) {
     -+		fn(1, dotgit.buf, _("unable to locate repository; .git file is not for a linked worktree"), cb_data);
     ++		fn(1, dotgit.buf,
     ++		   _("unable to locate repository; .git file is not for a linked worktree"),
     ++		   cb_data);
      +		goto done;
      +	}

Signature · 1 line

gitgitgadget

Yoichi NAKAYAMA via GitGitGadget

author

2026-10-14 21:20 UTC #8 lore raw

PATCH v2 1/5 worktree repair: align .git file handling with repair_gitfile()

From: Yoichi NAKAYAMA <[email protected]>

Use read_gitfile_raw() instead of read_gitfile_gently() in repair_worktree_at_path(), moving path resolution and repository validation to the caller. This makes .git file handling consistent with repair_gitfile() and avoids using the non-reentrant read_gitfile_gently().

A subsequent commit will update infer_backlink() to use read_gitfile_raw() as well.

Signed-off-by Yoichi NAKAYAMA <[email protected]>

---
 worktree.c | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

worktree.c +28 ~~-12~~

index 8cb8637b18..4492ca835d 100644
@@ -840,7 +840,7 @@ void repair_worktree_at_path(struct repository *repo,
 	struct strbuf inferred_backlink = STRBUF_INIT;
 	struct strbuf gitdir = STRBUF_INIT;
 	struct strbuf olddotgit = STRBUF_INIT;
-	char *dotgit_contents = NULL;
+	struct strbuf contents = STRBUF_INIT;
 	const char *repair = NULL;
 	int err;

@@ -858,14 +858,26 @@ void repair_worktree_at_path(struct repository *repo,
 
 	infer_backlink(repo, dotgit.buf, &inferred_backlink);
 	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
-	dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));
-	if (dotgit_contents) {
-		strbuf_addstr(&backlink, dotgit_contents);
-	} else if (err == READ_GITFILE_ERR_NOT_A_FILE ||
-			err == READ_GITFILE_ERR_IS_A_DIR) {
-		fn(1, dotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
+	err = read_gitfile_raw(&contents, dotgit.buf);
+	if (!err) {
+		const char *dotgit_contents = contents.buf;
+		if (is_absolute_path(dotgit_contents)) {
+			strbuf_addstr(&backlink, dotgit_contents);
+		} else {
+			strbuf_addbuf(&backlink, &dotgit);
+			strbuf_strip_suffix(&backlink, ".git");
+			strbuf_addstr(&backlink, dotgit_contents);
+			strbuf_realpath_forgiving(&backlink, backlink.buf, 0);
+		}
+	}
+	if (err == READ_GITFILE_ERR_NOT_A_FILE ||
+	    err == READ_GITFILE_ERR_IS_A_DIR) {
+		fn(1, dotgit.buf,
+		   _("unable to locate repository; .git is not a file"),
+		   cb_data);
 		goto done;
-	} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {
+	} else if (!err && !is_git_directory(backlink.buf)) {
+		strbuf_release(&backlink);
 		if (inferred_backlink.len) {
 			/*
 			 * Worktree's .git file does not point at a repository

@@ -876,11 +888,15 @@ void repair_worktree_at_path(struct repository *repo,
 			 */
 			strbuf_swap(&backlink, &inferred_backlink);
 		} else {
-			fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
+			fn(1, dotgit.buf,
+			   _("unable to locate repository; .git file does not reference a repository"),
+			   cb_data);
 			goto done;
 		}
-	} else {
-		fn(1, dotgit.buf, _("unable to locate repository; .git file broken"), cb_data);
+	} else if (err) {
+		fn(1, dotgit.buf,
+		   _("unable to locate repository; .git file broken"),
+		   cb_data);
 		goto done;
 	}

@@ -926,12 +942,12 @@ void repair_worktree_at_path(struct repository *repo,
 					     gitdir.buf, use_relative_paths);
 	}
 done:
-	free(dotgit_contents);
 	strbuf_release(&olddotgit);
 	strbuf_release(&backlink);
 	strbuf_release(&inferred_backlink);
 	strbuf_release(&gitdir);
 	strbuf_release(&dotgit);
+	strbuf_release(&contents);
 }
 
 int should_prune_worktree(struct repository *repo,

Signature · 1 line

gitgitgadget

Yoichi NAKAYAMA via GitGitGadget

author

2026-10-14 21:20 UTC #9 lore raw

PATCH v2 2/5 worktree repair: centralize inferred backlink selection

In reply to Yoichi NAKAYAMA via GitGitGadget

From: Yoichi NAKAYAMA <[email protected]>

Reorder the processing steps to centralize the logic for preferring the inferred_backlink.

Signed-off-by Yoichi NAKAYAMA <[email protected]>

---
 worktree.c | 69 +++++++++++++++++++++++-------------------------------
 1 file changed, 29 insertions(+), 40 deletions(-)

worktree.c +29 ~~-40~~

index 4492ca835d..f8e9850f6f 100644
@@ -841,6 +841,7 @@ void repair_worktree_at_path(struct repository *repo,
 	struct strbuf gitdir = STRBUF_INIT;
 	struct strbuf olddotgit = STRBUF_INIT;
 	struct strbuf contents = STRBUF_INIT;
+	const char *dotgit_contents = NULL;
 	const char *repair = NULL;
 	int err;

@@ -856,43 +857,13 @@ void repair_worktree_at_path(struct repository *repo,
 		goto done;
 	}
 
-	infer_backlink(repo, dotgit.buf, &inferred_backlink);
-	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
 	err = read_gitfile_raw(&contents, dotgit.buf);
-	if (!err) {
-		const char *dotgit_contents = contents.buf;
-		if (is_absolute_path(dotgit_contents)) {
-			strbuf_addstr(&backlink, dotgit_contents);
-		} else {
-			strbuf_addbuf(&backlink, &dotgit);
-			strbuf_strip_suffix(&backlink, ".git");
-			strbuf_addstr(&backlink, dotgit_contents);
-			strbuf_realpath_forgiving(&backlink, backlink.buf, 0);
-		}
-	}
 	if (err == READ_GITFILE_ERR_NOT_A_FILE ||
 	    err == READ_GITFILE_ERR_IS_A_DIR) {
 		fn(1, dotgit.buf,
 		   _("unable to locate repository; .git is not a file"),
 		   cb_data);
 		goto done;
-	} else if (!err && !is_git_directory(backlink.buf)) {
-		strbuf_release(&backlink);
-		if (inferred_backlink.len) {
-			/*
-			 * Worktree's .git file does not point at a repository
-			 * but we found a .git/worktrees/<id> in this
-			 * repository with the same <id> as recorded in the
-			 * worktree's .git file so make the worktree point at
-			 * the discovered .git/worktrees/<id>.
-			 */
-			strbuf_swap(&backlink, &inferred_backlink);
-		} else {
-			fn(1, dotgit.buf,
-			   _("unable to locate repository; .git file does not reference a repository"),
-			   cb_data);
-			goto done;
-		}
 	} else if (err) {
 		fn(1, dotgit.buf,
 		   _("unable to locate repository; .git file broken"),

@@ -900,23 +871,41 @@ void repair_worktree_at_path(struct repository *repo,
 		goto done;
 	}
 
+	dotgit_contents = contents.buf;
+	infer_backlink(repo, dotgit.buf, &inferred_backlink);
+	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
+
+	if (is_absolute_path(dotgit_contents)) {
+		strbuf_addstr(&backlink, dotgit_contents);
+	} else {
+		strbuf_addbuf(&backlink, &dotgit);
+		strbuf_strip_suffix(&backlink, ".git");
+		strbuf_addstr(&backlink, dotgit_contents);
+		strbuf_realpath_forgiving(&backlink, backlink.buf, 0);
+	}
+
+	if (!is_git_directory(backlink.buf) && !inferred_backlink.len) {
+		fn(1, dotgit.buf,
+		   _("unable to locate repository; .git file does not reference a repository"),
+		   cb_data);
+		goto done;
+	}
+
 	/*
 	 * If we got this far, either the worktree's .git file pointed at a
-	 * valid repository (i.e. read_gitfile_gently() returned success) or
+	 * valid repository (i.e. is_git_directory() returned true) or
 	 * the .git file did not point at a repository but we were able to
 	 * infer a suitable new value for the .git file by locating a
 	 * .git/worktrees/<id> in *this* repository corresponding to the <id>
 	 * recorded in the worktree's .git file.
 	 *
-	 * However, if, at this point, inferred_backlink is non-NULL (i.e. we
-	 * found a suitable .git/worktrees/<id> in *this* repository) *and* the
-	 * worktree's .git file points at a valid repository *and* those two
-	 * paths differ, then that indicates that the user probably *copied*
-	 * the main and linked worktrees to a new location as a unit rather
-	 * than *moving* them. Thus, the copied worktree's .git file actually
-	 * points at the .git/worktrees/<id> in the *original* repository, not
-	 * in the "copy" repository. In this case, point the "copy" worktree's
-	 * .git file at the "copy" repository.
+	 * Even if the worktree's .git file pointed at a valid repository,
+	 * it doesn't always mean that the backlink is correct. For example,
+	 * the user might have *copied* the main and linked worktrees to a
+	 * new location as a unit rather than *moving* them (the copied
+	 * worktree's .git file actually points at the .git/worktrees/<id>
+	 * in the *original* repository, not in the "copy" repository).
+	 * Therefore, we prioritize inferred_backlink over backlink.
 	 */
 	if (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf))
 		strbuf_swap(&backlink, &inferred_backlink);

Signature · 1 line

gitgitgadget

Yoichi NAKAYAMA via GitGitGadget

author

2026-10-14 21:20 UTC #10 lore raw

PATCH v2 3/5 worktree repair: use read_gitfile_raw() in infer_backlink()

In reply to Yoichi NAKAYAMA via GitGitGadget

From: Yoichi NAKAYAMA <[email protected]>

Use read_gitfile_raw() in infer_backlink() to parse .git files consistently with repair_gitfile(). Unlike strbuf_trim(), read_gitfile_raw() does not silently discard trailing whitespace. Such whitespace is treated as part of the path and may cause the path to be treated as not referencing a repository.

Extract get_worktree_id() for use in a subsequent commit.

Signed-off-by Yoichi NAKAYAMA <[email protected]>

---
 worktree.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

worktree.c +15 ~~-10~~

index f8e9850f6f..3627dbc044 100644
@@ -637,6 +637,14 @@ int other_head_refs(struct repository *repo,
 	return ret;
 }
 
+static const char *get_worktree_id(const char *dotgit_contents)
+{
+	const char *slash = find_last_dir_sep(dotgit_contents);
+	if (!slash)
+		return "";
+	return slash + 1;
+}
+
 /*
  * Repair worktree's /path/to/worktree/.git file if missing, corrupt, or not
  * pointing at <repo>/worktrees/<id>.

@@ -801,27 +809,24 @@ static ssize_t infer_backlink(struct repository *repo,
 			      const char *gitfile,
 			      struct strbuf *inferred)
 {
-	struct strbuf actual = STRBUF_INIT;
+	struct strbuf contents = STRBUF_INIT;
 	const char *id;
+	int err;
 
-	if (strbuf_read_file(&actual, gitfile, 0) < 0)
+	err = read_gitfile_raw(&contents, gitfile);
+	if (err)
 		goto error;
-	if (!starts_with(actual.buf, "gitdir:"))
-		goto error;
-	if (!(id = find_last_dir_sep(actual.buf)))
-		goto error;
-	strbuf_trim(&actual);
-	id++; /* advance past '/' to point at <id> */
+	id = get_worktree_id(contents.buf);
 	if (!*id)
 		goto error;
 	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
 	if (!is_directory(inferred->buf))
 		goto error;
 
-	strbuf_release(&actual);
+	strbuf_release(&contents);
 	return inferred->len;
 error:
-	strbuf_release(&actual);
+	strbuf_release(&contents);
 	strbuf_reset(inferred); /* clear invalid path */
 	return -1;
 }

Signature · 1 line

gitgitgadget

Yoichi NAKAYAMA via GitGitGadget

author

2026-10-14 21:20 UTC #11 lore raw

PATCH v2 4/5 worktree repair: reduce .git file reads

In reply to Yoichi NAKAYAMA via GitGitGadget

From: Yoichi NAKAYAMA <[email protected]>

Pass the loaded contents of the .git file to infer_backlink().

Signed-off-by Yoichi NAKAYAMA <[email protected]>

---
 worktree.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

worktree.c +3 ~~-10~~

index 3627dbc044..879941a977 100644
@@ -806,27 +806,20 @@ static int is_main_worktree_path(struct repository *repo, const char *path)
  * Returns -1 on failure and strbuf.len on success.
  */
 static ssize_t infer_backlink(struct repository *repo,
-			      const char *gitfile,
+			      const char *dotgit_contents,
 			      struct strbuf *inferred)
 {
-	struct strbuf contents = STRBUF_INIT;
 	const char *id;
-	int err;
 
-	err = read_gitfile_raw(&contents, gitfile);
-	if (err)
-		goto error;
-	id = get_worktree_id(contents.buf);
+	id = get_worktree_id(dotgit_contents);
 	if (!*id)
 		goto error;
 	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
 	if (!is_directory(inferred->buf))
 		goto error;
 
-	strbuf_release(&contents);
 	return inferred->len;
 error:
-	strbuf_release(&contents);
 	strbuf_reset(inferred); /* clear invalid path */
 	return -1;
 }

@@ -877,7 +870,7 @@ void repair_worktree_at_path(struct repository *repo,
 	}
 
 	dotgit_contents = contents.buf;
-	infer_backlink(repo, dotgit.buf, &inferred_backlink);
+	infer_backlink(repo, dotgit_contents, &inferred_backlink);
 	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
 
 	if (is_absolute_path(dotgit_contents)) {

Signature · 1 line

gitgitgadget

Yoichi NAKAYAMA via GitGitGadget

author

2026-10-14 21:20 UTC #12 lore raw

PATCH v2 5/5 worktree repair: avoid breaking unrelated .git file and gitdir

In reply to Yoichi NAKAYAMA via GitGitGadget

From: Yoichi NAKAYAMA <[email protected]>

Resolve these issues by validating the worktree ID and stopping the repair when the ID does not match or the gitdir path cannot be inferred.

Signed-off-by Yoichi NAKAYAMA <[email protected]>

---
 t/t2406-worktree-repair.sh | 33 +++++++++++++++++++++++++++------
 worktree.c                 | 20 ++++++++++++++++----
 2 files changed, 43 insertions(+), 10 deletions(-)

t/t2406-worktree-repair.sh +27 -6

index d4e53d492b..2ffa123f42 100755
@@ -56,15 +56,12 @@ test_expect_success 'repair missing .git file' '
 '
 
 test_expect_success 'repair bogus .git file' '
-	test_corrupt_gitfile "echo \"gitdir: /nowhere\" >corrupt/.git" \
+	test_corrupt_gitfile "echo \"contents not started with gitdir:\" >corrupt/.git" \
 		".git file broken"
 '
 
-test_expect_success 'repair incorrect .git file' '
-	test_when_finished "rm -rf other && git worktree prune" &&
-	test_create_repo other &&
-	other=$(git -C other rev-parse --absolute-git-dir) &&
-	test_corrupt_gitfile "echo \"gitdir: $other\" >corrupt/.git" \
+test_expect_success 'repair unlinked .git file' '
+	test_corrupt_gitfile "echo \"gitdir: /nowhere/worktrees/corrupt\" >corrupt/.git" \
 		".git file incorrect"
 '

@@ -89,6 +86,18 @@ test_expect_success 'repair .git file from bare.git' '
 	test_cmp expect actual
 '
 
+test_expect_success 'skip unrelated .git file' '
+	test_when_finished "rm -rf corrupt other && git worktree prune" &&
+	git worktree add --detach corrupt &&
+	rm -rf corrupt &&
+	git worktree add --detach other &&
+	mv other corrupt &&
+	cat corrupt/.git >expect &&
+	test_must_fail git worktree repair 2>err &&
+	test_cmp expect corrupt/.git &&
+	test_grep "unrelated .git file" err
+'
+
 test_expect_success 'invalid worktree path' '
 	test_must_fail git worktree repair /notvalid >out 2>err &&
 	test_must_be_empty out &&

@@ -113,6 +122,18 @@ test_expect_success 'repo not found; .git not referencing repo' '
 	test_grep ".git file does not reference a repository" err
 '
 
+test_expect_success 'repo not found; .git not for worktree' '
+	test_when_finished "rm -rf side other-repo && git worktree prune" &&
+	test_create_repo other-repo &&
+	git worktree add --detach side &&
+	cat .git/worktrees/side/gitdir >expect &&
+	cp -R side other-repo/side &&
+	test_must_fail git -C other-repo worktree repair side >out 2>err &&
+	test_cmp expect .git/worktrees/side/gitdir &&
+	test_must_be_empty out &&
+	test_grep ".git file is not for a linked worktree" err
+'
+
 test_expect_success 'repo not found; .git file broken' '
 	test_when_finished "rm -rf orig moved && git worktree prune" &&
 	git worktree add --detach orig &&

worktree.c +16 -4

index 879941a977..86094307b1 100644
@@ -640,7 +640,11 @@ int other_head_refs(struct repository *repo,
 static const char *get_worktree_id(const char *dotgit_contents)
 {
 	const char *slash = find_last_dir_sep(dotgit_contents);
-	if (!slash)
+	const char *prefix = "/worktrees";
+	int prefixlen = strlen(prefix);
+	if (!slash ||
+	    slash - dotgit_contents < prefixlen ||
+	    strncmp(slash - prefixlen, prefix, prefixlen))
 		return "";
 	return slash + 1;
 }

@@ -692,8 +696,10 @@ static void repair_gitfile(struct worktree *wt,
 	if (err == READ_GITFILE_ERR_NOT_A_FILE ||
 		err == READ_GITFILE_ERR_IS_A_DIR)
 		fn(1, wt->path, _(".git is not a file"), cb_data);
-	else if (err || !is_git_directory(backlink.buf))
+	else if (err)
 		repair = _(".git file broken");
+	else if (strcmp(get_worktree_id(dotgit_contents), wt->id))
+		fn(1, wt->path, _("unrelated .git file"), cb_data);
 	else if (fspathcmp(backlink.buf, repo.buf))
 		repair = _(".git file incorrect");
 	else if (use_relative_paths == is_absolute_path(dotgit_contents))

@@ -815,7 +821,7 @@ static ssize_t infer_backlink(struct repository *repo,
 	if (!*id)
 		goto error;
 	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
-	if (!is_directory(inferred->buf))
+	if (!is_git_directory(inferred->buf))
 		goto error;
 
 	return inferred->len;

@@ -888,6 +894,12 @@ void repair_worktree_at_path(struct repository *repo,
 		   cb_data);
 		goto done;
 	}
+	if (!inferred_backlink.len) {
+		fn(1, dotgit.buf,
+		   _("unable to locate repository; .git file is not for a linked worktree"),
+		   cb_data);
+		goto done;
+	}
 
 	/*
 	 * If we got this far, either the worktree's .git file pointed at a

@@ -905,7 +917,7 @@ void repair_worktree_at_path(struct repository *repo,
 	 * in the *original* repository, not in the "copy" repository).
 	 * Therefore, we prioritize inferred_backlink over backlink.
 	 */
-	if (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf))
+	if (fspathcmp(backlink.buf, inferred_backlink.buf))
 		strbuf_swap(&backlink, &inferred_backlink);
 
 	strbuf_addf(&gitdir, "%s/gitdir", backlink.buf);

Signature · 1 line

gitgitgadget

Junio C Hamano

2026-10-14 22:46 UTC #15 lore raw

In reply to Yoichi NAKAYAMA via GitGitGadget

Quoted 9 lines · Yoichi NAKAYAMA via GitGitGadget

'git worktree repair' does not sufficiently validate the cross-references between a linked working tree and its administrative data before repairing them. This can cause the repair to modify the wrong .git file or gitdir in certain situations.
v2 (0156021)
============

Quoted 8 lines

Split the first commit of v1 to make the changes easier to understand. Fold long lines.
* [1/5] align .git file handling with repair_gitfile()
* [2/5] centralize inferred backlink selection
* [3/5] use read_gitfile_raw() in infer_backlink()
* [4/5] reduce .git file reads
* [5/5] avoid breaking unrelated .git file and gitdir

Makes more sense to have this series as a logical progression of smaller pieces compared to v1.

What is "0156021" by the way?

Thanks.

Quoted 3 lines

 t/t2406-worktree-repair.sh |  33 +++++++++--
 worktree.c                 | 113 +++++++++++++++++++++----------------
 2 files changed, 91 insertions(+), 55 deletions(-)

Yoichi NAKAYAMA

2026-10-15 13:41 UTC #16 lore raw

Sorry, I sent the previous email in HTML. I'm resending it as plain text.

Quoted 21 lines · Junio C Hamano

Quoted 9 lines · Yoichi NAKAYAMA via GitGitGadget
'git worktree repair' does not sufficiently validate the cross-references between a linked working tree and its administrative data before repairing them. This can cause the repair to modify the wrong .git file or gitdir in certain situations.
v2 (0156021)
============
Quoted 8 lines
Split the first commit of v1 to make the changes easier to understand. Fold long lines.
* [1/5] align .git file handling with repair_gitfile()
* [2/5] centralize inferred backlink selection
* [3/5] use read_gitfile_raw() in infer_backlink()
* [4/5] reduce .git file reads
* [5/5] avoid breaking unrelated .git file and gitdir

Quoted 4 lines

Makes more sense to have this series as a logical progression of smaller pieces compared to v1.
What is "0156021" by the way?

It is the first 7 characters of the commit hash
in https://github.com/gitgitgadget/git/
(Creating a commit from an email changes the
commit hash, so the identifier was meaningless.)

Thanks,
--
Yoichi NAKAYAMA

Junio C Hamano

2026-10-15 18:03 UTC #13 lore raw

In reply to Yoichi NAKAYAMA via GitGitGadget

Quoted 6 lines · Yoichi NAKAYAMA via GitGitGadget

worktree.c +0 -0

index 879941a977..86094307b1 100644
@@ -640,7 +640,11 @@ int other_head_refs(struct repository *repo,
 static const char *get_worktree_id(const char *dotgit_contents)

worktree.c int other_head_refs(struct repository *repo, · line 649

Quoted 10 lines

worktree.c +5 -1

 {
 	const char *slash = find_last_dir_sep(dotgit_contents);
-	if (!slash)
+	const char *prefix = "/worktrees";
+	int prefixlen = strlen(prefix);
+	if (!slash ||
+	    slash - dotgit_contents < prefixlen ||
+	    strncmp(slash - prefixlen, prefix, prefixlen))
 		return "";
 	return slash + 1;

Just being curious, but slash is found by scanning dotgit_contents for either '/' or '\'. We make sure what comes before the slash is "/worktree".

Could there be situations where this code needs to accept

	"...\worktrees\my-worktree"

where slash points at the backslash before "my-worktree"? Because somebody wrote the pathname with backslash before the worktree name, it would be natural to expect another backslash before the "worktrees" constant, no? For that matter, would this code need to also accept

	".../worktrees//my-worktree"

where three are extra/duplicated slashes (or backslashes)? The normal code paths to create these directories and records the path to these directories should hopefully be using the canonical '/' without unnecessary duplicate slashes, but (1) existing use of find_last_dir_sep() might be a sign that we may have to deal with backslashes, and (2) since this is about reparing a broken set-up, it does not seem too far-fetched to attempt dealing with the above two sample input, hence this question.

Thanks.

Yoichi NAKAYAMA

2026-10-17 02:40 UTC #14 lore raw

Quoted 35 lines · Junio C Hamano

Quoted 6 lines · Yoichi NAKAYAMA via GitGitGadget
worktree.c +0 -0
index 879941a977..86094307b1 100644
@@ -640,7 +640,11 @@ int other_head_refs(struct repository *repo,
 static const char *get_worktree_id(const char *dotgit_contents)
worktree.c int other_head_refs(struct repository *repo, · line 649
Quoted 10 lines
worktree.c +5 -1
 {
      const char *slash = find_last_dir_sep(dotgit_contents);
-     if (!slash)
+     const char *prefix = "/worktrees";
+     int prefixlen = strlen(prefix);
+     if (!slash ||
+         slash - dotgit_contents < prefixlen ||
+         strncmp(slash - prefixlen, prefix, prefixlen))
              return "";
      return slash + 1;
Just being curious, but slash is found by scanning dotgit_contents for either '/' or '\'. We make sure what comes before the slash is "/worktree".
Could there be situations where this code needs to accept
        "...\worktrees\my-worktree"
where slash points at the backslash before "my-worktree"? Because somebody wrote the pathname with backslash before the worktree name, it would be natural to expect another backslash before the "worktrees" constant, no? For that matter, would this code need to also accept
        ".../worktrees//my-worktree"

Quoted 8 lines

where three are extra/duplicated slashes (or backslashes)? The normal code paths to create these directories and records the path to these directories should hopefully be using the canonical '/' without unnecessary duplicate slashes, but (1) existing use of find_last_dir_sep() might be a sign that we may have to deal with backslashes, and (2) since this is about reparing a broken set-up, it does not seem too far-fetched to attempt dealing with the above two sample input, hence this question.

While Git doesn't create a .git file with backslash separators, it can find the repository from such .git file on Windows. So I think 'worktree repair' should handle it.

I will also add handling for duplicated separators.

Thanks,

Signature · 1 line

Yoichi NAKAYAMA

Yoichi NAKAYAMA via GitGitGadget

author

2026-10-17 12:55 UTC #17 lore raw

v3 cover worktree repair: avoid breaking unrelated .git file and gitdir

In reply to Yoichi NAKAYAMA via GitGitGadget

v3
==

Make the separator check in get_worktree_id() more lenient.

v2
==

Split the first commit of v1 to make the changes easier to understand. Fold long lines.

v1
==

This series first refactors the code to read the .git file once and extract the worktree ID, then uses that information to validate the repair target before modifying the cross-references.

* [1/2] Refactor the code without changing functionality before making the fix
* [2/2] Validate the worktree ID and inferred gitdir path before repairing

 t/t2406-worktree-repair.sh |  57 ++++++++++++--
 worktree.c                 | 153 ++++++++++++++++++++++++-------------
 2 files changed, 152 insertions(+), 58 deletions(-)

base-commit: c51408889c3b372814bc20b9431c3b74d0d268eb
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2225%2Fyoichi%2Fworktree-repair-keep-unrelated-gitfile-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2225/yoichi/worktree-repair-keep-unrelated-gitfile-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2225

Range-diff vs v2:

 1:  21b2878aa5 = 1:  e336afa029 worktree repair: align .git file handling with repair_gitfile()
 2:  30ac7acad3 = 2:  86021f31af worktree repair: centralize inferred backlink selection
 3:  2cc7b817ee = 3:  7087e829bb worktree repair: use read_gitfile_raw() in infer_backlink()
 4:  8c80b9ae05 = 4:  e35285a4ea worktree repair: reduce .git file reads
 5:  015602168c ! 5:  f1d5c7b5f7 worktree repair: avoid breaking unrelated .git file and gitdir
     @@ t/t2406-worktree-repair.sh: test_expect_success 'repair .git file from bare.git'
      +	test_cmp expect corrupt/.git &&
      +	test_grep "unrelated .git file" err
      +'
     ++
     ++test_expect_success 'repair moved worktree whose .git file has extra separators' '
     ++	test_when_finished "rm -rf side sidemoved && git worktree prune" &&
     ++	git worktree add --detach side &&
     ++	mv side sidemoved &&
     ++	cat sidemoved/.git >expect &&
     ++	sed s,/,//,g sidemoved/.git | sed s,$,//, >sidemoved/.newgit &&
     ++	mv sidemoved/.newgit sidemoved/.git &&
     ++	git worktree repair sidemoved 2>err &&
     ++	test_cmp expect sidemoved/.git &&
     ++	test_grep "gitdir incorrect" err
     ++'
     ++
     ++test_expect_success WINDOWS 'repair moved worktree whose .git file uses backslash' '
     ++	test_when_finished "rm -rf side sidemoved && git worktree prune" &&
     ++	git worktree add --detach side &&
     ++	mv side sidemoved &&
     ++	cat sidemoved/.git >expect &&
     ++	sed s,\/,\\\\,g sidemoved/.git >sidemoved/.newgit &&
     ++	mv sidemoved/.newgit sidemoved/.git &&
     ++	git worktree repair sidemoved 2>err &&
     ++	test_cmp expect sidemoved/.git &&
     ++	test_grep "gitdir incorrect" err
     ++'
      +
       test_expect_success 'invalid worktree path' '
       	test_must_fail git worktree repair /notvalid >out 2>err &&
     @@ t/t2406-worktree-repair.sh: test_expect_success 'repo not found; .git not refere
      
       ## worktree.c ##
      @@ worktree.c: int other_head_refs(struct repository *repo,
     - static const char *get_worktree_id(const char *dotgit_contents)
     + 	return ret;
     + }
     + 
     +-static const char *get_worktree_id(const char *dotgit_contents)
     ++static int get_worktree_id(struct strbuf *worktree_id,
     ++			   const char *dotgit_contents)
       {
     - 	const char *slash = find_last_dir_sep(dotgit_contents);
     +-	const char *slash = find_last_dir_sep(dotgit_contents);
      -	if (!slash)
     -+	const char *prefix = "/worktrees";
     +-		return "";
     +-	return slash + 1;
     ++	const char *id_end;
     ++	const char *id;
     ++	const char *prefix_end;
     ++	const char *prefix = "worktrees";
      +	int prefixlen = strlen(prefix);
     -+	if (!slash ||
     -+	    slash - dotgit_contents < prefixlen ||
     -+	    strncmp(slash - prefixlen, prefix, prefixlen))
     - 		return "";
     - 	return slash + 1;
     ++
     ++	id_end = dotgit_contents + strlen(dotgit_contents);
     ++
     ++	/* skip trailing separators */
     ++	while (dotgit_contents < id_end && is_dir_sep(*(id_end - 1)))
     ++		id_end--;
     ++	if (id_end == dotgit_contents)
     ++		return 1;
     ++
     ++	id = id_end;
     ++
     ++	/* find the last separator */
     ++	while (dotgit_contents < id && !is_dir_sep(*(id - 1)))
     ++		id--;
     ++	if (id == dotgit_contents || id == id_end)
     ++		return 1;
     ++
     ++	prefix_end = id - 1;
     ++
     ++	/* skip repeated separators */
     ++	while (dotgit_contents < prefix_end && is_dir_sep(*(prefix_end - 1)))
     ++		prefix_end--;
     ++
     ++	/* check for "/worktrees" */
     ++	if (prefix_end - dotgit_contents < prefixlen + 1 ||
     ++	    strncmp(prefix_end - prefixlen, prefix, prefixlen) ||
     ++	    !is_dir_sep(*(prefix_end - prefixlen - 1)))
     ++		return 1;
     ++
     ++	strbuf_add(worktree_id, id, id_end - id);
     ++	return 0;
       }
     + 
     + /*
     +@@ worktree.c: static void repair_gitfile(struct worktree *wt,
     + 	struct strbuf repo = STRBUF_INIT;
     + 	struct strbuf backlink = STRBUF_INIT;
     + 	struct strbuf contents = STRBUF_INIT;
     ++	struct strbuf worktree_id = STRBUF_INIT;
     + 	const char *dotgit_contents = NULL;
     + 	const char *repair = NULL;
     + 	char *path = NULL;
      @@ worktree.c: static void repair_gitfile(struct worktree *wt,
       	if (err == READ_GITFILE_ERR_NOT_A_FILE ||
       		err == READ_GITFILE_ERR_IS_A_DIR)
     @@ worktree.c: static void repair_gitfile(struct worktree *wt,
      -	else if (err || !is_git_directory(backlink.buf))
      +	else if (err)
       		repair = _(".git file broken");
     -+	else if (strcmp(get_worktree_id(dotgit_contents), wt->id))
     ++	else if (get_worktree_id(&worktree_id, dotgit_contents) ||
     ++		 strcmp(worktree_id.buf, wt->id))
      +		fn(1, wt->path, _("unrelated .git file"), cb_data);
       	else if (fspathcmp(backlink.buf, repo.buf))
       		repair = _(".git file incorrect");
       	else if (use_relative_paths == is_absolute_path(dotgit_contents))
     +@@ worktree.c: done:
     + 	strbuf_release(&gitdir);
     + 	strbuf_release(&backlink);
     + 	strbuf_release(&contents);
     ++	strbuf_release(&worktree_id);
     + }
     + 
     + static void repair_noop(int iserr UNUSED,
      @@ worktree.c: static ssize_t infer_backlink(struct repository *repo,
     - 	if (!*id)
     + 			      const char *dotgit_contents,
     + 			      struct strbuf *inferred)
     + {
     +-	const char *id;
     ++	struct strbuf worktree_id = STRBUF_INIT;
     + 
     +-	id = get_worktree_id(dotgit_contents);
     +-	if (!*id)
     ++	if (get_worktree_id(&worktree_id, dotgit_contents))
       		goto error;
     - 	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
     +-	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
      -	if (!is_directory(inferred->buf))
     ++	repo_common_path_replace(repo, inferred,
     ++				 "worktrees/%s", worktree_id.buf);
      +	if (!is_git_directory(inferred->buf))
       		goto error;
       
     ++	strbuf_release(&worktree_id);
       	return inferred->len;
     + error:
     + 	strbuf_reset(inferred); /* clear invalid path */
     ++	strbuf_release(&worktree_id);
     + 	return -1;
     + }
     + 
      @@ worktree.c: void repair_worktree_at_path(struct repository *repo,
       		   cb_data);
       		goto done;

Signature · 1 line

gitgitgadget

Yoichi NAKAYAMA via GitGitGadget

author

2026-10-17 12:55 UTC #18 lore raw

PATCH v3 1/5 worktree repair: align .git file handling with repair_gitfile()

From: Yoichi NAKAYAMA <[email protected]>

A subsequent commit will update infer_backlink() to use read_gitfile_raw() as well.

Signed-off-by Yoichi NAKAYAMA <[email protected]>

---
 worktree.c | 40 ++++++++++++++++++++++++++++------------
 1 file changed, 28 insertions(+), 12 deletions(-)

worktree.c +28 ~~-12~~

index 8cb8637b18..4492ca835d 100644
@@ -840,7 +840,7 @@ void repair_worktree_at_path(struct repository *repo,
 	struct strbuf inferred_backlink = STRBUF_INIT;
 	struct strbuf gitdir = STRBUF_INIT;
 	struct strbuf olddotgit = STRBUF_INIT;
-	char *dotgit_contents = NULL;
+	struct strbuf contents = STRBUF_INIT;
 	const char *repair = NULL;
 	int err;

@@ -858,14 +858,26 @@ void repair_worktree_at_path(struct repository *repo,
 
 	infer_backlink(repo, dotgit.buf, &inferred_backlink);
 	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
-	dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));
-	if (dotgit_contents) {
-		strbuf_addstr(&backlink, dotgit_contents);
-	} else if (err == READ_GITFILE_ERR_NOT_A_FILE ||
-			err == READ_GITFILE_ERR_IS_A_DIR) {
-		fn(1, dotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
+	err = read_gitfile_raw(&contents, dotgit.buf);
+	if (!err) {
+		const char *dotgit_contents = contents.buf;
+		if (is_absolute_path(dotgit_contents)) {
+			strbuf_addstr(&backlink, dotgit_contents);
+		} else {
+			strbuf_addbuf(&backlink, &dotgit);
+			strbuf_strip_suffix(&backlink, ".git");
+			strbuf_addstr(&backlink, dotgit_contents);
+			strbuf_realpath_forgiving(&backlink, backlink.buf, 0);
+		}
+	}
+	if (err == READ_GITFILE_ERR_NOT_A_FILE ||
+	    err == READ_GITFILE_ERR_IS_A_DIR) {
+		fn(1, dotgit.buf,
+		   _("unable to locate repository; .git is not a file"),
+		   cb_data);
 		goto done;
-	} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {
+	} else if (!err && !is_git_directory(backlink.buf)) {
+		strbuf_release(&backlink);
 		if (inferred_backlink.len) {
 			/*
 			 * Worktree's .git file does not point at a repository

@@ -876,11 +888,15 @@ void repair_worktree_at_path(struct repository *repo,
 			 */
 			strbuf_swap(&backlink, &inferred_backlink);
 		} else {
-			fn(1, dotgit.buf, _("unable to locate repository; .git file does not reference a repository"), cb_data);
+			fn(1, dotgit.buf,
+			   _("unable to locate repository; .git file does not reference a repository"),
+			   cb_data);
 			goto done;
 		}
-	} else {
-		fn(1, dotgit.buf, _("unable to locate repository; .git file broken"), cb_data);
+	} else if (err) {
+		fn(1, dotgit.buf,
+		   _("unable to locate repository; .git file broken"),
+		   cb_data);
 		goto done;
 	}

@@ -926,12 +942,12 @@ void repair_worktree_at_path(struct repository *repo,
 					     gitdir.buf, use_relative_paths);
 	}
 done:
-	free(dotgit_contents);
 	strbuf_release(&olddotgit);
 	strbuf_release(&backlink);
 	strbuf_release(&inferred_backlink);
 	strbuf_release(&gitdir);
 	strbuf_release(&dotgit);
+	strbuf_release(&contents);
 }
 
 int should_prune_worktree(struct repository *repo,

Signature · 1 line

gitgitgadget

Yoichi NAKAYAMA via GitGitGadget

author

2026-10-17 12:55 UTC #19 lore raw

PATCH v3 2/5 worktree repair: centralize inferred backlink selection

In reply to Yoichi NAKAYAMA via GitGitGadget

From: Yoichi NAKAYAMA <[email protected]>

Reorder the processing steps to centralize the logic for preferring the inferred_backlink.

Signed-off-by Yoichi NAKAYAMA <[email protected]>

---
 worktree.c | 69 +++++++++++++++++++++++-------------------------------
 1 file changed, 29 insertions(+), 40 deletions(-)

worktree.c +29 ~~-40~~

index 4492ca835d..f8e9850f6f 100644
@@ -841,6 +841,7 @@ void repair_worktree_at_path(struct repository *repo,
 	struct strbuf gitdir = STRBUF_INIT;
 	struct strbuf olddotgit = STRBUF_INIT;
 	struct strbuf contents = STRBUF_INIT;
+	const char *dotgit_contents = NULL;
 	const char *repair = NULL;
 	int err;

@@ -856,43 +857,13 @@ void repair_worktree_at_path(struct repository *repo,
 		goto done;
 	}
 
-	infer_backlink(repo, dotgit.buf, &inferred_backlink);
-	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
 	err = read_gitfile_raw(&contents, dotgit.buf);
-	if (!err) {
-		const char *dotgit_contents = contents.buf;
-		if (is_absolute_path(dotgit_contents)) {
-			strbuf_addstr(&backlink, dotgit_contents);
-		} else {
-			strbuf_addbuf(&backlink, &dotgit);
-			strbuf_strip_suffix(&backlink, ".git");
-			strbuf_addstr(&backlink, dotgit_contents);
-			strbuf_realpath_forgiving(&backlink, backlink.buf, 0);
-		}
-	}
 	if (err == READ_GITFILE_ERR_NOT_A_FILE ||
 	    err == READ_GITFILE_ERR_IS_A_DIR) {
 		fn(1, dotgit.buf,
 		   _("unable to locate repository; .git is not a file"),
 		   cb_data);
 		goto done;
-	} else if (!err && !is_git_directory(backlink.buf)) {
-		strbuf_release(&backlink);
-		if (inferred_backlink.len) {
-			/*
-			 * Worktree's .git file does not point at a repository
-			 * but we found a .git/worktrees/<id> in this
-			 * repository with the same <id> as recorded in the
-			 * worktree's .git file so make the worktree point at
-			 * the discovered .git/worktrees/<id>.
-			 */
-			strbuf_swap(&backlink, &inferred_backlink);
-		} else {
-			fn(1, dotgit.buf,
-			   _("unable to locate repository; .git file does not reference a repository"),
-			   cb_data);
-			goto done;
-		}
 	} else if (err) {
 		fn(1, dotgit.buf,
 		   _("unable to locate repository; .git file broken"),

@@ -900,23 +871,41 @@ void repair_worktree_at_path(struct repository *repo,
 		goto done;
 	}
 
+	dotgit_contents = contents.buf;
+	infer_backlink(repo, dotgit.buf, &inferred_backlink);
+	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
+
+	if (is_absolute_path(dotgit_contents)) {
+		strbuf_addstr(&backlink, dotgit_contents);
+	} else {
+		strbuf_addbuf(&backlink, &dotgit);
+		strbuf_strip_suffix(&backlink, ".git");
+		strbuf_addstr(&backlink, dotgit_contents);
+		strbuf_realpath_forgiving(&backlink, backlink.buf, 0);
+	}
+
+	if (!is_git_directory(backlink.buf) && !inferred_backlink.len) {
+		fn(1, dotgit.buf,
+		   _("unable to locate repository; .git file does not reference a repository"),
+		   cb_data);
+		goto done;
+	}
+
 	/*
 	 * If we got this far, either the worktree's .git file pointed at a
-	 * valid repository (i.e. read_gitfile_gently() returned success) or
+	 * valid repository (i.e. is_git_directory() returned true) or
 	 * the .git file did not point at a repository but we were able to
 	 * infer a suitable new value for the .git file by locating a
 	 * .git/worktrees/<id> in *this* repository corresponding to the <id>
 	 * recorded in the worktree's .git file.
 	 *
-	 * However, if, at this point, inferred_backlink is non-NULL (i.e. we
-	 * found a suitable .git/worktrees/<id> in *this* repository) *and* the
-	 * worktree's .git file points at a valid repository *and* those two
-	 * paths differ, then that indicates that the user probably *copied*
-	 * the main and linked worktrees to a new location as a unit rather
-	 * than *moving* them. Thus, the copied worktree's .git file actually
-	 * points at the .git/worktrees/<id> in the *original* repository, not
-	 * in the "copy" repository. In this case, point the "copy" worktree's
-	 * .git file at the "copy" repository.
+	 * Even if the worktree's .git file pointed at a valid repository,
+	 * it doesn't always mean that the backlink is correct. For example,
+	 * the user might have *copied* the main and linked worktrees to a
+	 * new location as a unit rather than *moving* them (the copied
+	 * worktree's .git file actually points at the .git/worktrees/<id>
+	 * in the *original* repository, not in the "copy" repository).
+	 * Therefore, we prioritize inferred_backlink over backlink.
 	 */
 	if (inferred_backlink.len && fspathcmp(backlink.buf, inferred_backlink.buf))
 		strbuf_swap(&backlink, &inferred_backlink);

Signature · 1 line

gitgitgadget

Yoichi NAKAYAMA via GitGitGadget

author

2026-10-17 12:55 UTC #20 lore raw

PATCH v3 3/5 worktree repair: use read_gitfile_raw() in infer_backlink()

In reply to Yoichi NAKAYAMA via GitGitGadget

From: Yoichi NAKAYAMA <[email protected]>

Extract get_worktree_id() for use in a subsequent commit.

Signed-off-by Yoichi NAKAYAMA <[email protected]>

---
 worktree.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

worktree.c +15 ~~-10~~

index f8e9850f6f..3627dbc044 100644
@@ -637,6 +637,14 @@ int other_head_refs(struct repository *repo,
 	return ret;
 }
 
+static const char *get_worktree_id(const char *dotgit_contents)
+{
+	const char *slash = find_last_dir_sep(dotgit_contents);
+	if (!slash)
+		return "";
+	return slash + 1;
+}
+
 /*
  * Repair worktree's /path/to/worktree/.git file if missing, corrupt, or not
  * pointing at <repo>/worktrees/<id>.

@@ -801,27 +809,24 @@ static ssize_t infer_backlink(struct repository *repo,
 			      const char *gitfile,
 			      struct strbuf *inferred)
 {
-	struct strbuf actual = STRBUF_INIT;
+	struct strbuf contents = STRBUF_INIT;
 	const char *id;
+	int err;
 
-	if (strbuf_read_file(&actual, gitfile, 0) < 0)
+	err = read_gitfile_raw(&contents, gitfile);
+	if (err)
 		goto error;
-	if (!starts_with(actual.buf, "gitdir:"))
-		goto error;
-	if (!(id = find_last_dir_sep(actual.buf)))
-		goto error;
-	strbuf_trim(&actual);
-	id++; /* advance past '/' to point at <id> */
+	id = get_worktree_id(contents.buf);
 	if (!*id)
 		goto error;
 	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
 	if (!is_directory(inferred->buf))
 		goto error;
 
-	strbuf_release(&actual);
+	strbuf_release(&contents);
 	return inferred->len;
 error:
-	strbuf_release(&actual);
+	strbuf_release(&contents);
 	strbuf_reset(inferred); /* clear invalid path */
 	return -1;
 }

Signature · 1 line

gitgitgadget

Yoichi NAKAYAMA via GitGitGadget

author

2026-10-17 12:55 UTC #21 lore raw

PATCH v3 4/5 worktree repair: reduce .git file reads

In reply to Yoichi NAKAYAMA via GitGitGadget

From: Yoichi NAKAYAMA <[email protected]>

Pass the loaded contents of the .git file to infer_backlink().

Signed-off-by Yoichi NAKAYAMA <[email protected]>

---
 worktree.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

worktree.c +3 ~~-10~~

index 3627dbc044..879941a977 100644
@@ -806,27 +806,20 @@ static int is_main_worktree_path(struct repository *repo, const char *path)
  * Returns -1 on failure and strbuf.len on success.
  */
 static ssize_t infer_backlink(struct repository *repo,
-			      const char *gitfile,
+			      const char *dotgit_contents,
 			      struct strbuf *inferred)
 {
-	struct strbuf contents = STRBUF_INIT;
 	const char *id;
-	int err;
 
-	err = read_gitfile_raw(&contents, gitfile);
-	if (err)
-		goto error;
-	id = get_worktree_id(contents.buf);
+	id = get_worktree_id(dotgit_contents);
 	if (!*id)
 		goto error;
 	repo_common_path_replace(repo, inferred, "worktrees/%s", id);
 	if (!is_directory(inferred->buf))
 		goto error;
 
-	strbuf_release(&contents);
 	return inferred->len;
 error:
-	strbuf_release(&contents);
 	strbuf_reset(inferred); /* clear invalid path */
 	return -1;
 }

@@ -877,7 +870,7 @@ void repair_worktree_at_path(struct repository *repo,
 	}
 
 	dotgit_contents = contents.buf;
-	infer_backlink(repo, dotgit.buf, &inferred_backlink);
+	infer_backlink(repo, dotgit_contents, &inferred_backlink);
 	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
 
 	if (is_absolute_path(dotgit_contents)) {

Signature · 1 line

gitgitgadget

Yoichi NAKAYAMA via GitGitGadget

author

2026-10-17 12:55 UTC #22 lore raw

PATCH v3 5/5 worktree repair: avoid breaking unrelated .git file and gitdir

In reply to Yoichi NAKAYAMA via GitGitGadget

From: Yoichi NAKAYAMA <[email protected]>

Resolve these issues by validating the worktree ID and stopping the repair when the ID does not match or the gitdir path cannot be inferred.

Signed-off-by Yoichi NAKAYAMA <[email protected]>

---
 t/t2406-worktree-repair.sh | 57 +++++++++++++++++++++++++++----
 worktree.c                 | 70 +++++++++++++++++++++++++++++++-------
 2 files changed, 109 insertions(+), 18 deletions(-)

t/t2406-worktree-repair.sh +51 -6

index d4e53d492b..fa26565d0a 100755
@@ -56,15 +56,12 @@ test_expect_success 'repair missing .git file' '
 '
 
 test_expect_success 'repair bogus .git file' '
-	test_corrupt_gitfile "echo \"gitdir: /nowhere\" >corrupt/.git" \
+	test_corrupt_gitfile "echo \"contents not started with gitdir:\" >corrupt/.git" \
 		".git file broken"
 '
 
-test_expect_success 'repair incorrect .git file' '
-	test_when_finished "rm -rf other && git worktree prune" &&
-	test_create_repo other &&
-	other=$(git -C other rev-parse --absolute-git-dir) &&
-	test_corrupt_gitfile "echo \"gitdir: $other\" >corrupt/.git" \
+test_expect_success 'repair unlinked .git file' '
+	test_corrupt_gitfile "echo \"gitdir: /nowhere/worktrees/corrupt\" >corrupt/.git" \
 		".git file incorrect"
 '

@@ -89,6 +86,42 @@ test_expect_success 'repair .git file from bare.git' '
 	test_cmp expect actual
 '
 
+test_expect_success 'skip unrelated .git file' '
+	test_when_finished "rm -rf corrupt other && git worktree prune" &&
+	git worktree add --detach corrupt &&
+	rm -rf corrupt &&
+	git worktree add --detach other &&
+	mv other corrupt &&
+	cat corrupt/.git >expect &&
+	test_must_fail git worktree repair 2>err &&
+	test_cmp expect corrupt/.git &&
+	test_grep "unrelated .git file" err
+'
+
+test_expect_success 'repair moved worktree whose .git file has extra separators' '
+	test_when_finished "rm -rf side sidemoved && git worktree prune" &&
+	git worktree add --detach side &&
+	mv side sidemoved &&
+	cat sidemoved/.git >expect &&
+	sed s,/,//,g sidemoved/.git | sed s,$,//, >sidemoved/.newgit &&
+	mv sidemoved/.newgit sidemoved/.git &&
+	git worktree repair sidemoved 2>err &&
+	test_cmp expect sidemoved/.git &&
+	test_grep "gitdir incorrect" err
+'
+
+test_expect_success WINDOWS 'repair moved worktree whose .git file uses backslash' '
+	test_when_finished "rm -rf side sidemoved && git worktree prune" &&
+	git worktree add --detach side &&
+	mv side sidemoved &&
+	cat sidemoved/.git >expect &&
+	sed s,\/,\\\\,g sidemoved/.git >sidemoved/.newgit &&
+	mv sidemoved/.newgit sidemoved/.git &&
+	git worktree repair sidemoved 2>err &&
+	test_cmp expect sidemoved/.git &&
+	test_grep "gitdir incorrect" err
+'
+
 test_expect_success 'invalid worktree path' '
 	test_must_fail git worktree repair /notvalid >out 2>err &&
 	test_must_be_empty out &&

@@ -113,6 +146,18 @@ test_expect_success 'repo not found; .git not referencing repo' '
 	test_grep ".git file does not reference a repository" err
 '
 
+test_expect_success 'repo not found; .git not for worktree' '
+	test_when_finished "rm -rf side other-repo && git worktree prune" &&
+	test_create_repo other-repo &&
+	git worktree add --detach side &&
+	cat .git/worktrees/side/gitdir >expect &&
+	cp -R side other-repo/side &&
+	test_must_fail git -C other-repo worktree repair side >out 2>err &&
+	test_cmp expect .git/worktrees/side/gitdir &&
+	test_must_be_empty out &&
+	test_grep ".git file is not for a linked worktree" err
+'
+
 test_expect_success 'repo not found; .git file broken' '
 	test_when_finished "rm -rf orig moved && git worktree prune" &&
 	git worktree add --detach orig &&

worktree.c +58 ~~-12~~

121 lines not on this page · raw message

Signature · 1 line

gitgitgadget