Re: [PATCH v9] setup: improve error diagnosis for invalid .git files

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 22, 2026, 05:42 UTC
Message-ID: <xmqqseatqqpr.fsf@gitster.g>
In-Reply-To: <20260221083001.220061-1-a3205153416@gmail.com>

Tian Yuchen <a3205153416@gmail.com> writes:

Show 16 quoted lines

>  void read_gitfile_error_die(int error_code, const char *path, const char *dir)
>  {
>  	switch (error_code) {
> -	case READ_GITFILE_ERR_STAT_FAILED:
> -	case READ_GITFILE_ERR_NOT_A_FILE:
> +	case READ_GITFILE_ERR_STAT_ENOENT:
> +	case READ_GITFILE_ERR_IS_A_DIR:
>  		/* non-fatal; follow return path */
>  		break;
> +	case READ_GITFILE_ERR_STAT_FAILED:
> +		die(_("error reading %s"), path);
> +	case READ_GITFILE_ERR_NOT_A_FILE:
> +		die(_("not a regular file: %s"), path);
>  	case READ_GITFILE_ERR_OPEN_FAILED:
>  		die_errno(_("error opening '%s'"), path);
>  	case READ_GITFILE_ERR_TOO_LARGE:

The changes to these two functions require us to audit callers of them that are outside the call graph of the main focus of this patch. For example, we see the following code in submodule.c:

        void absorb_git_dir_into_superproject(const char *path,
                                              const char *super_prefix)
        {
                int err_code;
                const char *sub_git_dir;
                struct strbuf gitdir = STRBUF_INIT;

                if (validate_submodule_path(path) < 0)
                        exit(128);

                strbuf_addf(&gitdir, "%s/.git", path);
                sub_git_dir = resolve_gitdir_gently(gitdir.buf, &err_code);

                /* Not populated? */
                if (!sub_git_dir) {
                        const struct submodule *sub;
                        struct strbuf sub_gitdir = STRBUF_INIT;

                        if (err_code == READ_GITFILE_ERR_STAT_FAILED) {
                                /* unpopulated as expected */
                                strbuf_release(&gitdir);
                                return;
                        }

                        if (err_code != READ_GITFILE_ERR_NOT_A_REPO)
                                /* We don't know what broke here. */
                                read_gitfile_error_die(err_code, path, NULL);

Let's take a look.

ERR_STAT_FAILED used to be "If we got ENOENT, it is an unpopulated submodule so it is OK; it is so unlikely that we get other kinds of failure and we cannot deal with them anyway".

With the patch we have been looking at, shouldn't the above code check for ERR_STAT_ENOENT instead, to do the same "unpopulated, nothing to do here, happy!" return?

This is merely just one example. All hits from "git grep" for the functions whose semantics have been updated by the patch must be looked at in a similar way, but I didn't look at how many releavnt code paths there are. It could be an easier way to find code paths that may need to be updated to grep for ERR_STAT_FAILED and ERR_NOT_A_FILE, the two symbols whose meaning have changed.

Thanks.