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

[Phillip Wood <phillip.wood123@gmail.com>]

On 02/03/2026 16:26, Junio C Hamano wrote:
> Let's try again.
> 
> Can folks equipped with knowledge and environment to debug breakages
> that hapepns only on Windows lend a hand to figure out what this
> patch gets wrong to help the topic move forward?

Looking at the test failures the tests are failing because

     GIT_DIR=/dev/null git diff --no-index ...

which is used by test_cmp() on Windows is dying. That happens because 
stat("nul", &st) fails and this series makes that an error (somewhere 
along the line "/dev/null" is rewritten to "nul" on Windows). I'm afraid 
I don't know enough about Windows to be sure how to fix it but maybe we 
should special case "nul" in the setup code or mingw_stat(). We should 
also check what happens when GIT_DIR=/dev/null on linux and other POSIX 
platforms.

Thanks

Phillip


> Thanks.
> 
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Tian Yuchen <a3205153416@gmail.com> writes:
>>
>>> '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_MISSING'(which groups the "file missing" scenarios
>>> together) and 'READ_GITFILE_ERR_IS_A_DIR'.
>>>
>>> To preserve the original intent of the setup process:
>>> 1. Update 'read_gitfile_error_die()' to treat both 'IS_A_DIR' and
>>>     'MISSING' as no-ops, while continuing to call 'die()' on true
>>>     'NOT_A_FILE' errors to prevent security hazards (like FIFOs).
>>> 2. Unconditionally pass '&error_code' to 'read_gitfile_gently()'.
>>> 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                       | 45 ++++++++++++++------
>>>   setup.h                       |  2 +
>>>   submodule.c                   |  2 +-
>>>   t/meson.build                 |  1 +
>>>   t/t0009-git-dir-validation.sh | 77 +++++++++++++++++++++++++++++++++++
>>>   worktree.c                    |  6 ++-
>>>   6 files changed, 118 insertions(+), 15 deletions(-)
>>>   create mode 100755 t/t0009-git-dir-validation.sh
>>
>> Unfortunately this seems to break almost all the tests, not just the
>> test the patch adds, on Windows (which I almost know nothing about,
>> but I can observe that CI jobs die).
>>
>> https://github.com/git/git/actions/runs/22464017037 is a CI run that
>> merged this patch on top of the commit that corresponds to the tip
>> of 'next' as of today.  We can see "win test (N)" jobs dying all
>> over.  I cancelled the workflow before seeing everything die,
>> though.
>>
>> https://github.com/git/git/actions/runs/22464479533 is a CI run that
>> tests this patch applied directly on v2.53.0 in isolation.
>>
>> As I said, I do not know Windows well, so this may be a red-herring,
>> but in this CI run, we see "GIT_DIR=/dev/null git diff --no-index ..."
>> results in "fatal: error reading 'nul'":
>>
>>    https://github.com/git/git/actions/runs/22464479533/job/65067515458#step:5:95419
>>
>> which is an expected thing to happen, but we probably used to ignore
>> it as a non-error?
>>
>> For now, I'll kick this topic out of my tree to give other topics a
>> bit more test exposure so that we can notice new bugs in them (not
>> in this topic) that causes the tests fail.  With this topic in 'seen',
>> such bugs in other topics are all masked.
>>
>>
>>
>> Thanks.
>