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

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

Tian Yuchen <a3205153416@gmail.com> writes:

Show 33 quoted lines

> 'read_gitfile_gently()' treats any non-regular file as
> 'READ_GITFILE_ERR_NOT_A_FILE' and fails to discern between 'ENOENT'
> and other stat failures. This flawed error reporting is noted by two
> 'NEEDSWORK' comments.
>
> Address these comments by introducing two new error codes:
> 'READ_GITFILE_ERR_STAT_ENOENT' and 'READ_GITFILE_ERR_IS_A_DIR'.
>
> To preserve the original intent of the setup process:
> 1. Update 'read_gitfile_error_die()' to treat 'IS_A_DIR' as a no-op
>    (like 'ENOENT'), while still calling 'die()' on true 'NOT_A_FILE'
>    errors.
> 2. Unconditionally pass '&error_code' to 'read_gitfile_gently()'. This
>    eliminates an uninitialized variable hazard that occurred when
>    'die_on_error' was true and 'NULL' was passed.
> 3. Only invoke 'is_git_directory()' when we explicitly receive
>    'READ_GITFILE_ERR_IS_A_DIR', avoiding redundant filesystem checks.
> 4. Correctly return 'GIT_DIR_INVALID_GITFILE' on unrecognized errors
>    when 'die_on_error' is false.
>
> Additionally, audit external callers of 'read_gitfile_gently()' in
> 'submodule.c' and 'worktree.c' to accommodate the refined error codes.
>
> Signed-off-by: Tian Yuchen <a3205153416@gmail.com>
> ---
>  setup.c                       | 42 ++++++++++++++------
>  setup.h                       |  2 +
>  submodule.c                   |  2 +-
>  t/meson.build                 |  1 +
>  t/t0009-git-dir-validation.sh | 72 +++++++++++++++++++++++++++++++++++
>  worktree.c                    |  6 ++-
>  6 files changed, 110 insertions(+), 15 deletions(-)
>  create mode 100755 t/t0009-git-dir-validation.sh

We'd probably need to treat ENOTDIR the same way as ENOENT to deal with cases where we expect a directory "sm1" to be the root of a submodule working tree, and we have a modification that removes the submodule directory and replace it with a regular file "sm1". In the code path touched by this patch in submodule.c, we would ask "is sm1/.git a git directory?" and the stat(2) call on that path in read_gitfile_gently() used to say "Ah, a failure, that means we cannot positively say that 'sm1/.git' is a git directory or a gitdir file." Now we inspect the error code in an attempt to tell if it is a system failure (e.g., a corrupt filesystem), but catching only ENOENT is probably a bit too tight. In the above scenario, asking about 'sm1/.git' when 'sm1' is a regular file will not result in ENOENT but in ENOTDIR (i.e., "the leading 'sm1' is not a directory so it makes no sense to ask about 'sm1/.git'").

Is it always sensible to treat ENOTDIR and ENOENT as two equivalent errors for the purpose of read_gitfile_gently()? I have no clear answer offhand myself. This is part of what we need to think about and resolve while addressing the original "NEEDSWORK:" comment.