All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Yoichi NAKAYAMA via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Yoichi Nakayama <yoichi.nakayama@gmail.com>,
	Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>,
	Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>
Subject: [PATCH v3] worktree repair: detect relative path in .git file correctly
Date: Fri, 21 Aug 2026 20:36:26 +0000	[thread overview]
Message-ID: <pull.2205.v3.git.1787344586470.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2205.git.1786799480344.gitgitgadget@gmail.com>

From: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>

Given a state in which the cross-references between the worktree and
the repository (specifically worktree/id/gitdir in the main repository
and the .git file in the worktree) are recorded using absolute paths,
setting 'worktree.useRelativePaths=true' and running 'git worktree
repair' within the main worktree converts them to relative paths.

Conversely, given a state in which the cross-references are recorded
using relative paths, one would expect that setting
'worktree.useRelativePaths=false' and running 'git worktree repair'
would convert them to absolute paths. However, they remain as relative
paths.

This is because we incorrectly use read_gitfile_gently(), which always
returns an absolute path. To fix this, introduce read_gitfile_raw(),
which is almost identical to read_gitfile_gently(), but skips checking
the existence of the referenced repository and returns the path as-is
from the .git file.

Signed-off-by: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>
---
    worktree repair: detect relative path in .git file correctly

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2205%2Fyoichi%2Fworktree-repair-relative-path-handling-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2205/yoichi/worktree-repair-relative-path-handling-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/2205

