Re: [PATCH v2] setup: fail if .git is not a file or directory

[Tian Yuchen <a3205153416@gmail.com>]

On 2/13/26 04:59, Junio C Hamano wrote:

> Why this indentation change?
> 
>> -				/* NEEDSWORK: fail if .git is not file nor dir */
>> -				if (is_git_directory(dir->buf)) {
>> +				if (is_git_directory(dir->buf)){
> 
> Why this change (which is style violation that lack necessary SP
> between "){")?

Sorry for the typo. I will fix it in the next patch :(

> Stepping back a bit, even though the NEEDSWORK comment is placed
> here, I am not sure if this is the bast place to do much of the
> necessary work.
> 
> When gitdirenv is NULL and die_on_error is set, we would have died
> on any error other than STAT_FAILED (most often, this is because
> there wasn't .git at the path in the first place) or NOT_A_FILE
> (again, a happy case is that .git existed and it was a directory).
> We would have already died in read_gitfile_error_die() in all other
> cases.  But these two error cases are not necessarily entirely happy
> and that is what the NEEDSWORK comment is about.
> 
> So, if we wanted to tighten the error checking to help users
> diagnose problems in their filesystem, I wonder if it is a better
> approach to refine the set of READ_GITFILE_ERR_* error codes:
> 
>   - STAT_ENOENT (new) is returned when stat failed and we got ENOENT,
>     and it is not a fatal error.
> 
>   - STAT_FAILED becomes a fatal error in read_gitfile_error_die().
> 
>   - IS_A_DIR (new) is returned when stat succeeded and it is a
>     directory.  It is not a fatal error.
> 
>   - NOT_A_FILE becomes a fatal error in read_gitfile_error_die().

This approach makes sense to me. I will work on v3 patch that:

1. Refines 'read_gitfile_error' with 'STAT_ENOENT' and 'IS_A_DIR'.

2. Makes 'STAT_FAILED' and 'NOT_A_FILE' fatal by default in 
'read_gitfile_error_die()'

3. Adjusts existing callers to handle these new codes.

This will be a larger refactor, so it might take a bit of time to ensure 
I don't break other call sites.

> Existing callers of the two functions, read_gitfile_gently() and
> read_gitfile_error_die(), must be audited and adjusted
> appropriately, but once it is done, it would become much simpler,
> wouldn't it?
Yes indeed.

Thanks for guiding me toward a cleaner architecture.


Buon san valentino,

Yuchen