Re: [PATCH v2] setup: fail if .git is not a file or directory

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 12, 2026, 20:59 UTC
Message-ID: <xmqqtsvln0ev.fsf@gitster.g>
In-Reply-To: <20260212172405.48614-1-a3205153416@gmail.com>

Tian Yuchen <a3205153416@gmail.com> writes:

>  		if (!gitdirenv) {
>  			if (die_on_error ||
> -			    error_code == READ_GITFILE_ERR_NOT_A_FILE) {
> +				error_code == READ_GITFILE_ERR_NOT_A_FILE) {

Why this indentation change?

> -				/* NEEDSWORK: fail if .git is not file nor dir */
> -				if (is_git_directory(dir->buf)) {
> +				if (is_git_directory(dir->buf)){

Why this change (which is style violation that lack necessary SP between "){")?

Show 16 quoted lines

>  					gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
>  					gitdir_path = xstrdup(dir->buf);
> +				} else {
> +					/*
> +					 * It is neither a gitfile nor a git directory.
> +					 */
> +					struct stat st;
> +					if (!lstat(dir->buf, &st) &&
> +						!S_ISREG(st.st_mode) &&
> +						!S_ISDIR(st.st_mode)) {
> +						if (die_on_error)
> +							die(_("Invalid %s: not a regular file or directory"), dir->buf);
> +						else
> +							return GIT_DIR_INVALID_GITFILE;
> +					}
>  				}

Stepping back a bit, even though the NEEDSWORK comment is placed here, I am not sure if this is the bast place to do much of the necessary work.

When gitdirenv is NULL and die_on_error is set, we would have died on any error other than STAT_FAILED (most often, this is because there wasn't .git at the path in the first place) or NOT_A_FILE (again, a happy case is that .git existed and it was a directory). We would have already died in read_gitfile_error_die() in all other cases. But these two error cases are not necessarily entirely happy and that is what the NEEDSWORK comment is about.

So, if we wanted to tighten the error checking to help users diagnose problems in their filesystem, I wonder if it is a better approach to refine the set of READ_GITFILE_ERR_* error codes:

 - STAT_ENOENT (new) is returned when stat failed and we got ENOENT,
   and it is not a fatal error.

 - STAT_FAILED becomes a fatal error in read_gitfile_error_die().

 - IS_A_DIR (new) is returned when stat succeeded and it is a
   directory.  It is not a fatal error.

 - NOT_A_FILE becomes a fatal error in read_gitfile_error_die().

Existing callers of the two functions, read_gitfile_gently() and read_gitfile_error_die(), must be audited and adjusted appropriately, but once it is done, it would become much simpler, wouldn't it?

Thanks.