Re: [PATCH v11] setup: improve error diagnosis for invalid .git files
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Mar 3, 2026, 19:31 UTC
- Message-ID
- <460f00d5-97b4-4a6c-be45-6f60a17cd33e@gmail.com>
- In-Reply-To
- <xmqqjyvu42pw.fsf@gitster.g>
On 02/03/2026 16:26, Junio C Hamano wrote:
Show 5 quoted lines
> 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
Show 70 quoted lines
> 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. >