Re: [RFC PATCH 2/3] setup: introduce new helper 'is_git_directory_verbose'
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 24, 2026, 22:11 UTC
- Message-ID
- <xmqqcxu2z4cy.fsf@gitster.g>
- In-Reply-To
- <20260924120502.2642141-3-kaartic.sivaraam@gmail.com>
Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:
Show 66 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.
>
> Call-site(s) will be made to use this helper in a follow-up commit.
>
> Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
> ---
> setup.c | 152 +++++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 112 insertions(+), 40 deletions(-)
>
> diff --git a/setup.c b/setup.c
> index 0d157ac254..b3b53a1cfc 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -347,7 +347,7 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)
> return ret;
> }
>
> -static int validate_headref(const char *path)
> +static int validate_headref(const char *path, struct strbuf *err)
> {
> struct stat st;
> char buffer[256];
> @@ -356,14 +356,32 @@ static int validate_headref(const char *path)
> int fd;
> ssize_t len;
>
> - if (lstat(path, &st) < 0)
> + if (lstat(path, &st) < 0) {
> + if (err)
> + strbuf_addf(
> + err, _("could not stat HEAD at '%s'"), path
> + );
> return -1;
> + }
>
> /* Make sure it is a "refs/.." symlink */
> if (S_ISLNK(st.st_mode)) {
> len = readlink(path, buffer, sizeof(buffer)-1);
> if (len >= 5 && !memcmp("refs/", buffer, 5))
> return 0;
> + if (len == -1 && err)
> + strbuf_addf(
> + err,
> + _("could not read the symlink HEAD at '%s'"),
> + path
> + );
> + else if (err)
> + strbuf_addf(
> + err,
> + _("HEAD is a symlink ('%s') but target"
> + " lives outside refs/"),
> + path
> + );
> return -1;
> }All of the above (and below---ellided) look fairly funny way to indent them. If you are trying ot match the style used in the existing code around the same area, I wouldn't complain, but I didn't look beyond what is visible in the patch.
> + }
> + else {Style: "} else {" go on a single line.