Re: [PATCH v3] worktree repair: detect relative path in .git file correctly

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 21, 2026, 22:02 UTC
Message-ID: <xmqq8q5zyvwd.fsf@gitster.g>
In-Reply-To: <pull.2205.v3.git.1787344586470.gitgitgadget@gmail.com>

"Yoichi NAKAYAMA via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 19 quoted lines

> 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.

Excellent observation of the problem addressed by the patch. I wish everybody wrote his or her proposed log message this clearly.

Show 25 quoted lines

> 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);
> +	}

This massages path = "worktrees/foo/.git" into "worktrees/foo". And the non-absolute contents.buf "../main/.git/worktrees/foo" that is relative to gitfile is turned into relative to cwd of our process by prepending "worktrees/foo" to it.

> +	if (!is_git_directory(contents.buf)) {
> +		error_code = READ_GITFILE_ERR_NOT_A_REPO;
> +		goto cleanup_return;
> +	}

This ensures that the thing referenced by .git file (i.e., what comes after "gitdir:") is a sanely formatted git directory.

> +	strbuf_realpath(&realpath, contents.buf, 1);

This turns the thing into an absolute path.

Among these three, the last one obviously belongs here. Leaving the relative path relative was the reason why we wanted to add read_gitfile_raw() in the first place.

But moving the other two to here is a bit iffy. The worktree repair job used to call read_gitfile_gently(), which means it used to depend on what the first two did for it, namely, to make the relative path after "gitdir:" from the .git file relative to the current process to make it usable, and to ensure that the directory pointed at by .git is indeed a git directory. Is it correct to drop these from the caller, which now calls read_gitfile_raw() instead?

IOW, I am not sure if the two functions are split correctly. I expected that the only two things read_gitfile_gently() would do after read_gitfile_raw() are (1) upon error, jump to cleanup_return, and (2) otherwise call strbuf_realpath().

Thanks.