Range-diff vs v2:

 1:  5bcf19ef50 ! 1:  1cd25e315e worktree repair: detect relative path in .git file correctly
     @@ Metadata
       ## Commit message ##
          worktree repair: detect relative path in .git file correctly
      
     -    Given a state where the cross references between the worktree and the
     -    repository (specifically worktree/id/gitdir in the main repository and
     -    the .git file in the worktree) are recorded using absolute paths,
     +    Given a state in which the cross-references between the worktree and
     +    the repository (specifically worktree/id/gitdir in the main repository
     +    and the .git file in the worktree) are recorded using absolute paths,
          setting 'worktree.useRelativePaths=true' and running 'git worktree
          repair' within the main worktree converts them to relative paths.
      
     -    On the other hand, given a state where the cross references are
     -    recorded using relative paths, one would expect (by symmetry) that
     -    setting 'worktree.useRelativePath=false' and running 'git worktree
     -    repair' would convert them to absolute paths. However, they remain as
     -    relative paths.
     +    Conversely, given a state in which the cross-references are recorded
     +    using relative paths, one would expect that setting
     +    'worktree.useRelativePaths=false' and running 'git worktree repair'
     +    would convert them to absolute paths. However, they remain as relative
     +    paths.
      
     -    This is because we wrongly use read_gitfile_gently() which always
     -    returns an absolute path. To fix this, introduce read_gitfile_raw()
     -    that is almost same as read_gitfile_gently(), but it skips existence
     -    check of the referenced repository and returns the unmodified path
     -    read from .git file.
     +    This is because we incorrectly use read_gitfile_gently(), which always
     +    returns an absolute path. To fix this, introduce read_gitfile_raw(),
     +    which is almost identical to read_gitfile_gently(), but skips checking
     +    the existence of the referenced repository and returns the path as-is
     +    from the .git file.
      
          Signed-off-by: Yoichi NAKAYAMA <yoichi.nakayama@gmail.com>
      
       ## setup.c ##
      @@ setup.c: void read_gitfile_error_die(int error_code, const char *path)
     +  * cases).
        */
       const char *read_gitfile_gently(const char *path, int *return_error_code)
     - {
     --	const int max_file_size = 1 << 20;  /* 1MB */
     - 	int error_code = 0;
     - 	char *buf = NULL;
     --	char *dir = NULL;
     - 	const char *slash;
     ++{
     ++	int error_code = 0;
     ++	const char *slash;
     ++	struct strbuf contents = STRBUF_INIT;
      +	static struct strbuf realpath = STRBUF_INIT;
      +
     -+	buf = xstrdup_or_null(read_gitfile_raw(path, &error_code));
     ++	error_code = read_gitfile_raw(&contents, path);
      +	if (error_code)
      +		goto cleanup_return;
      +
     -+	if (!is_absolute_path(buf) && (slash = strrchr(path, '/'))) {
     ++	if (!is_absolute_path(contents.buf) && (slash = strrchr(path, '/'))) {
      +		size_t pathlen = slash+1 - path;
     -+		char *dir = xstrfmt("%.*s%.*s", (int)pathlen, path,
     -+				    (int)strlen(buf), buf);
     -+		free(buf);
     -+		buf = dir;
     ++		char *dir = xstrfmt("%.*s%s", (int)pathlen, path, contents.buf);
     ++		strbuf_reset(&contents);
     ++		strbuf_addstr(&contents, dir);
     ++		free(dir);
      +	}
     -+	if (!is_git_directory(buf)) {
     ++	if (!is_git_directory(contents.buf)) {
      +		error_code = READ_GITFILE_ERR_NOT_A_REPO;
      +		goto cleanup_return;
      +	}
      +
     -+	strbuf_realpath(&realpath, buf, 1);
     ++	strbuf_realpath(&realpath, contents.buf, 1);
      +
      +cleanup_return:
      +	if (return_error_code)
     @@ setup.c: void read_gitfile_error_die(int error_code, const char *path)
      +	else if (error_code)
      +		read_gitfile_error_die(error_code, path);
      +
     -+	free(buf);
     ++	strbuf_release(&contents);
      +	return error_code ? NULL : realpath.buf;
      +}
      +
     -+const char *read_gitfile_raw(const char *path, int *return_error_code)
     -+{
     -+	const int max_file_size = 1 << 20;  /* 1MB */
     -+	int error_code = 0;
     -+	char *buf = NULL;
     ++int read_gitfile_raw(struct strbuf *contents, const char *path)
     + {
     + 	const int max_file_size = 1 << 20;  /* 1MB */
     + 	int error_code = 0;
     + 	char *buf = NULL;
     +-	char *dir = NULL;
     +-	const char *slash;
       	struct stat st;
       	int fd;
       	ssize_t len;
      -	static struct strbuf realpath = STRBUF_INIT;
     -+	static struct strbuf contents = STRBUF_INIT;
       
       	if (stat(path, &st)) {
       		if (errno == ENOENT || errno == ENOTDIR)
     @@ setup.c: const char *read_gitfile_gently(const char *path, int *return_error_cod
      -
      -	strbuf_realpath(&realpath, dir, 1);
      -	path = realpath.buf;
     -+	strbuf_reset(&contents);
     -+	strbuf_add(&contents, buf+8, len-8);
     ++	strbuf_add(contents, buf+8, len-8);
       
       cleanup_return:
      -	if (return_error_code)
     @@ setup.c: const char *read_gitfile_gently(const char *path, int *return_error_cod
      -	else if (error_code)
      -		read_gitfile_error_die(error_code, path);
      -
     -+	*return_error_code = error_code;
       	free(buf);
      -	return error_code ? NULL : path;
     -+	return error_code ? NULL : contents.buf;
     ++	return error_code;
       }
       
       static void apply_gitdir_and_environment(struct repository *repo, const char *path)
     @@ setup.h: int is_nonbare_repository_dir(struct strbuf *path);
       #define READ_GITFILE_ERR_IS_A_DIR 10
       void read_gitfile_error_die(int error_code, const char *path);
       const char *read_gitfile_gently(const char *path, int *return_error_code);
     -+const char *read_gitfile_raw(const char *path, int *return_error_code);
     ++int read_gitfile_raw(struct strbuf *contents, const char *path);
       #define read_gitfile(path) read_gitfile_gently((path), NULL)
       const char *resolve_gitdir_gently(const char *suspect, int *return_error_code);
       #define resolve_gitdir(path) resolve_gitdir_gently((path), NULL)
     @@ t/t2406-worktree-repair.sh: test_expect_success 'repair worktree with relative p
       test_done
      
       ## worktree.c ##
     +@@ worktree.c: static void repair_gitfile(struct worktree *wt,
     + 	struct strbuf gitdir = STRBUF_INIT;
     + 	struct strbuf repo = STRBUF_INIT;
     + 	struct strbuf backlink = STRBUF_INIT;
     +-	char *dotgit_contents = NULL;
     ++	struct strbuf contents = STRBUF_INIT;
     ++	const char *dotgit_contents = NULL;
     + 	const char *repair = NULL;
     + 	char *path = NULL;
     + 	int err;
      @@ worktree.c: static void repair_gitfile(struct worktree *wt,
       	strbuf_realpath(&repo, path, 1);
       	strbuf_addf(&dotgit, "%s/.git", wt->path);
       	strbuf_addf(&gitdir, "%s/gitdir", repo.buf);
      -	dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));
     -+	dotgit_contents = xstrdup_or_null(read_gitfile_raw(dotgit.buf, &err));
     ++	err = read_gitfile_raw(&contents, dotgit.buf);
     ++	if (!err)
     ++		dotgit_contents = contents.buf;
       
       	if (dotgit_contents) {
       		if (is_absolute_path(dotgit_contents)) {
     @@ worktree.c: static void repair_gitfile(struct worktree *wt,
       		repair = _(".git file broken");
       	else if (fspathcmp(backlink.buf, repo.buf))
       		repair = _(".git file incorrect");
     +@@ worktree.c: static void repair_gitfile(struct worktree *wt,
     + 	}
     + 
     + done:
     +-	free(dotgit_contents);
     + 	free(path);
     + 	strbuf_release(&repo);
     + 	strbuf_release(&dotgit);
     + 	strbuf_release(&gitdir);
     + 	strbuf_release(&backlink);
     ++	strbuf_release(&contents);
     + }
     + 
     + static void repair_noop(int iserr UNUSED,
      @@ worktree.c: void repair_worktree_at_path(struct repository *repo,
       	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
       	dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));


 setup.c                    | 63 ++++++++++++++++++++++----------------
 setup.h                    |  1 +
 t/t2406-worktree-repair.sh | 54 ++++++++++++++++++++++++--------
 worktree.c                 | 20 +++++-------
 4 files changed, 88 insertions(+), 50 deletions(-)

diff --git a/setup.c b/setup.c
index 95909e9603..9041827336 100644
--- a/setup.c
+++ b/setup.c
@@ -962,16 +962,48 @@ void read_gitfile_error_die(int error_code, const char *path)
  * cases).
  */
 const char *read_gitfile_gently(const char *path, int *return_error_code)
+{
+	int error_code = 0;
+	const char *slash;
+	struct strbuf contents = STRBUF_INIT;
+	static struct strbuf realpath = STRBUF_INIT;
+
+	error_code = read_gitfile_raw(&contents, path);
+	if (error_code)
+		goto cleanup_return;
+
+	if (!is_absolute_path(contents.buf) && (slash = strrchr(path, '/'))) {
+		size_t pathlen = slash+1 - path;
+		char *dir = xstrfmt("%.*s%s", (int)pathlen, path, contents.buf);
+		strbuf_reset(&contents);
+		strbuf_addstr(&contents, dir);
+		free(dir);
+	}
+	if (!is_git_directory(contents.buf)) {
+		error_code = READ_GITFILE_ERR_NOT_A_REPO;
+		goto cleanup_return;
+	}
+
+	strbuf_realpath(&realpath, contents.buf, 1);
+
+cleanup_return:
+	if (return_error_code)
+		*return_error_code = error_code;
+	else if (error_code)
+		read_gitfile_error_die(error_code, path);
+
+	strbuf_release(&contents);
+	return error_code ? NULL : realpath.buf;
+}
+
+int read_gitfile_raw(struct strbuf *contents, const char *path)
 {
 	const int max_file_size = 1 << 20;  /* 1MB */
 	int error_code = 0;
 	char *buf = NULL;
-	char *dir = NULL;
-	const char *slash;
 	struct stat st;
 	int fd;
 	ssize_t len;
-	static struct strbuf realpath = STRBUF_INIT;
 
 	if (stat(path, &st)) {
 		if (errno == ENOENT || errno == ENOTDIR)
@@ -1014,32 +1046,11 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
 		error_code = READ_GITFILE_ERR_NO_PATH;
 		goto cleanup_return;
 	}
-	buf[len] = '\0';
-	dir = buf + 8;
-
-	if (!is_absolute_path(dir) && (slash = strrchr(path, '/'))) {
-		size_t pathlen = slash+1 - path;
-		dir = xstrfmt("%.*s%.*s", (int)pathlen, path,
-			      (int)(len - 8), buf + 8);
-		free(buf);
-		buf = dir;
-	}
-	if (!is_git_directory(dir)) {
-		error_code = READ_GITFILE_ERR_NOT_A_REPO;
-		goto cleanup_return;
-	}
-
-	strbuf_realpath(&realpath, dir, 1);
-	path = realpath.buf;
+	strbuf_add(contents, buf+8, len-8);
 
 cleanup_return:
-	if (return_error_code)
-		*return_error_code = error_code;
-	else if (error_code)
-		read_gitfile_error_die(error_code, path);
-
 	free(buf);
-	return error_code ? NULL : path;
+	return error_code;
 }
 
 static void apply_gitdir_and_environment(struct repository *repo, const char *path)
diff --git a/setup.h b/setup.h
index 654f10e059..e6e71bda3d 100644
--- a/setup.h
+++ b/setup.h
@@ -40,6 +40,7 @@ int is_nonbare_repository_dir(struct strbuf *path);
 #define READ_GITFILE_ERR_IS_A_DIR 10
 void read_gitfile_error_die(int error_code, const char *path);
 const char *read_gitfile_gently(const char *path, int *return_error_code);
+int read_gitfile_raw(struct strbuf *contents, const char *path);
 #define read_gitfile(path) read_gitfile_gently((path), NULL)
 const char *resolve_gitdir_gently(const char *suspect, int *return_error_code);
 #define resolve_gitdir(path) resolve_gitdir_gently((path), NULL)
diff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh
index f5f19b3169..d4e53d492b 100755
--- a/t/t2406-worktree-repair.sh
+++ b/t/t2406-worktree-repair.sh
@@ -228,30 +228,60 @@ test_expect_success 'repair worktree with relative path with missing gitfile' '
 	test_cmp expect wt/.git
 '
 
-test_expect_success 'repair absolute worktree to use relative paths' '
-	test_when_finished "rm -rf main side sidemoved" &&
+test_expect_success 'repair absolute to relative from side worktree' '
+	test_when_finished "rm -rf main side" &&
 	test_create_repo main &&
 	test_commit -C main init &&
 	git -C main worktree add --detach ../side &&
-	echo "../../../../sidemoved/.git" >expect-gitdir &&
+	echo "../../../../side/.git" >expect-gitdir &&
 	echo "gitdir: ../main/.git/worktrees/side" >expect-gitfile &&
-	mv side sidemoved &&
-	git -C main worktree repair --relative-paths ../sidemoved &&
+	git -C main worktree repair --relative-paths ../side 2>main/err &&
+	test_grep "gitdir absolute/relative path mismatch" main/err &&
 	test_cmp expect-gitdir main/.git/worktrees/side/gitdir &&
-	test_cmp expect-gitfile sidemoved/.git
+	test_cmp expect-gitfile side/.git
 '
 
-test_expect_success 'repair relative worktree to use absolute paths' '
-	test_when_finished "rm -rf main side sidemoved" &&
+test_expect_success 'repair relative to absolute from side worktree' '
+	test_when_finished "rm -rf main side" &&
 	test_create_repo main &&
 	test_commit -C main init &&
 	git -C main worktree add --relative-paths --detach ../side &&
-	echo "$(pwd)/sidemoved/.git" >expect-gitdir &&
+	echo "$(pwd)/side/.git" >expect-gitdir &&
 	echo "gitdir: $(pwd)/main/.git/worktrees/side" >expect-gitfile &&
-	mv side sidemoved &&
-	git -C main worktree repair ../sidemoved &&
+	git -C main worktree repair ../side 2>main/err &&
+	test_grep "gitdir absolute/relative path mismatch" main/err &&
 	test_cmp expect-gitdir main/.git/worktrees/side/gitdir &&
-	test_cmp expect-gitfile sidemoved/.git
+	test_cmp expect-gitfile side/.git
+'
+
+test_expect_success 'repair absolute to relative from main worktree' '
+	test_when_finished "rm -rf main side" &&
+	test_create_repo main &&
+	git -C main config worktree.useRelativePaths false &&
+	test_commit -C main init &&
+	git -C main worktree add --detach ../side &&
+	echo "../../../../side/.git" >expect-gitdir &&
+	echo "gitdir: ../main/.git/worktrees/side" >expect-gitfile &&
+	git -C main config worktree.useRelativePaths true &&
+	git -C main worktree repair 2>main/err &&
+	test_grep ".git file absolute/relative path mismatch" main/err &&
+	test_cmp expect-gitdir main/.git/worktrees/side/gitdir &&
+	test_cmp expect-gitfile side/.git
+'
+
+test_expect_success 'repair relative to absolute from main worktree' '
+	test_when_finished "rm -rf main side" &&
+	test_create_repo main &&
+	git -C main config worktree.useRelativePaths true &&
+	test_commit -C main init &&
+	git -C main worktree add --detach ../side &&
+	echo "$(pwd)/side/.git" >expect-gitdir &&
+	echo "gitdir: $(pwd)/main/.git/worktrees/side" >expect-gitfile &&
+	git -C main config worktree.useRelativePaths false &&
+	git -C main worktree repair 2>main/err &&
+	test_grep ".git file absolute/relative path mismatch" main/err &&
+	test_cmp expect-gitdir main/.git/worktrees/side/gitdir &&
+	test_cmp expect-gitfile side/.git
 '
 
 test_done
diff --git a/worktree.c b/worktree.c
index cbf95328a3..8cb8637b18 100644
--- a/worktree.c
+++ b/worktree.c
@@ -649,7 +649,8 @@ static void repair_gitfile(struct worktree *wt,
 	struct strbuf gitdir = STRBUF_INIT;
 	struct strbuf repo = STRBUF_INIT;
 	struct strbuf backlink = STRBUF_INIT;
-	char *dotgit_contents = NULL;
+	struct strbuf contents = STRBUF_INIT;
+	const char *dotgit_contents = NULL;
 	const char *repair = NULL;
 	char *path = NULL;
 	int err;
@@ -667,7 +668,9 @@ static void repair_gitfile(struct worktree *wt,
 	strbuf_realpath(&repo, path, 1);
 	strbuf_addf(&dotgit, "%s/.git", wt->path);
 	strbuf_addf(&gitdir, "%s/gitdir", repo.buf);
-	dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));
+	err = read_gitfile_raw(&contents, dotgit.buf);
+	if (!err)
+		dotgit_contents = contents.buf;
 
 	if (dotgit_contents) {
 		if (is_absolute_path(dotgit_contents)) {
@@ -681,7 +684,7 @@ 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)
+	else if (err || !is_git_directory(backlink.buf))
 		repair = _(".git file broken");
 	else if (fspathcmp(backlink.buf, repo.buf))
 		repair = _(".git file incorrect");
@@ -695,12 +698,12 @@ static void repair_gitfile(struct worktree *wt,
 	}
 
 done:
-	free(dotgit_contents);
 	free(path);
 	strbuf_release(&repo);
 	strbuf_release(&dotgit);
 	strbuf_release(&gitdir);
 	strbuf_release(&backlink);
+	strbuf_release(&contents);
 }
 
 static void repair_noop(int iserr UNUSED,
@@ -857,14 +860,7 @@ void repair_worktree_at_path(struct repository *repo,
 	strbuf_realpath_forgiving(&inferred_backlink, inferred_backlink.buf, 0);
 	dotgit_contents = xstrdup_or_null(read_gitfile_gently(dotgit.buf, &err));
 	if (dotgit_contents) {
-		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);
-		}
+		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);

base-commit: dea0ea3582e6980ddbc1173cc8e3e9f9db91cde0
-- 
gitgitgadget

  parent reply	other threads:[~2026-08-21 20:36 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-15 13:11 [PATCH] worktree repair: detect relative path in .git file correctly Yoichi NAKAYAMA via GitGitGadget
2026-08-17 17:21 ` Junio C Hamano
2026-08-17 21:27   ` Yoichi Nakayama
2026-08-20 15:46 ` [PATCH v2] " Yoichi NAKAYAMA via GitGitGadget
2026-08-21  2:09   ` Junio C Hamano
2026-08-21 20:36 ` Yoichi NAKAYAMA via GitGitGadget [this message]
2026-08-21 22:02   ` [PATCH v3] " Junio C Hamano
2026-08-21 22:20     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.2205.v3.git.1787344586470.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=yoichi.nakayama@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.