Re: [PATCH v7 2/3] repository: keep a symlink-preserving copy of the worktree path

[Patrick Steinhardt <ps@pks.im>]

On Thu, Jul 09, 2026 at 10:41:42AM +0800, Chen Linxuan via B4 Relay wrote:
> diff --git a/repository.c b/repository.c
> index 73d80bcffdf5..a29d55a6fcd3 100644
> --- a/repository.c
> +++ b/repository.c
> @@ -149,6 +149,11 @@ const char *repo_get_work_tree(struct repository *repo)
>  	return repo->worktree;
>  }
>  
> +const char *repo_get_work_tree_original(struct repository *repo)
> +{
> +	return repo->worktree_original;
> +}

Feels a bit heavy-handed to have such an accessor, as we could've just
as well accessed the member directly via the structure.

> diff --git a/setup.c b/setup.c
> index 0de56a074f7c..fbbeb95f99db 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1213,12 +1213,94 @@ static const char *setup_explicit_git_dir(struct repository *repo,
>  	return NULL;
>  }
>  
> +/*
> + * Do "a" and "b" refer to the same filesystem entry? Both must report a
> + * nonzero (dev,ino): some filesystems return (0,0) for unrelated paths,
> + * which would otherwise look identical.
> + */
> +static int same_entry(const char *a, const char *b)
> +{
> +	struct stat sa, sb;
> +
> +	if (stat(a, &sa) || stat(b, &sb))
> +		return 0;
> +	return (sa.st_dev || sa.st_ino) &&
> +	       sa.st_dev == sb.st_dev && sa.st_ino == sb.st_ino;
> +}
> +
> +/*
> + * Recover the symlink-preserving spelling of the worktree root.
> + *
> + * strbuf_add_absolute_path() already consults $PWD to keep symlinks when
> + * resolving a relative path, so set_git_work_tree()'s other callers get a
> + * symlink-preserving worktree path for free.  This function exists for the
> + * discovered-repository case: setup_git_directory_gently() chdir()s to the
> + * worktree root *before* set_git_work_tree(repo, ".") runs, so by the time
> + * "." is resolved $PWD still names the caller's original directory and no
> + * longer agrees with the physical cwd, and strbuf_add_absolute_path()
> + * falls back to the realpath.  We close that gap by deriving the logical
> + * root here, from $PWD, while we still have the original physical cwd and
> + * the root offset in hand.
> + *
> + * "cwd" is the physical current directory (getcwd), and "root_len" is the
> + * length of the worktree root within it; cwd->buf[root_len..] is therefore
> + * the part of the path below the root (empty when git ran at the root).
> + *
> + * $PWD, maintained by the shell, may spell that same directory through
> + * symlinks.  If we can confirm $PWD really names cwd's directory (same
> + * device/inode) and that the below-root suffix matches, we swap the
> + * physical root prefix for $PWD's prefix and keep the user's symlinks.
> + * Only symlinks in the root prefix itself are preserved: the below-root
> + * suffix is matched byte-for-byte, so a symlink below the root is not.
> + *
> + * Returns the allocated logical path, or NULL when $PWD is missing, already
> + * physical, or untrustworthy.
> + */

Oof.

> +static char *logical_path_from_cwd(struct strbuf *cwd, int root_len)
> +{
> +	const char *pwd = getenv("PWD");
> +	size_t suffix_len, pwd_len;
> +	struct strbuf path = STRBUF_INIT;
> +
> +	if (!pwd || !is_absolute_path(pwd) || !strcmp(pwd, cwd->buf))
> +		return NULL;
> +	/*
> +	 * $PWD is a plain environment variable: it can be set to anything,
> +	 * or left stale after a chdir.  Only borrow its symlink-preserving
> +	 * spelling once we prove it still points at the same directory as
> +	 * the physical cwd; otherwise give up and return NULL.
> +	 */
> +	if (!same_entry(cwd->buf, pwd))
> +		return NULL;
> +
> +	/*
> +	 * Drop the below-root suffix from $PWD.  It must match the physical
> +	 * suffix exactly; the only spelling difference we accept is in the
> +	 * root prefix -- i.e. the symlinks we want to preserve.
> +	 */
> +	suffix_len = cwd->len - root_len;
> +	pwd_len = strlen(pwd);
> +	if (suffix_len) {
> +		const char *suffix = cwd->buf + root_len;
> +
> +		if (suffix_len > pwd_len ||
> +		    fspathcmp(pwd + pwd_len - suffix_len, suffix))
> +			return NULL;
> +		pwd_len -= suffix_len;
> +	}
> +
> +	strbuf_add(&path, pwd, pwd_len);
> +	return strbuf_detach(&path, NULL);
> +}

This feels quite awkward to me, and I assume that these changes will
lead to conflicts with ps/setup-split-discovery-and-setup.

I wonder whether we can maybe avoid this whole mess by removing the call
to chdir(3p) when discovering Git directories in the first place.
Instead, we'd only chdir(3p) after we have fully discovered the Git
repository's paths, and that may allow us to not have to worry about
reconstructing the logical path?

It's something that I wanted to explore after the mentioned patch series
has landed, but maybe it's something we should try to do as part of this
patch series here.

Alternatively, I'm less certain that this complexity is ultimately
really worth it now... so another alternative could be to document the
issue and fix it at a later point in time.

Patrick