Re: [RFC PATCH v2 4/4] setup: communicate why a directory is not a valid git directory

[Kaartic Sivaraam <kaartic.sivaraam@gmail.com>]

On 9/30/26 21:33, Patrick Steinhardt wrote:
> On Tue, Sep 29, 2026 at 03:55:10PM +0530, Kaartic Sivaraam wrote:
>> At the moment, there are a few scenarios in which the error message
>> surrounding an invalid Git repository is a bit blunt:
>>
>>    $ GIT_OBJECT_DIRECTORY=/does/not/exist git --git-dir repo.git rev-parse --is-bare-repository
>>    fatal: not a git repository: 'repo.git'
>>
>> In this case, even though repo.git is a valid Git repository,
>> we get an output saying it is not since the GIT_OBJECT_DIRECTORY
>> does not point to a valid object directory. At the moment, the
>> user is on their own in figuring this out.
>>
>> Instead, make it more easy for users to figure such issues
> 
> s/more easy/easier/
>

Ah. Will correct.

>> particularly in cases where they have explicitly specified
>> a Git directory. This intends to improve the error reporting UX
>> by clarifying why the specified repository is not considered valid.
> 
> Which I think is a good motivation.
> 
>> We achieve this by means of using the new helper
>> is_git_directory_verbose() that has been introduced. With the
>> same, we get a more helpful error message as follows:
>>
>>    $ GIT_OBJECT_DIRECTORY=/does/not/exist git --git-dir repo.git rev-parse --is-bare-repository
>>    fatal: not a git repository: 'repo.git'
>>    reason: cannot access object directory '/does/not/exist' set via $GIT_OBJECT_DIRECTORY
> 
> Having a separate "reason:" line feels a bit off to me, but that may be
> subjective. I'd have preferred to have it on the same line, or maybe
> first have "error:" followed by "fatal:".
> 

Hmm. Let me think which of these I could adopt. Thank you for the 
suggestion.

>> diff --git a/setup.c b/setup.c
>> index a0fb68f7f6..a0d3c0c5bb 100644
>> --- a/setup.c
>> +++ b/setup.c
>> @@ -1195,6 +1195,7 @@ static void repo_discover_explicit_gitdir(struct repo_discovery *discovery,
>>   					  int *nongit_ok)
>>   {
>>   	const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);
>> +	struct strbuf invalid_gitdir_reason = STRBUF_INIT;
> 
> Nit, please feel free to ignore: I'd just have called this `errbuf`.
> 

Noted.

-- 
Sivaraam