From: Karthik Nayak <karthik.188@gmail.com>
Date: Wed, 18 Feb 2026 10:27:44 GMT
Subject: Re: [PATCH v5 2/2] setup: allow cwd/.git to be a symlink to a directory
Message-ID: <CAOLa=ZSAjDbC5bM+XvNwXW_WLWDiPfzAgaB+gHR6+DwhMW3uEw@mail.gmail.com>
In-Reply-To: <20260218051850.164972-3-a3205153416@gmail.com>

Tian Yuchen <a3205153416@gmail.com> writes:

> Strictly enforcing 'lstat()' prevents valid '.git' symlinks.
>
> Switch 'setup_git_directory_gently_1()' to use 'stat()' to allow
> filesystem resolution.

But we don't really do this no? We were calling `read_gitfile_gently()`
before and continue to do so, so there was no change regards to calling
`stat()` here. Or am I missing something?

>
> Calling the refactored 'read_gitfile_error_die()' to ensure safety:
>
> 1. Happy cases ('ENOENT', 'IS_A_DIR') are ignored automatically;
> 2. Invalid types (like FIFOs and sockets) trigger 'die()' via
>    'NOT_A_FILE'.
>
> Add 't/t0009-git-dir-validation.sh' to verify symlink support and FIFO
> rejection, and register it in 't/meson.build'.
>
> Signed-off-by: Tian Yuchen <a3205153416@gmail.com>
> ---
>  setup.c                       | 18 ++++-----
>  t/meson.build                 |  1 +
>  t/t0009-git-dir-validation.sh | 72 +++++++++++++++++++++++++++++++++++
>  3 files changed, 81 insertions(+), 10 deletions(-)
>  create mode 100755 t/t0009-git-dir-validation.sh
>
> diff --git a/setup.c b/setup.c
> index 0ca129623e..6e6068e5eb 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1590,17 +1590,15 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
>  		gitdirenv = read_gitfile_gently(dir->buf, die_on_error ?
>  						NULL : &error_code);
>  		if (!gitdirenv) {
> -			if (die_on_error ||
> -			    error_code == READ_GITFILE_ERR_NOT_A_FILE) {

Earlier if die_on_error was false and we got any other error, let's say
READ_GITFILE_ERR_INVALID_FORMAT. Then we'd return
GIT_DIR_INVALID_GITFILE from here.

> -				/* NEEDSWORK: fail if .git is not file nor dir */
> -				if (is_git_directory(dir->buf)) {
> -					gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
> -					gitdir_path = xstrdup(dir->buf);
> -				}
> -			} else if (error_code != READ_GITFILE_ERR_STAT_FAILED)
> -				return GIT_DIR_INVALID_GITFILE;
> -		} else
> +			if (error_code)
> +				read_gitfile_error_die(error_code, dir->buf, NULL);
> +			if (is_git_directory(dir->buf)) {
> +				gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
> +				gitdir_path = xstrdup(dir->buf);
> +			}

But now we'd die. Correct? Doesn't that change the expected flow?

> +		} else {
>  			gitfile = xstrdup(dir->buf);
> +		}
>  		/*
>  		 * Earlier, we tentatively added DEFAULT_GIT_DIR_ENVIRONMENT
>  		 * to check that directory for a repository.

[snip]