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

[Junio C Hamano <gitster@pobox.com>]

Tian Yuchen <a3205153416@gmail.com> writes:

> It seems that failure "fatal: error reading 'nul'" matches the 
> 'die(_("error reading %s"), path)', if my understanding is correct?

The message is likely to be with its own pair of single quotes,
i.e., "error reading '%s'".

> So during 'git diff --no-index', the test passes 'GIT_DIR=/dev/null'. I 
> highly suspect that 'stat("nul")' on Windows fails with an 'errno' other 
> than 'ENOENT', so it falls into the 'STAT_FAILED' branch...

In the expected case, how would

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

start up?  setup_git_directory_gently() is called with a non-NULL
&nongit in turn it calls setup_git_directory_gently_1() that gives
us GIT_DIR_EXPLICIT, which makes us call setup_explicit_git_dir().

It would make us call is_git_diretory() on "/dev/null", and a
non-NULL nongit_ok pointer causes us to return NULL from there.

I am not sure how the result of calling stat() gets in the picture.

> ...which can be simply fixed by reverting 'READ_GITFILE_ERR_STAT_FAILED' 
> (and probably 'READ_GITFILE_ERR_NOT_A_FILE') back to being non-fatal 
> inside 'read_gitfile_error_die()', if I'm correct? In that case, the 
> logic of the test script should also be changed, shouldn't it?
>
> However, I'm sure that this change runs counter to our previous 
> discussions. I can't think of any good ideas since I'm no windows expert 
> either. So you can pretend I never said anything :P

It sounds like saying "oh, this is too hard, let's punt and roll all
failures from stat() into 'nothing there, move on'", which is how
the code used to work before this patch X-<.