Re: [RFC PATCH v2 3/4] setup: introduce new helper 'is_git_directory_verbose'

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 30, 2026, 18:32 UTC
Message-ID: <xmqqv77ma8ty.fsf@gitster.g>
In-Reply-To: <20260929102513.712181-4-kaartic.sivaraam@gmail.com>

Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:

Show 12 quoted lines

> Introduce a new helper is_git_directory_verbose() as a
> counterpart to the existing is_git_directory().
>
> is_git_directory_verbose() also populates an optional
> string strbuf with reasoning around why the given suspect
> is not a valid git directory. This strbuf in turn can be
> used to improve the error reporting which is currently blunt:
>
>   fatal: not a git repository
>
> This is not helpful as the user does not get any hint about "why"
> the repository is not considered valid.

I suspect that it is much less the failure to report the reason that may confuse users than the failure to report which directory was inspected and why we picked that directory. Once we make it clear which directory was used as the repository to run the Git operation the user specified, they can visit the directory themselves and see that there is no HEAD, etc. The user may have a leftover GIT_DIR in the environment that is interfering with their operation without their remembering they still had it, for example. For this reason, I am not so enthused to see this much code churn to tell the user the subtle differences among a dangling symlink HEAD, a HEAD pointing outside refs, and a missing HEAD.

What would be preferable is a much less invasive patch that reports which path was assumed to be the repository and where it came from (among GIT_DIR, auto-discovery stopped at GIT_CEILING_DIRECTORIES, etc.). It would help the user figure out why the directory they thought was the Git directory is not what the git binary inspected.

This patch does report which path we thought HEAD should be at in its messages, but does not explain where that assumption came from, unlike the verification of the objects/ directory, which mentions the environment variable if it is involved. This feels uneven.

Thanks.