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

On 9/30/26 21:33, Patrick Steinhardt wrote:

On Tue, Sep 29, 2026 at 03:55:10PM +0530, Kaartic Sivaraam wrote:

quoted

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.

quoted

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.

quoted

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.

quoted

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