{"thread":{"id":"66383","subject":"[RFC PATCH 0/3] Improve error reporting to mention \"why\" a directory is not a repository","startedAt":"2026-09-24T12:05:13Z","lastAt":"2026-10-05T12:29:28Z","messageCount":20,"participants":["Kaartic Sivaraam","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"553170","messageId":"20260924120502.2642141-1-kaartic.sivaraam@gmail.com","threadId":"66383","inReplyTo":null,"subject":"[RFC PATCH 0/3] Improve error reporting to mention \"why\" a directory is not a repository","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-24T12:02:18Z","receivedAt":"2026-09-24T12:05:13Z","isPatch":true,"body":"At the moment, there are a few scenarios where the error message for an\ninvalid Git repository is a bit blunt. For instance, when we point\nGIT_OBJECT_DIRECTORY at a directory that does not exist, we get this:\n\n  $ GIT_OBJECT_DIRECTORY=/does/not/exist git --git-dir repo.git rev-parse --is-bare-repository\n  fatal: not a git repository: 'repo.git'\n\nEven though repo.git itself is a valid repository, we get this rather\npuzzling error saying that it is not. The actual problem is the invalid\nvalue given to GIT_OBJECT_DIRECTORY, and the user is left on their\nto figure that out. This series aims to make such issues easier to\ndiagnose by saying why the specified repository was not considered\nvalid.\n\nWith this series, the same command outputs:\n\n  $ GIT_OBJECT_DIRECTORY=/does/not/exist git --git-dir repo.git rev-parse --is-bare-repository\n  fatal: not a git repository: 'repo.git'\n  reason: cannot access object directory '/does/not/exist' set via $GIT_OBJECT_DIRECTORY\n\nThe following scenarios are covered when --git-dir is given explicitly:\n\n  - HEAD is missing, or its path is not traversable\n  - HEAD is a symlink that cannot be read\n  - HEAD is a symlink whose target lives outside refs/\n  - HEAD cannot be opened, or cannot be read\n  - HEAD contains neither a ref under refs/ nor an object ID\n  - $GIT_OBJECT_DIRECTORY is set to something we cannot access\n  - the object directory in the common directory is inaccessible\n  - the refs directory in the common directory is inaccessible\n\nThis series does not yet report a reason in the following cases.\n\nThe discovery walk, where we iterate up to the ceiling or the mount\npoint looking for a repository:\n\n  $ GIT_OBJECT_DIRECTORY=/does/not/exist git rev-parse --is-bare-repository\n  fatal: not a git repository (or any parent up to mount point /)\n  Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)\n\nThe gitfile case, for instance when run from inside a submodule /\nworktree:\n\n  $ GIT_OBJECT_DIRECTORY=/does/not/exist git rev-parse --is-bare-repository\n  fatal: gitfile does not point to a valid repository: /path/to/super/sub/.git\n\nGIT_ALTERNATE_OBJECT_DIRECTORIES is also left alone. Its diagnostic\nare misleading in their own way, but they are emitted much later, from\nthe object database rather than from setup, so they need a separate\ntreatment.\n\nI would be very interested in hearing what others think about this\ndirection. If it looks reasonable, I'm happy to cover the remaining\ncases too, either in this series or separately.\n\nKaartic Sivaraam (3):\n  t0009: add tests to cover more error reporting scenarios\n  setup: introduce new helper 'is_git_directory_verbose'\n  setup: communicate why a directory is not a valid git directory\n\n setup.c                       | 165 +++++++++++++++++++++++++---------\n t/t0009-git-dir-validation.sh |  38 ++++++++\n 2 files changed, 161 insertions(+), 42 deletions(-)\n\n-- \n2.56.0.rc1.12.g2c9c8d64bb\n\n"},{"id":"553171","messageId":"20260924120502.2642141-2-kaartic.sivaraam@gmail.com","threadId":"66383","inReplyTo":"20260924120502.2642141-1-kaartic.sivaraam@gmail.com","subject":"[RFC PATCH 1/3] t0009: add tests to cover more error reporting scenarios","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-24T12:02:19Z","receivedAt":"2026-09-24T12:05:15Z","isPatch":true,"body":"Introduce few more tests to t0009 to cover error reporting scenarios\nwhen --git-dir is used.\n\nSigned-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n---\n t/t0009-git-dir-validation.sh | 36 +++++++++++++++++++++++++++++++++++\n 1 file changed, 36 insertions(+)\n\ndiff --git a/t/t0009-git-dir-validation.sh b/t/t0009-git-dir-validation.sh\nindex 4cba478e50..244dc07c0e 100755\n--- a/t/t0009-git-dir-validation.sh\n+++ b/t/t0009-git-dir-validation.sh\n@@ -74,4 +74,40 @@ test_expect_success 'setup: .git as an empty directory is ignored' '\n \t)\n '\n \n+test_expect_success 'setup: custom git directory with missing HEAD is rejected' '\n+\ttest_when_finished \"rm -rf parent/empty-dir\" &&\n+\tmkdir -p parent/empty-dir &&\n+\t(\n+\t\ttest_must_fail git --git-dir parent/empty-dir rev-parse --is-bare-repository 2>stderr &&\n+\t\ttest_grep \"not a git repository\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: custom git directory with HEAD as a symlink outside refs/ is rejected' '\n+\ttest_when_finished \"rm -rf parent/head-as-link-to-garbage\" &&\n+\tmkdir -p parent/head-as-link-to-garbage &&\n+\t(\n+\t\tcd parent/head-as-link-to-garbage &&\n+\t\tgit init --bare real-repo &&\n+\t\ttouch garbage &&\n+\t\trm real-repo/HEAD &&\n+\t\tln -s ../garbage real-repo/HEAD &&\n+\t\ttest_must_fail git --git-dir real-repo rev-parse --is-bare-repository 2>stderr &&\n+\t\ttest_grep \"not a git repository\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: custom git directory with invalid GIT_OBJECT_DIRECTORY configuration is rejected' '\n+\ttest_when_finished \"rm -rf parent/invalid-git-object-directory-config\" &&\n+\tmkdir -p parent/invalid-git-object-directory-config &&\n+\t(\n+\t\tcd parent/invalid-git-object-directory-config &&\n+\t\tgit init --bare real-repo &&\n+\t\ttest_must_fail env GIT_OBJECT_DIRECTORY=\"$(pwd)/does-not-exist\" \\\n+\t\t\tgit --git-dir real-repo rev-parse --is-bare-repository 2>stderr &&\n+\t\ttest_grep \"not a git repository\" stderr\n+\t)\n+'\n+\n+\n test_done\n-- \n2.56.0.rc1.12.g2c9c8d64bb\n\n"},{"id":"553172","messageId":"20260924120502.2642141-3-kaartic.sivaraam@gmail.com","threadId":"66383","inReplyTo":"20260924120502.2642141-1-kaartic.sivaraam@gmail.com","subject":"[RFC PATCH 2/3] setup: introduce new helper 'is_git_directory_verbose'","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-24T12:02:20Z","receivedAt":"2026-09-24T12:05:18Z","isPatch":true,"body":"Introduce a new helper is_git_directory_verbose() as a\ncounterpart to the existing is_git_directory().\n\nis_git_directory_verbose() also populates an optional\nstring strbuf with reasoning around why the given suspect\nis not a valid git directory. This strbuf in turn can be\nused to improve the error reporting which is currently blunt:\n\n  fatal: not a git repository\n\nThis is not helpful as the user does not get any hint about \"why\"\nthe repository is not considered valid.\n\nCall-site(s) will be made to use this helper in a follow-up commit.\n\nSigned-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n---\n setup.c | 152 +++++++++++++++++++++++++++++++++++++++++---------------\n 1 file changed, 112 insertions(+), 40 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex 0d157ac254..b3b53a1cfc 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -347,7 +347,7 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)\n \treturn ret;\n }\n \n-static int validate_headref(const char *path)\n+static int validate_headref(const char *path, struct strbuf *err)\n {\n \tstruct stat st;\n \tchar buffer[256];\n@@ -356,14 +356,32 @@ static int validate_headref(const char *path)\n \tint fd;\n \tssize_t len;\n \n-\tif (lstat(path, &st) < 0)\n+\tif (lstat(path, &st) < 0) {\n+\t\tif (err)\n+\t\t\tstrbuf_addf(\n+\t\t\t\terr, _(\"could not stat HEAD at '%s'\"), path\n+\t\t\t);\n \t\treturn -1;\n+\t}\n \n \t/* Make sure it is a \"refs/..\" symlink */\n \tif (S_ISLNK(st.st_mode)) {\n \t\tlen = readlink(path, buffer, sizeof(buffer)-1);\n \t\tif (len >= 5 && !memcmp(\"refs/\", buffer, 5))\n \t\t\treturn 0;\n+\t\tif (len == -1 && err)\n+\t\t\tstrbuf_addf(\n+\t\t\t\terr,\n+\t\t\t\t_(\"could not read the symlink HEAD at '%s'\"),\n+\t\t\t\tpath\n+\t\t\t);\n+\t\telse if (err)\n+\t\t\tstrbuf_addf(\n+\t\t\t\terr,\n+\t\t\t\t_(\"HEAD is a symlink ('%s') but target\"\n+\t\t\t\t  \" lives outside refs/\"),\n+\t\t\t\tpath\n+\t\t\t);\n \t\treturn -1;\n \t}\n \n@@ -371,13 +389,23 @@ static int validate_headref(const char *path)\n \t * Anything else, just open it and try to see if it is a symbolic ref.\n \t */\n \tfd = open(path, O_RDONLY);\n-\tif (fd < 0)\n+\tif (fd < 0) {\n+\t\tif (err)\n+\t\t\tstrbuf_addf(\n+\t\t\t\terr, _(\"could not open HEAD at '%s'\"), path\n+\t\t\t);\n \t\treturn -1;\n+\t}\n \tlen = read_in_full(fd, buffer, sizeof(buffer)-1);\n \tclose(fd);\n \n-\tif (len < 0)\n+\tif (len < 0) {\n+\t\tif (err)\n+\t\t\tstrbuf_addf(\n+\t\t\t\terr, _(\"could not read HEAD at '%s'\"), path\n+\t\t\t);\n \t\treturn -1;\n+\t}\n \tbuffer[len] = '\\0';\n \n \t/*\n@@ -396,9 +424,88 @@ static int validate_headref(const char *path)\n \tif (get_oid_hex_any(buffer, &oid) != GIT_HASH_UNKNOWN)\n \t\treturn 0;\n \n+\tif (err)\n+\t\tstrbuf_addf(\n+\t\t\terr,\n+\t\t\t_(\"HEAD at '%s' does not point to a valid\"\n+\t\t\t  \" symbolic link or an object ID\"),\n+\t\t\tpath\n+\t\t);\n+\n \treturn -1;\n }\n \n+/*\n+ * A variant of is_git_directory that gives additional\n+ * context via 'err' about why a given suspect is not\n+ * a valid git repository.\n+ */\n+static int is_git_directory_verbose(const char *suspect, struct strbuf *err)\n+{\n+\tstruct strbuf path = STRBUF_INIT;\n+\tchar *objdir;\n+\tint ret = 0;\n+\tsize_t len;\n+\n+\t/* Check worktree-related signatures */\n+\tstrbuf_addstr(&path, suspect);\n+\tstrbuf_complete(&path, '/');\n+\tstrbuf_addstr(&path, \"HEAD\");\n+\tif (validate_headref(path.buf, err))\n+\t\tgoto done;\n+\n+\tstrbuf_reset(&path);\n+\tget_common_dir(&path, suspect);\n+\tlen = path.len;\n+\n+\t/* Check non-worktree-related signatures */\n+\tobjdir = getenv(DB_ENVIRONMENT);\n+\tif (objdir) {\n+\t\tif (access(objdir, X_OK)) {\n+\t\t\tif (err)\n+\t\t\t\tstrbuf_addf(\n+\t\t\t\t\terr,\n+\t\t\t\t\t_(\"cannot access object directory '%s'\"\n+\t\t\t\t\t  \" set via $%s\\n\"),\n+\t\t\t\tobjdir,\n+\t\t\t\tDB_ENVIRONMENT\n+\t\t\t);\n+\t\t\tgoto done;\n+\t\t}\n+\t}\n+\telse {\n+\t\tstrbuf_setlen(&path, len);\n+\t\tstrbuf_addstr(&path, \"/objects\");\n+\t\tif (access(path.buf, X_OK)) {\n+\t\t\tif (err)\n+\t\t\t\tstrbuf_addf(\n+\t\t\t\t\terr,\n+\t\t\t\t\t_(\"cannot access object directory '%s'\"),\n+\t\t\t\t\tpath.buf\n+\t\t\t\t);\n+\t\t\tgoto done;\n+\t\t}\n+\t}\n+\n+\tstrbuf_setlen(&path, len);\n+\tstrbuf_addstr(&path, \"/refs\");\n+\tif (access(path.buf, X_OK)) {\n+\t\tif (err)\n+\t\t\tstrbuf_addf(\n+\t\t\t\terr,\n+\t\t\t\t_(\"cannot access refs directory '%s'\"),\n+\t\t\t\tpath.buf\n+\t\t\t);\n+\t\tgoto done;\n+\t}\n+\n+\tret = 1;\n+done:\n+\tstrbuf_release(&path);\n+\treturn ret;\n+\n+}\n+\n /*\n  * Test if it looks like we're at a git directory.\n  * We want to see:\n@@ -412,42 +519,7 @@ static int validate_headref(const char *path)\n  */\n int is_git_directory(const char *suspect)\n {\n-\tstruct strbuf path = STRBUF_INIT;\n-\tint ret = 0;\n-\tsize_t len;\n-\n-\t/* Check worktree-related signatures */\n-\tstrbuf_addstr(&path, suspect);\n-\tstrbuf_complete(&path, '/');\n-\tstrbuf_addstr(&path, \"HEAD\");\n-\tif (validate_headref(path.buf))\n-\t\tgoto done;\n-\n-\tstrbuf_reset(&path);\n-\tget_common_dir(&path, suspect);\n-\tlen = path.len;\n-\n-\t/* Check non-worktree-related signatures */\n-\tif (getenv(DB_ENVIRONMENT)) {\n-\t\tif (access(getenv(DB_ENVIRONMENT), X_OK))\n-\t\t\tgoto done;\n-\t}\n-\telse {\n-\t\tstrbuf_setlen(&path, len);\n-\t\tstrbuf_addstr(&path, \"/objects\");\n-\t\tif (access(path.buf, X_OK))\n-\t\t\tgoto done;\n-\t}\n-\n-\tstrbuf_setlen(&path, len);\n-\tstrbuf_addstr(&path, \"/refs\");\n-\tif (access(path.buf, X_OK))\n-\t\tgoto done;\n-\n-\tret = 1;\n-done:\n-\tstrbuf_release(&path);\n-\treturn ret;\n+\treturn is_git_directory_verbose(suspect, NULL);\n }\n \n int is_nonbare_repository_dir(struct strbuf *path)\n-- \n2.56.0.rc1.12.g2c9c8d64bb\n\n"},{"id":"553173","messageId":"20260924120502.2642141-4-kaartic.sivaraam@gmail.com","threadId":"66383","inReplyTo":"20260924120502.2642141-1-kaartic.sivaraam@gmail.com","subject":"[RFC PATCH 3/3] setup: communicate why a directory is not a valid git directory","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-24T12:02:21Z","receivedAt":"2026-09-24T12:05:20Z","isPatch":true,"body":"At the moment, there are a few scenarios in which the error message\nsurrounding an invalid Git repository is a bit blunt:\n\n  $ GIT_OBJECT_DIRECTORY=/does/not/exist git --git-dir repo.git rev-parse --is-bare-repository\n  fatal: not a git repository: 'repo.git'\n\nIn this case, even though repo.git is a valid Git repository,\nwe get an output saying it is not since the GIT_OBJECT_DIRECTORY\ndoes not point to a valid object directory. At the moment, the\nuser is on their own in figuring this out.\n\nInstead, make it more easy for users to figure such issues\nparticularly in cases where they have explicitly specified\na Git directory. This intends to improve the error reporting UX\nby clarifying why the specified repository is not considered valid.\n\nWe achieve this by means of using the new helper\nis_git_directory_verbose() that has been introduced. With the\nsame, we get a more helpful error message as follows:\n\n  $ GIT_OBJECT_DIRECTORY=/does/not/exist git --git-dir repo.git rev-parse --is-bare-repository\n  fatal: not a git repository: 'repo.git'\n  reason: cannot access object directory '/does/not/exist' set via $GIT_OBJECT_DIRECTORY\n\nSigned-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n---\n setup.c                       | 13 +++++++++++--\n t/t0009-git-dir-validation.sh | 10 ++++++----\n 2 files changed, 17 insertions(+), 6 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex b3b53a1cfc..3e99141474 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -1225,6 +1225,7 @@ static void repo_discover_explicit_gitdir(struct repo_discovery *discovery,\n \t\t\t\t\t  int *nongit_ok)\n {\n \tconst char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);\n+\tstruct strbuf invalid_gitdir_reason = STRBUF_INIT;\n \tchar *gitfile;\n \tint offset;\n \n@@ -1237,12 +1238,19 @@ static void repo_discover_explicit_gitdir(struct repo_discovery *discovery,\n \t\tgitdirenv = gitfile;\n \t}\n \n-\tif (!is_git_directory(gitdirenv)) {\n+\tif (!is_git_directory_verbose(gitdirenv, &invalid_gitdir_reason)) {\n+\t\tstruct strbuf die_msg = STRBUF_INIT;\n \t\tif (nongit_ok) {\n \t\t\t*nongit_ok = 1;\n \t\t\tgoto out;\n \t\t}\n-\t\tdie(_(\"not a git repository: '%s'\"), gitdirenv);\n+\n+\t\tstrbuf_addf(&die_msg, _(\"not a git repository: '%s'\"), gitdirenv);\n+\t\tstrbuf_addch(&die_msg, '\\n');\n+\t\tstrbuf_addf(&die_msg, _(\"reason: %s\"), invalid_gitdir_reason.buf);\n+\t\tdie(\"%s\", die_msg.buf);\n+\n+\t\tstrbuf_release(&die_msg);\n \t}\n \n \tif (read_and_verify_repository_format(&discovery->format, gitdirenv, nongit_ok))\n@@ -1304,6 +1312,7 @@ static void repo_discover_explicit_gitdir(struct repo_discovery *discovery,\n \trepo_discovery_set_gitdir(discovery, gitdirenv, 0);\n \n out:\n+\tstrbuf_release(&invalid_gitdir_reason);\n \tfree(gitfile);\n }\n \ndiff --git a/t/t0009-git-dir-validation.sh b/t/t0009-git-dir-validation.sh\nindex 244dc07c0e..411aac1d9e 100755\n--- a/t/t0009-git-dir-validation.sh\n+++ b/t/t0009-git-dir-validation.sh\n@@ -79,7 +79,8 @@ test_expect_success 'setup: custom git directory with missing HEAD is rejected'\n \tmkdir -p parent/empty-dir &&\n \t(\n \t\ttest_must_fail git --git-dir parent/empty-dir rev-parse --is-bare-repository 2>stderr &&\n-\t\ttest_grep \"not a git repository\" stderr\n+\t\ttest_grep \"not a git repository\" stderr &&\n+\t\ttest_grep \"reason: could not stat HEAD at\" stderr\n \t)\n '\n \n@@ -93,7 +94,8 @@ test_expect_success 'setup: custom git directory with HEAD as a symlink outside\n \t\trm real-repo/HEAD &&\n \t\tln -s ../garbage real-repo/HEAD &&\n \t\ttest_must_fail git --git-dir real-repo rev-parse --is-bare-repository 2>stderr &&\n-\t\ttest_grep \"not a git repository\" stderr\n+\t\ttest_grep \"not a git repository\" stderr &&\n+\t\ttest_grep \"reason: HEAD is a symlink .* but target lives outside refs\" stderr\n \t)\n '\n \n@@ -105,9 +107,9 @@ test_expect_success 'setup: custom git directory with invalid GIT_OBJECT_DIRECTO\n \t\tgit init --bare real-repo &&\n \t\ttest_must_fail env GIT_OBJECT_DIRECTORY=\"$(pwd)/does-not-exist\" \\\n \t\t\tgit --git-dir real-repo rev-parse --is-bare-repository 2>stderr &&\n-\t\ttest_grep \"not a git repository\" stderr\n+\t\ttest_grep \"not a git repository\" stderr &&\n+\t\ttest_grep \"reason: cannot access object directory .* set via \\$GIT_OBJECT_DIRECTORY\"   stderr\n \t)\n '\n \n-\n test_done\n-- \n2.56.0.rc1.12.g2c9c8d64bb\n\n"},{"id":"553244","messageId":"xmqqjyoaz4ig.fsf@gitster.g","threadId":"66383","inReplyTo":"20260924120502.2642141-2-kaartic.sivaraam@gmail.com","subject":"Re: [RFC PATCH 1/3] t0009: add tests to cover more error reporting scenarios","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-24T22:08:39Z","receivedAt":"2026-09-24T22:08:41Z","isPatch":true,"body":"Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:\n\n> Introduce few more tests to t0009 to cover error reporting scenarios\n> when --git-dir is used.\n>\n> Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n> ---\n>  t/t0009-git-dir-validation.sh | 36 +++++++++++++++++++++++++++++++++++\n>  1 file changed, 36 insertions(+)\n>\n> diff --git a/t/t0009-git-dir-validation.sh b/t/t0009-git-dir-validation.sh\n> index 4cba478e50..244dc07c0e 100755\n> --- a/t/t0009-git-dir-validation.sh\n> +++ b/t/t0009-git-dir-validation.sh\n> @@ -74,4 +74,40 @@ test_expect_success 'setup: .git as an empty directory is ignored' '\n>  \t)\n>  '\n>  \n> +test_expect_success 'setup: custom git directory with missing HEAD is rejected' '\n> +\ttest_when_finished \"rm -rf parent/empty-dir\" &&\n> +\tmkdir -p parent/empty-dir &&\n> +\t(\n> +\t\ttest_must_fail git --git-dir parent/empty-dir rev-parse --is-bare-repository 2>stderr &&\n> +\t\ttest_grep \"not a git repository\" stderr\n> +\t)\n> +'\n\nWhy subshell?\n\n> +\n> +test_expect_success 'setup: custom git directory with HEAD as a symlink outside refs/ is rejected' '\n> +\ttest_when_finished \"rm -rf parent/head-as-link-to-garbage\" &&\n> +\tmkdir -p parent/head-as-link-to-garbage &&\n> +\t(\n> +\t\tcd parent/head-as-link-to-garbage &&\n> +\t\tgit init --bare real-repo &&\n> +\t\ttouch garbage &&\n> +\t\trm real-repo/HEAD &&\n> +\t\tln -s ../garbage real-repo/HEAD &&\n> +\t\ttest_must_fail git --git-dir real-repo rev-parse --is-bare-repository 2>stderr &&\n> +\t\ttest_grep \"not a git repository\" stderr\n> +\t)\n> +'\n> +\n> +test_expect_success 'setup: custom git directory with invalid GIT_OBJECT_DIRECTORY configuration is rejected' '\n> +\ttest_when_finished \"rm -rf parent/invalid-git-object-directory-config\" &&\n> +\tmkdir -p parent/invalid-git-object-directory-config &&\n> +\t(\n> +\t\tcd parent/invalid-git-object-directory-config &&\n> +\t\tgit init --bare real-repo &&\n> +\t\ttest_must_fail env GIT_OBJECT_DIRECTORY=\"$(pwd)/does-not-exist\" \\\n> +\t\t\tgit --git-dir real-repo rev-parse --is-bare-repository 2>stderr &&\n> +\t\ttest_grep \"not a git repository\" stderr\n> +\t)\n> +'\n> +\n> +\n>  test_done\n"},{"id":"553245","messageId":"xmqqcxu2z4cy.fsf@gitster.g","threadId":"66383","inReplyTo":"20260924120502.2642141-3-kaartic.sivaraam@gmail.com","subject":"Re: [RFC PATCH 2/3] setup: introduce new helper 'is_git_directory_verbose'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-24T22:11:57Z","receivedAt":"2026-09-24T22:11:59Z","isPatch":true,"body":"Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:\n\n> Introduce a new helper is_git_directory_verbose() as a\n> counterpart to the existing is_git_directory().\n>\n> is_git_directory_verbose() also populates an optional\n> string strbuf with reasoning around why the given suspect\n> is not a valid git directory. This strbuf in turn can be\n> used to improve the error reporting which is currently blunt:\n>\n>   fatal: not a git repository\n>\n> This is not helpful as the user does not get any hint about \"why\"\n> the repository is not considered valid.\n>\n> Call-site(s) will be made to use this helper in a follow-up commit.\n>\n> Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n> ---\n>  setup.c | 152 +++++++++++++++++++++++++++++++++++++++++---------------\n>  1 file changed, 112 insertions(+), 40 deletions(-)\n>\n> diff --git a/setup.c b/setup.c\n> index 0d157ac254..b3b53a1cfc 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -347,7 +347,7 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)\n>  \treturn ret;\n>  }\n>  \n> -static int validate_headref(const char *path)\n> +static int validate_headref(const char *path, struct strbuf *err)\n>  {\n>  \tstruct stat st;\n>  \tchar buffer[256];\n> @@ -356,14 +356,32 @@ static int validate_headref(const char *path)\n>  \tint fd;\n>  \tssize_t len;\n>  \n> -\tif (lstat(path, &st) < 0)\n> +\tif (lstat(path, &st) < 0) {\n> +\t\tif (err)\n> +\t\t\tstrbuf_addf(\n> +\t\t\t\terr, _(\"could not stat HEAD at '%s'\"), path\n> +\t\t\t);\n>  \t\treturn -1;\n> +\t}\n>  \n>  \t/* Make sure it is a \"refs/..\" symlink */\n>  \tif (S_ISLNK(st.st_mode)) {\n>  \t\tlen = readlink(path, buffer, sizeof(buffer)-1);\n>  \t\tif (len >= 5 && !memcmp(\"refs/\", buffer, 5))\n>  \t\t\treturn 0;\n> +\t\tif (len == -1 && err)\n> +\t\t\tstrbuf_addf(\n> +\t\t\t\terr,\n> +\t\t\t\t_(\"could not read the symlink HEAD at '%s'\"),\n> +\t\t\t\tpath\n> +\t\t\t);\n> +\t\telse if (err)\n> +\t\t\tstrbuf_addf(\n> +\t\t\t\terr,\n> +\t\t\t\t_(\"HEAD is a symlink ('%s') but target\"\n> +\t\t\t\t  \" lives outside refs/\"),\n> +\t\t\t\tpath\n> +\t\t\t);\n>  \t\treturn -1;\n>  \t}\n\nAll of the above (and below---ellided) look fairly funny way to\nindent them.  If you are trying ot match the style used in the\nexisting code around the same area, I wouldn't complain, but I\ndidn't look beyond what is visible in the patch.\n\n> +\t}\n> +\telse {\n\nStyle: \"} else {\" go on a single line.\n"},{"id":"553246","messageId":"xmqq7bkaz45e.fsf@gitster.g","threadId":"66383","inReplyTo":"20260924120502.2642141-4-kaartic.sivaraam@gmail.com","subject":"Re: [RFC PATCH 3/3] setup: communicate why a directory is not a valid git directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-24T22:16:29Z","receivedAt":"2026-09-24T22:16:32Z","isPatch":true,"body":"Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:\n\n> +\t\tstrbuf_addf(&die_msg, _(\"not a git repository: '%s'\"), gitdirenv);\n> +\t\tstrbuf_addch(&die_msg, '\\n');\n> +\t\tstrbuf_addf(&die_msg, _(\"reason: %s\"), invalid_gitdir_reason.buf);\n> +\t\tdie(\"%s\", die_msg.buf);\n> +\n> +\t\tstrbuf_release(&die_msg);\n\nYou just called die(); nobody will execute this strbuf_release() for\nyou, and because die() is marked with NORETURN, smart enough compilers\nwould scold you for introducing dead code.\n\nWhy are you lego-assembling localized message yourself, instead of\ndoing something like ...\n\n\tdie(_(\"not a git repository: '%s'\\nreason: %s\"),\n\t    gitdirenv, invalid_gitdir_reason.buf);\n\n... which is what is usually done?\n\n"},{"id":"553282","messageId":"b024b447-4c63-494c-8ffc-f700fb3c7c46@gmail.com","threadId":"66383","inReplyTo":"xmqqjyoaz4ig.fsf@gitster.g","subject":"Re: [RFC PATCH 1/3] t0009: add tests to cover more error reporting scenarios","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-25T13:46:18Z","receivedAt":"2026-09-25T13:46:22Z","isPatch":true,"body":"On 9/25/26 03:38, Junio C Hamano wrote:\n> Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:\n> \n>> Introduce few more tests to t0009 to cover error reporting scenarios\n>> when --git-dir is used.\n>>\n>> Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n>> ---\n>>   t/t0009-git-dir-validation.sh | 36 +++++++++++++++++++++++++++++++++++\n>>   1 file changed, 36 insertions(+)\n>>\n>> diff --git a/t/t0009-git-dir-validation.sh b/t/t0009-git-dir-validation.sh\n>> index 4cba478e50..244dc07c0e 100755\n>> --- a/t/t0009-git-dir-validation.sh\n>> +++ b/t/t0009-git-dir-validation.sh\n>> @@ -74,4 +74,40 @@ test_expect_success 'setup: .git as an empty directory is ignored' '\n>>   \t)\n>>   '\n>>   \n>> +test_expect_success 'setup: custom git directory with missing HEAD is rejected' '\n>> +\ttest_when_finished \"rm -rf parent/empty-dir\" &&\n>> +\tmkdir -p parent/empty-dir &&\n>> +\t(\n>> +\t\ttest_must_fail git --git-dir parent/empty-dir rev-parse --is-bare-repository 2>stderr &&\n>> +\t\ttest_grep \"not a git repository\" stderr\n>> +\t)\n>> +'\n> \n> Why subshell?\n>\n\nGood catch. It is unnecessary. An earlier iteration used to cd into \nparent/empty-dir. This is no longer the case. So, I'll avoid the \nsub-shell in this test.\n\n-- \nSivaraam\n\n"},{"id":"553284","messageId":"b27162ed-0c62-4ef0-b125-4340517e0d50@gmail.com","threadId":"66383","inReplyTo":"xmqq7bkaz45e.fsf@gitster.g","subject":"Re: [RFC PATCH 3/3] setup: communicate why a directory is not a valid git directory","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-25T14:33:24Z","receivedAt":"2026-09-25T14:33:32Z","isPatch":true,"body":"On 9/25/26 03:46, Junio C Hamano wrote:\n> Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:\n> \n>> +\t\tstrbuf_addf(&die_msg, _(\"not a git repository: '%s'\"), gitdirenv);\n>> +\t\tstrbuf_addch(&die_msg, '\\n');\n>> +\t\tstrbuf_addf(&die_msg, _(\"reason: %s\"), invalid_gitdir_reason.buf);\n>> +\t\tdie(\"%s\", die_msg.buf);\n>> +\n>> +\t\tstrbuf_release(&die_msg);\n> \n> You just called die(); nobody will execute this strbuf_release() for\n> you, and because die() is marked with NORETURN, smart enough compilers\n> would scold you for introducing dead code.\n>\n\nOops. It should indeed warn me. I did a quick check and seems gcc (which \nI was using) lost its ability to report this long ago[1]. Sad. Clang \nclearly reports this.\n\n[1]: https://gcc.gnu.org/legacy-ml/gcc-help/2011-05/msg00360.html\n\n> Why are you lego-assembling localized message yourself, instead of\n> doing something like ...\n> \n> \tdie(_(\"not a git repository: '%s'\\nreason: %s\"),\n> \t    gitdirenv, invalid_gitdir_reason.buf);\n> \n> ... which is what is usually done?\n> \n\nThat was my attempt at trying to save translators some work of \ntranslating the \"not a git repository\" part. I was thinking this lego \nwas not a problem as it was not so complex. If we just want to avoid \nlegos totally, I'm cool with doing it the usual way.\n\n-- \nSivaraam\n\n"},{"id":"553306","messageId":"c7808d4b-36f8-4583-8836-7b7d8bc24905@gmail.com","threadId":"66383","inReplyTo":"xmqqcxu2z4cy.fsf@gitster.g","subject":"Re: [RFC PATCH 2/3] setup: introduce new helper 'is_git_directory_verbose'","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-25T17:51:08Z","receivedAt":"2026-09-25T17:51:14Z","isPatch":true,"body":"On 9/25/26 03:41, Junio C Hamano wrote:\n> Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:\n> \n>> diff --git a/setup.c b/setup.c\n>> index 0d157ac254..b3b53a1cfc 100644\n>>   \n>>   \t/* Make sure it is a \"refs/..\" symlink */\n>>   \tif (S_ISLNK(st.st_mode)) {\n>>   \t\tlen = readlink(path, buffer, sizeof(buffer)-1);\n>>   \t\tif (len >= 5 && !memcmp(\"refs/\", buffer, 5))\n>>   \t\t\treturn 0;\n>> +\t\tif (len == -1 && err)\n>> +\t\t\tstrbuf_addf(\n>> +\t\t\t\terr,\n>> +\t\t\t\t_(\"could not read the symlink HEAD at '%s'\"),\n>> +\t\t\t\tpath\n>> +\t\t\t);\n>> +\t\telse if (err)\n>> +\t\t\tstrbuf_addf(\n>> +\t\t\t\terr,\n>> +\t\t\t\t_(\"HEAD is a symlink ('%s') but target\"\n>> +\t\t\t\t  \" lives outside refs/\"),\n>> +\t\t\t\tpath\n>> +\t\t\t);\n>>   \t\treturn -1;\n>>   \t}\n> \n> All of the above (and below---ellided) look fairly funny way to\n> indent them.  If you are trying ot match the style used in the\n> existing code around the same area, I wouldn't complain, but I\n> didn't look beyond what is visible in the patch.\n> \n\nIndeed. My bad. Does the following look like a good indentation style \nfor shorter messages?\n\n\tif (lstat(path, &st) < 0) {\n\t\tif (err)\n\t\t\tstrbuf_addf(err, _(\"could not stat HEAD at '%s'\"), path);\n\t\treturn -1;\n          }\n\n... and this for messages that are a bit longer:\n\n\tif (len == -1 && err)\n\t\tstrbuf_addf(err, _(\"could not read the symlink HEAD at '%s'\"),\n\t\t\t    path);\n\telse if (err)\n\t\tstrbuf_addf(err, _(\"HEAD is a symlink ('%s') but target lives\"\n\t\t\t\t   \" outside refs/\"), path);\n\n\n-- \nSivaraam\n\nPS: I'm not yet very sure if my MUA will send this as intended. Will \nresend if it doesn't. Excuse the noise in advance.\n\n"},{"id":"553574","messageId":"20260929102513.712181-2-kaartic.sivaraam@gmail.com","threadId":"66383","inReplyTo":"20260929102513.712181-1-kaartic.sivaraam@gmail.com","subject":"[RFC PATCH v2 1/4] setup: normalize an if-else to follow our convention","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-29T10:25:07Z","receivedAt":"2026-09-29T10:25:24Z","isPatch":true,"body":"The else case was not stuck with the corresponding if's\nclosing brace which is not in-line with our convention.\nFix the same.\n\nThis is a style-only change; no behaviour change intended.\n\nSigned-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n---\n setup.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex 0d157ac254..e9a9ecda19 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -1879,8 +1879,7 @@ const char *enter_repo(struct repository *repo, const char *path, unsigned flags\n \t\tif (chdir(used_path.buf))\n \t\t\treturn NULL;\n \t\tpath = validated_path.buf;\n-\t}\n-\telse {\n+\t} else {\n \t\tconst char *gitfile = read_gitfile(path);\n \t\tif (!(flags & ENTER_REPO_ANY_OWNER_OK))\n \t\t\tdie_upon_dubious_ownership(gitfile, NULL, path);\n-- \n2.56.0.rc1.12.g2c9c8d64bb\n\n"},{"id":"553576","messageId":"20260929102513.712181-1-kaartic.sivaraam@gmail.com","threadId":"66383","inReplyTo":"20260924120502.2642141-1-kaartic.sivaraam@gmail.com","subject":"[RFC PATCH v2 0/4] Improve error reporting to mention \"why\" a directory is not a repository","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-29T10:25:06Z","receivedAt":"2026-09-29T10:25:24Z","isPatch":true,"body":"At the moment, there are a few scenarios where the error message for an\ninvalid Git repository is a bit blunt. For instance, when we point\nGIT_OBJECT_DIRECTORY at a directory that does not exist, we get this:\n\n  $ GIT_OBJECT_DIRECTORY=/does/not/exist git --git-dir repo.git rev-parse --is-bare-repository\n  fatal: not a git repository: 'repo.git'\n\nEven though repo.git itself is a valid repository, we get this rather\npuzzling error saying that it is not. The actual problem is the invalid\nvalue given to GIT_OBJECT_DIRECTORY, and the user is left on their\nto figure that out. This series aims to make such issues easier to\ndiagnose by saying why the specified repository was not considered\nvalid.\n\nWith this series, the same command outputs:\n\n  $ GIT_OBJECT_DIRECTORY=/does/not/exist git --git-dir repo.git rev-parse --is-bare-repository\n  fatal: not a git repository: 'repo.git'\n  reason: cannot access object directory '/does/not/exist' set via $GIT_OBJECT_DIRECTORY\n\nThe following scenarios are covered when --git-dir is given explicitly:\n\n  - HEAD is missing, or its path is not traversable\n  - HEAD is a symlink that cannot be read\n  - HEAD is a symlink whose target lives outside refs/\n  - HEAD cannot be opened, or cannot be read\n  - HEAD contains neither a ref under refs/ nor an object ID\n  - $GIT_OBJECT_DIRECTORY is set to something we cannot access\n  - the object directory in the common directory is inaccessible\n  - the refs directory in the common directory is inaccessible\n\nThis series does not yet report a reason in the following cases.\n\nThe discovery walk, where we iterate up to the ceiling or the mount\npoint looking for a repository:\n\n  $ GIT_OBJECT_DIRECTORY=/does/not/exist git rev-parse --is-bare-repository\n  fatal: not a git repository (or any parent up to mount point /)\n  Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set)\n\nThe gitfile case, for instance when run from inside a submodule /\nworktree:\n\n  $ GIT_OBJECT_DIRECTORY=/does/not/exist git rev-parse --is-bare-repository\n  fatal: gitfile does not point to a valid repository: /path/to/super/sub/.git\n\nGIT_ALTERNATE_OBJECT_DIRECTORIES is also left alone. Its diagnostic\nare misleading in their own way, but they are emitted much later, from\nthe object database rather than from setup, so they need a separate\ntreatment.\n\nI would be very interested in hearing what others think about this\ndirection. If it looks reasonable, I'm happy to cover the remaining\ncases too, either in this series or separately.\n\nCI:\n\nOne symlink related test appears to be failing in Windows. I'll check the same.\nAll others pass.\n\nSee here: https://github.com/sivaraam/git/actions/runs/36548531586\n\nChanges since v2:\n\n- Included 1/4 which fixes a style nit\n- Dropped the sub-shell from the test that was unnecessary\n- Fixed some style issue and tried to improve the code intentation corresponding\n  to the strbuf construction code\n- Replaced sentence lego with a single string even though that means\n  additional work for translators. On the plus side, avoiding sentence\n  lego provides them more context about the message.\n\nRange-diff between v1 and v2\n\n-:  ---------- > 1:  17b72331d0 setup: normalize an if-else to follow our convention\n1:  b4d7a4efa2 ! 2:  962418c9f3 t0009: add tests to cover more error reporting scenarios\n    @@ t/t0009-git-dir-validation.sh: test_expect_success 'setup: .git as an empty dire\n     +test_expect_success 'setup: custom git directory with missing HEAD is rejected' '\n     +\ttest_when_finished \"rm -rf parent/empty-dir\" &&\n     +\tmkdir -p parent/empty-dir &&\n    -+\t(\n    -+\t\ttest_must_fail git --git-dir parent/empty-dir rev-parse --is-bare-repository 2>stderr &&\n    -+\t\ttest_grep \"not a git repository\" stderr\n    -+\t)\n    ++\ttest_must_fail git --git-dir parent/empty-dir rev-parse --is-bare-repository 2>stderr &&\n    ++\ttest_grep \"not a git repository\" stderr\n     +'\n     +\n     +test_expect_success 'setup: custom git directory with HEAD as a symlink outside refs/ is rejected' '\n2:  8591dc4162 ! 3:  4a1503a6dc setup: introduce new helper 'is_git_directory_verbose'\n    @@ setup.c: static int validate_headref(const char *path)\n     -\tif (lstat(path, &st) < 0)\n     +\tif (lstat(path, &st) < 0) {\n     +\t\tif (err)\n    -+\t\t\tstrbuf_addf(\n    -+\t\t\t\terr, _(\"could not stat HEAD at '%s'\"), path\n    -+\t\t\t);\n    ++\t\t\tstrbuf_addf(err, _(\"could not stat HEAD at '%s'\"), path);\n      \t\treturn -1;\n     +\t}\n      \n    @@ setup.c: static int validate_headref(const char *path)\n      \t\tif (len >= 5 && !memcmp(\"refs/\", buffer, 5))\n      \t\t\treturn 0;\n     +\t\tif (len == -1 && err)\n    -+\t\t\tstrbuf_addf(\n    -+\t\t\t\terr,\n    -+\t\t\t\t_(\"could not read the symlink HEAD at '%s'\"),\n    -+\t\t\t\tpath\n    -+\t\t\t);\n    ++\t\t\tstrbuf_addf(err, _(\"could not read the symlink HEAD at '%s'\"),\n    ++\t\t\t\t    path);\n     +\t\telse if (err)\n    -+\t\t\tstrbuf_addf(\n    -+\t\t\t\terr,\n    -+\t\t\t\t_(\"HEAD is a symlink ('%s') but target\"\n    -+\t\t\t\t  \" lives outside refs/\"),\n    -+\t\t\t\tpath\n    -+\t\t\t);\n    ++\t\t\tstrbuf_addf(err, _(\"HEAD is a symlink ('%s') but target lives\"\n    ++\t\t\t\t\t   \" outside refs/\"), path);\n      \t\treturn -1;\n      \t}\n      \n    @@ setup.c: static int validate_headref(const char *path)\n     -\tif (fd < 0)\n     +\tif (fd < 0) {\n     +\t\tif (err)\n    -+\t\t\tstrbuf_addf(\n    -+\t\t\t\terr, _(\"could not open HEAD at '%s'\"), path\n    -+\t\t\t);\n    ++\t\t\tstrbuf_addf(err, _(\"could not open HEAD at '%s'\"), path);\n      \t\treturn -1;\n     +\t}\n      \tlen = read_in_full(fd, buffer, sizeof(buffer)-1);\n    @@ setup.c: static int validate_headref(const char *path)\n     -\tif (len < 0)\n     +\tif (len < 0) {\n     +\t\tif (err)\n    -+\t\t\tstrbuf_addf(\n    -+\t\t\t\terr, _(\"could not read HEAD at '%s'\"), path\n    -+\t\t\t);\n    ++\t\t\tstrbuf_addf(err, _(\"could not read HEAD at '%s'\"), path);\n      \t\treturn -1;\n     +\t}\n      \tbuffer[len] = '\\0';\n    @@ setup.c: static int validate_headref(const char *path)\n      \t\treturn 0;\n      \n     +\tif (err)\n    -+\t\tstrbuf_addf(\n    -+\t\t\terr,\n    -+\t\t\t_(\"HEAD at '%s' does not point to a valid\"\n    -+\t\t\t  \" symbolic link or an object ID\"),\n    -+\t\t\tpath\n    -+\t\t);\n    ++\t\tstrbuf_addf(err, _(\"HEAD at '%s' does not point to a valid symbolic\"\n    ++\t\t\t\t   \" link or an object ID\"), path);\n     +\n      \treturn -1;\n      }\n    @@ setup.c: static int validate_headref(const char *path)\n     +\tif (objdir) {\n     +\t\tif (access(objdir, X_OK)) {\n     +\t\t\tif (err)\n    -+\t\t\t\tstrbuf_addf(\n    -+\t\t\t\t\terr,\n    -+\t\t\t\t\t_(\"cannot access object directory '%s'\"\n    -+\t\t\t\t\t  \" set via $%s\\n\"),\n    -+\t\t\t\tobjdir,\n    -+\t\t\t\tDB_ENVIRONMENT\n    -+\t\t\t);\n    ++\t\t\t\tstrbuf_addf(err, _(\"cannot access object directory '%s'\"\n    ++\t\t\t\t\t\t   \"set via $%s\\n\"), objdir, DB_ENVIRONMENT);\n     +\t\t\tgoto done;\n     +\t\t}\n    -+\t}\n    -+\telse {\n    ++\t} else {\n     +\t\tstrbuf_setlen(&path, len);\n     +\t\tstrbuf_addstr(&path, \"/objects\");\n     +\t\tif (access(path.buf, X_OK)) {\n     +\t\t\tif (err)\n    -+\t\t\t\tstrbuf_addf(\n    -+\t\t\t\t\terr,\n    -+\t\t\t\t\t_(\"cannot access object directory '%s'\"),\n    -+\t\t\t\t\tpath.buf\n    -+\t\t\t\t);\n    ++\t\t\t\tstrbuf_addf(err, _(\"cannot access object directory '%s'\"),\n    ++\t\t\t\t\t    path.buf);\n     +\t\t\tgoto done;\n     +\t\t}\n     +\t}\n    @@ setup.c: static int validate_headref(const char *path)\n     +\tstrbuf_addstr(&path, \"/refs\");\n     +\tif (access(path.buf, X_OK)) {\n     +\t\tif (err)\n    -+\t\t\tstrbuf_addf(\n    -+\t\t\t\terr,\n    -+\t\t\t\t_(\"cannot access refs directory '%s'\"),\n    -+\t\t\t\tpath.buf\n    -+\t\t\t);\n    ++\t\t\tstrbuf_addf(err, _(\"cannot access refs directory '%s'\"), path.buf);\n     +\t\tgoto done;\n     +\t}\n     +\n3:  2c9c8d64bb ! 4:  27a9f668ab setup: communicate why a directory is not a valid git directory\n    @@ setup.c: static void repo_discover_explicit_gitdir(struct repo_discovery *discov\n      \t\t}\n     -\t\tdie(_(\"not a git repository: '%s'\"), gitdirenv);\n     +\n    -+\t\tstrbuf_addf(&die_msg, _(\"not a git repository: '%s'\"), gitdirenv);\n    -+\t\tstrbuf_addch(&die_msg, '\\n');\n    -+\t\tstrbuf_addf(&die_msg, _(\"reason: %s\"), invalid_gitdir_reason.buf);\n    ++\t\tstrbuf_addf(&die_msg, _(\"not a git repository: '%s'\\nreason: %s\"),\n    ++\t\t\t    gitdirenv, invalid_gitdir_reason.buf);\n     +\t\tdie(\"%s\", die_msg.buf);\n    -+\n    -+\t\tstrbuf_release(&die_msg);\n      \t}\n      \n      \tif (read_and_verify_repository_format(&discovery->format, gitdirenv, nongit_ok))\n    @@ setup.c: static void repo_discover_explicit_gitdir(struct repo_discovery *discov\n     \n      ## t/t0009-git-dir-validation.sh ##\n     @@ t/t0009-git-dir-validation.sh: test_expect_success 'setup: custom git directory with missing HEAD is rejected'\n    + \ttest_when_finished \"rm -rf parent/empty-dir\" &&\n      \tmkdir -p parent/empty-dir &&\n    - \t(\n    - \t\ttest_must_fail git --git-dir parent/empty-dir rev-parse --is-bare-repository 2>stderr &&\n    --\t\ttest_grep \"not a git repository\" stderr\n    -+\t\ttest_grep \"not a git repository\" stderr &&\n    -+\t\ttest_grep \"reason: could not stat HEAD at\" stderr\n    - \t)\n    + \ttest_must_fail git --git-dir parent/empty-dir rev-parse --is-bare-repository 2>stderr &&\n    +-\ttest_grep \"not a git repository\" stderr\n    ++\ttest_grep \"not a git repository\" stderr &&\n    ++\ttest_grep \"reason: could not stat HEAD at\" stderr\n      '\n      \n    + test_expect_success 'setup: custom git directory with HEAD as a symlink outside refs/ is rejected' '\n     @@ t/t0009-git-dir-validation.sh: test_expect_success 'setup: custom git directory with HEAD as a symlink outside\n      \t\trm real-repo/HEAD &&\n      \t\tln -s ../garbage real-repo/HEAD &&\n\nKaartic Sivaraam (4):\n  setup: normalize an if-else to follow our convention\n  t0009: add tests to cover more error reporting scenarios\n  setup: introduce new helper 'is_git_directory_verbose'\n  setup: communicate why a directory is not a valid git directory\n\n setup.c                       | 135 +++++++++++++++++++++++-----------\n t/t0009-git-dir-validation.sh |  36 +++++++++\n 2 files changed, 127 insertions(+), 44 deletions(-)\n\n-- \n2.56.0.rc1.12.g2c9c8d64bb\n\n"},{"id":"553575","messageId":"20260929102513.712181-3-kaartic.sivaraam@gmail.com","threadId":"66383","inReplyTo":"20260929102513.712181-1-kaartic.sivaraam@gmail.com","subject":"[RFC PATCH v2 2/4] t0009: add tests to cover more error reporting scenarios","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-29T10:25:08Z","receivedAt":"2026-09-29T10:25:27Z","isPatch":true,"body":"Introduce few more tests to t0009 to cover error reporting scenarios\nwhen --git-dir is used.\n\nSigned-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n---\n t/t0009-git-dir-validation.sh | 34 ++++++++++++++++++++++++++++++++++\n 1 file changed, 34 insertions(+)\n\ndiff --git a/t/t0009-git-dir-validation.sh b/t/t0009-git-dir-validation.sh\nindex 4cba478e50..6c40925aa4 100755\n--- a/t/t0009-git-dir-validation.sh\n+++ b/t/t0009-git-dir-validation.sh\n@@ -74,4 +74,38 @@ test_expect_success 'setup: .git as an empty directory is ignored' '\n \t)\n '\n \n+test_expect_success 'setup: custom git directory with missing HEAD is rejected' '\n+\ttest_when_finished \"rm -rf parent/empty-dir\" &&\n+\tmkdir -p parent/empty-dir &&\n+\ttest_must_fail git --git-dir parent/empty-dir rev-parse --is-bare-repository 2>stderr &&\n+\ttest_grep \"not a git repository\" stderr\n+'\n+\n+test_expect_success 'setup: custom git directory with HEAD as a symlink outside refs/ is rejected' '\n+\ttest_when_finished \"rm -rf parent/head-as-link-to-garbage\" &&\n+\tmkdir -p parent/head-as-link-to-garbage &&\n+\t(\n+\t\tcd parent/head-as-link-to-garbage &&\n+\t\tgit init --bare real-repo &&\n+\t\ttouch garbage &&\n+\t\trm real-repo/HEAD &&\n+\t\tln -s ../garbage real-repo/HEAD &&\n+\t\ttest_must_fail git --git-dir real-repo rev-parse --is-bare-repository 2>stderr &&\n+\t\ttest_grep \"not a git repository\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: custom git directory with invalid GIT_OBJECT_DIRECTORY configuration is rejected' '\n+\ttest_when_finished \"rm -rf parent/invalid-git-object-directory-config\" &&\n+\tmkdir -p parent/invalid-git-object-directory-config &&\n+\t(\n+\t\tcd parent/invalid-git-object-directory-config &&\n+\t\tgit init --bare real-repo &&\n+\t\ttest_must_fail env GIT_OBJECT_DIRECTORY=\"$(pwd)/does-not-exist\" \\\n+\t\t\tgit --git-dir real-repo rev-parse --is-bare-repository 2>stderr &&\n+\t\ttest_grep \"not a git repository\" stderr\n+\t)\n+'\n+\n+\n test_done\n-- \n2.56.0.rc1.12.g2c9c8d64bb\n\n"},{"id":"553577","messageId":"20260929102513.712181-4-kaartic.sivaraam@gmail.com","threadId":"66383","inReplyTo":"20260929102513.712181-1-kaartic.sivaraam@gmail.com","subject":"[RFC PATCH v2 3/4] setup: introduce new helper 'is_git_directory_verbose'","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-29T10:25:09Z","receivedAt":"2026-09-29T10:25:27Z","isPatch":true,"body":"Introduce a new helper is_git_directory_verbose() as a\ncounterpart to the existing is_git_directory().\n\nis_git_directory_verbose() also populates an optional\nstring strbuf with reasoning around why the given suspect\nis not a valid git directory. This strbuf in turn can be\nused to improve the error reporting which is currently blunt:\n\n  fatal: not a git repository\n\nThis is not helpful as the user does not get any hint about \"why\"\nthe repository is not considered valid.\n\nCall-site(s) will be made to use this helper in a follow-up commit.\n\nSigned-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n---\n setup.c | 122 +++++++++++++++++++++++++++++++++++++-------------------\n 1 file changed, 82 insertions(+), 40 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex e9a9ecda19..a0fb68f7f6 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -347,7 +347,7 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)\n \treturn ret;\n }\n \n-static int validate_headref(const char *path)\n+static int validate_headref(const char *path, struct strbuf *err)\n {\n \tstruct stat st;\n \tchar buffer[256];\n@@ -356,14 +356,23 @@ static int validate_headref(const char *path)\n \tint fd;\n \tssize_t len;\n \n-\tif (lstat(path, &st) < 0)\n+\tif (lstat(path, &st) < 0) {\n+\t\tif (err)\n+\t\t\tstrbuf_addf(err, _(\"could not stat HEAD at '%s'\"), path);\n \t\treturn -1;\n+\t}\n \n \t/* Make sure it is a \"refs/..\" symlink */\n \tif (S_ISLNK(st.st_mode)) {\n \t\tlen = readlink(path, buffer, sizeof(buffer)-1);\n \t\tif (len >= 5 && !memcmp(\"refs/\", buffer, 5))\n \t\t\treturn 0;\n+\t\tif (len == -1 && err)\n+\t\t\tstrbuf_addf(err, _(\"could not read the symlink HEAD at '%s'\"),\n+\t\t\t\t    path);\n+\t\telse if (err)\n+\t\t\tstrbuf_addf(err, _(\"HEAD is a symlink ('%s') but target lives\"\n+\t\t\t\t\t   \" outside refs/\"), path);\n \t\treturn -1;\n \t}\n \n@@ -371,13 +380,19 @@ static int validate_headref(const char *path)\n \t * Anything else, just open it and try to see if it is a symbolic ref.\n \t */\n \tfd = open(path, O_RDONLY);\n-\tif (fd < 0)\n+\tif (fd < 0) {\n+\t\tif (err)\n+\t\t\tstrbuf_addf(err, _(\"could not open HEAD at '%s'\"), path);\n \t\treturn -1;\n+\t}\n \tlen = read_in_full(fd, buffer, sizeof(buffer)-1);\n \tclose(fd);\n \n-\tif (len < 0)\n+\tif (len < 0) {\n+\t\tif (err)\n+\t\t\tstrbuf_addf(err, _(\"could not read HEAD at '%s'\"), path);\n \t\treturn -1;\n+\t}\n \tbuffer[len] = '\\0';\n \n \t/*\n@@ -396,9 +411,71 @@ static int validate_headref(const char *path)\n \tif (get_oid_hex_any(buffer, &oid) != GIT_HASH_UNKNOWN)\n \t\treturn 0;\n \n+\tif (err)\n+\t\tstrbuf_addf(err, _(\"HEAD at '%s' does not point to a valid symbolic\"\n+\t\t\t\t   \" link or an object ID\"), path);\n+\n \treturn -1;\n }\n \n+/*\n+ * A variant of is_git_directory that gives additional\n+ * context via 'err' about why a given suspect is not\n+ * a valid git repository.\n+ */\n+static int is_git_directory_verbose(const char *suspect, struct strbuf *err)\n+{\n+\tstruct strbuf path = STRBUF_INIT;\n+\tchar *objdir;\n+\tint ret = 0;\n+\tsize_t len;\n+\n+\t/* Check worktree-related signatures */\n+\tstrbuf_addstr(&path, suspect);\n+\tstrbuf_complete(&path, '/');\n+\tstrbuf_addstr(&path, \"HEAD\");\n+\tif (validate_headref(path.buf, err))\n+\t\tgoto done;\n+\n+\tstrbuf_reset(&path);\n+\tget_common_dir(&path, suspect);\n+\tlen = path.len;\n+\n+\t/* Check non-worktree-related signatures */\n+\tobjdir = getenv(DB_ENVIRONMENT);\n+\tif (objdir) {\n+\t\tif (access(objdir, X_OK)) {\n+\t\t\tif (err)\n+\t\t\t\tstrbuf_addf(err, _(\"cannot access object directory '%s'\"\n+\t\t\t\t\t\t   \" set via $%s\\n\"), objdir, DB_ENVIRONMENT);\n+\t\t\tgoto done;\n+\t\t}\n+\t} else {\n+\t\tstrbuf_setlen(&path, len);\n+\t\tstrbuf_addstr(&path, \"/objects\");\n+\t\tif (access(path.buf, X_OK)) {\n+\t\t\tif (err)\n+\t\t\t\tstrbuf_addf(err, _(\"cannot access object directory '%s'\"),\n+\t\t\t\t\t    path.buf);\n+\t\t\tgoto done;\n+\t\t}\n+\t}\n+\n+\tstrbuf_setlen(&path, len);\n+\tstrbuf_addstr(&path, \"/refs\");\n+\tif (access(path.buf, X_OK)) {\n+\t\tif (err)\n+\t\t\tstrbuf_addf(err, _(\"cannot access refs directory '%s'\"), path.buf);\n+\t\tgoto done;\n+\t}\n+\n+\tret = 1;\n+done:\n+\tstrbuf_release(&path);\n+\treturn ret;\n+\n+}\n+\n /*\n  * Test if it looks like we're at a git directory.\n  * We want to see:\n@@ -412,42 +489,7 @@ static int validate_headref(const char *path)\n  */\n int is_git_directory(const char *suspect)\n {\n-\tstruct strbuf path = STRBUF_INIT;\n-\tint ret = 0;\n-\tsize_t len;\n-\n-\t/* Check worktree-related signatures */\n-\tstrbuf_addstr(&path, suspect);\n-\tstrbuf_complete(&path, '/');\n-\tstrbuf_addstr(&path, \"HEAD\");\n-\tif (validate_headref(path.buf))\n-\t\tgoto done;\n-\n-\tstrbuf_reset(&path);\n-\tget_common_dir(&path, suspect);\n-\tlen = path.len;\n-\n-\t/* Check non-worktree-related signatures */\n-\tif (getenv(DB_ENVIRONMENT)) {\n-\t\tif (access(getenv(DB_ENVIRONMENT), X_OK))\n-\t\t\tgoto done;\n-\t}\n-\telse {\n-\t\tstrbuf_setlen(&path, len);\n-\t\tstrbuf_addstr(&path, \"/objects\");\n-\t\tif (access(path.buf, X_OK))\n-\t\t\tgoto done;\n-\t}\n-\n-\tstrbuf_setlen(&path, len);\n-\tstrbuf_addstr(&path, \"/refs\");\n-\tif (access(path.buf, X_OK))\n-\t\tgoto done;\n-\n-\tret = 1;\n-done:\n-\tstrbuf_release(&path);\n-\treturn ret;\n+\treturn is_git_directory_verbose(suspect, NULL);\n }\n \n int is_nonbare_repository_dir(struct strbuf *path)\n-- \n2.56.0.rc1.12.g2c9c8d64bb\n\n"},{"id":"553578","messageId":"20260929102513.712181-5-kaartic.sivaraam@gmail.com","threadId":"66383","inReplyTo":"20260929102513.712181-1-kaartic.sivaraam@gmail.com","subject":"[RFC PATCH v2 4/4] setup: communicate why a directory is not a valid git directory","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-09-29T10:25:10Z","receivedAt":"2026-09-29T10:25:31Z","isPatch":true,"body":"At the moment, there are a few scenarios in which the error message\nsurrounding an invalid Git repository is a bit blunt:\n\n  $ GIT_OBJECT_DIRECTORY=/does/not/exist git --git-dir repo.git rev-parse --is-bare-repository\n  fatal: not a git repository: 'repo.git'\n\nIn this case, even though repo.git is a valid Git repository,\nwe get an output saying it is not since the GIT_OBJECT_DIRECTORY\ndoes not point to a valid object directory. At the moment, the\nuser is on their own in figuring this out.\n\nInstead, make it more easy for users to figure such issues\nparticularly in cases where they have explicitly specified\na Git directory. This intends to improve the error reporting UX\nby clarifying why the specified repository is not considered valid.\n\nWe achieve this by means of using the new helper\nis_git_directory_verbose() that has been introduced. With the\nsame, we get a more helpful error message as follows:\n\n  $ GIT_OBJECT_DIRECTORY=/does/not/exist git --git-dir repo.git rev-parse --is-bare-repository\n  fatal: not a git repository: 'repo.git'\n  reason: cannot access object directory '/does/not/exist' set via $GIT_OBJECT_DIRECTORY\n\nSigned-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>\n---\n setup.c                       | 10 ++++++++--\n t/t0009-git-dir-validation.sh | 10 ++++++----\n 2 files changed, 14 insertions(+), 6 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex a0fb68f7f6..a0d3c0c5bb 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -1195,6 +1195,7 @@ static void repo_discover_explicit_gitdir(struct repo_discovery *discovery,\n \t\t\t\t\t  int *nongit_ok)\n {\n \tconst char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);\n+\tstruct strbuf invalid_gitdir_reason = STRBUF_INIT;\n \tchar *gitfile;\n \tint offset;\n \n@@ -1207,12 +1208,16 @@ static void repo_discover_explicit_gitdir(struct repo_discovery *discovery,\n \t\tgitdirenv = gitfile;\n \t}\n \n-\tif (!is_git_directory(gitdirenv)) {\n+\tif (!is_git_directory_verbose(gitdirenv, &invalid_gitdir_reason)) {\n+\t\tstruct strbuf die_msg = STRBUF_INIT;\n \t\tif (nongit_ok) {\n \t\t\t*nongit_ok = 1;\n \t\t\tgoto out;\n \t\t}\n-\t\tdie(_(\"not a git repository: '%s'\"), gitdirenv);\n+\n+\t\tstrbuf_addf(&die_msg, _(\"not a git repository: '%s'\\nreason: %s\"),\n+\t\t\t    gitdirenv, invalid_gitdir_reason.buf);\n+\t\tdie(\"%s\", die_msg.buf);\n \t}\n \n \tif (read_and_verify_repository_format(&discovery->format, gitdirenv, nongit_ok))\n@@ -1274,6 +1279,7 @@ static void repo_discover_explicit_gitdir(struct repo_discovery *discovery,\n \trepo_discovery_set_gitdir(discovery, gitdirenv, 0);\n \n out:\n+\tstrbuf_release(&invalid_gitdir_reason);\n \tfree(gitfile);\n }\n \ndiff --git a/t/t0009-git-dir-validation.sh b/t/t0009-git-dir-validation.sh\nindex 6c40925aa4..1f8ac3fad5 100755\n--- a/t/t0009-git-dir-validation.sh\n+++ b/t/t0009-git-dir-validation.sh\n@@ -78,7 +78,8 @@ test_expect_success 'setup: custom git directory with missing HEAD is rejected'\n \ttest_when_finished \"rm -rf parent/empty-dir\" &&\n \tmkdir -p parent/empty-dir &&\n \ttest_must_fail git --git-dir parent/empty-dir rev-parse --is-bare-repository 2>stderr &&\n-\ttest_grep \"not a git repository\" stderr\n+\ttest_grep \"not a git repository\" stderr &&\n+\ttest_grep \"reason: could not stat HEAD at\" stderr\n '\n \n test_expect_success 'setup: custom git directory with HEAD as a symlink outside refs/ is rejected' '\n@@ -91,7 +92,8 @@ test_expect_success 'setup: custom git directory with HEAD as a symlink outside\n \t\trm real-repo/HEAD &&\n \t\tln -s ../garbage real-repo/HEAD &&\n \t\ttest_must_fail git --git-dir real-repo rev-parse --is-bare-repository 2>stderr &&\n-\t\ttest_grep \"not a git repository\" stderr\n+\t\ttest_grep \"not a git repository\" stderr &&\n+\t\ttest_grep \"reason: HEAD is a symlink .* but target lives outside refs\" stderr\n \t)\n '\n \n@@ -103,9 +105,9 @@ test_expect_success 'setup: custom git directory with invalid GIT_OBJECT_DIRECTO\n \t\tgit init --bare real-repo &&\n \t\ttest_must_fail env GIT_OBJECT_DIRECTORY=\"$(pwd)/does-not-exist\" \\\n \t\t\tgit --git-dir real-repo rev-parse --is-bare-repository 2>stderr &&\n-\t\ttest_grep \"not a git repository\" stderr\n+\t\ttest_grep \"not a git repository\" stderr &&\n+\t\ttest_grep \"reason: cannot access object directory .* set via \\$GIT_OBJECT_DIRECTORY\"   stderr\n \t)\n '\n \n-\n test_done\n-- \n2.56.0.rc1.12.g2c9c8d64bb\n\n"},{"id":"553724","messageId":"ar0yutZ9ksSvaVmM@pks.im","threadId":"66383","inReplyTo":"20260929102513.712181-4-kaartic.sivaraam@gmail.com","subject":"Re: [RFC PATCH v2 3/4] setup: introduce new helper 'is_git_directory_verbose'","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-30T16:03:06Z","receivedAt":"2026-09-30T16:03:13Z","isPatch":true,"body":"On Tue, Sep 29, 2026 at 03:55:09PM +0530, Kaartic Sivaraam wrote:\n> diff --git a/setup.c b/setup.c\n> index e9a9ecda19..a0fb68f7f6 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -347,7 +347,7 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)\n>  \treturn ret;\n>  }\n>  \n> -static int validate_headref(const char *path)\n> +static int validate_headref(const char *path, struct strbuf *err)\n>  {\n>  \tstruct stat st;\n>  \tchar buffer[256];\n\nIf only we had structured errors.\n\n> @@ -356,14 +356,23 @@ static int validate_headref(const char *path)\n>  \tint fd;\n>  \tssize_t len;\n>  \n> -\tif (lstat(path, &st) < 0)\n> +\tif (lstat(path, &st) < 0) {\n> +\t\tif (err)\n> +\t\t\tstrbuf_addf(err, _(\"could not stat HEAD at '%s'\"), path);\n\nShouldn't this also include `strerror(errno)`? Otherwise you're still\nnot that much wiser what the root cause of this is.\n\n>  \t\treturn -1;\n> +\t}\n>  \n>  \t/* Make sure it is a \"refs/..\" symlink */\n>  \tif (S_ISLNK(st.st_mode)) {\n>  \t\tlen = readlink(path, buffer, sizeof(buffer)-1);\n>  \t\tif (len >= 5 && !memcmp(\"refs/\", buffer, 5))\n>  \t\t\treturn 0;\n> +\t\tif (len == -1 && err)\n> +\t\t\tstrbuf_addf(err, _(\"could not read the symlink HEAD at '%s'\"),\n> +\t\t\t\t    path);\n\nSame here, we should include `errno`. Other sites should probably be\nupdated, too.\n\n> @@ -396,9 +411,71 @@ static int validate_headref(const char *path)\n>  \tif (get_oid_hex_any(buffer, &oid) != GIT_HASH_UNKNOWN)\n>  \t\treturn 0;\n>  \n> +\tif (err)\n> +\t\tstrbuf_addf(err, _(\"HEAD at '%s' does not point to a valid symbolic\"\n> +\t\t\t\t   \" link or an object ID\"), path);\n> +\n>  \treturn -1;\n>  }\n>  \n> +/*\n> + * A variant of is_git_directory that gives additional\n> + * context via 'err' about why a given suspect is not\n> + * a valid git repository.\n> + */\n> +static int is_git_directory_verbose(const char *suspect, struct strbuf *err)\n> +{\n> +\tstruct strbuf path = STRBUF_INIT;\n> +\tchar *objdir;\n> +\tint ret = 0;\n> +\tsize_t len;\n> +\n> +\t/* Check worktree-related signatures */\n> +\tstrbuf_addstr(&path, suspect);\n> +\tstrbuf_complete(&path, '/');\n> +\tstrbuf_addstr(&path, \"HEAD\");\n> +\tif (validate_headref(path.buf, err))\n> +\t\tgoto done;\n> +\n> +\tstrbuf_reset(&path);\n> +\tget_common_dir(&path, suspect);\n> +\tlen = path.len;\n> +\n> +\t/* Check non-worktree-related signatures */\n> +\tobjdir = getenv(DB_ENVIRONMENT);\n> +\tif (objdir) {\n> +\t\tif (access(objdir, X_OK)) {\n> +\t\t\tif (err)\n> +\t\t\t\tstrbuf_addf(err, _(\"cannot access object directory '%s'\"\n> +\t\t\t\t\t\t   \" set via $%s\\n\"), objdir, DB_ENVIRONMENT);\n> +\t\t\tgoto done;\n> +\t\t}\n> +\t} else {\n> +\t\tstrbuf_setlen(&path, len);\n> +\t\tstrbuf_addstr(&path, \"/objects\");\n> +\t\tif (access(path.buf, X_OK)) {\n> +\t\t\tif (err)\n> +\t\t\t\tstrbuf_addf(err, _(\"cannot access object directory '%s'\"),\n> +\t\t\t\t\t    path.buf);\n> +\t\t\tgoto done;\n> +\t\t}\n> +\t}\n> +\n> +\tstrbuf_setlen(&path, len);\n> +\tstrbuf_addstr(&path, \"/refs\");\n> +\tif (access(path.buf, X_OK)) {\n> +\t\tif (err)\n> +\t\t\tstrbuf_addf(err, _(\"cannot access refs directory '%s'\"), path.buf);\n> +\t\tgoto done;\n> +\t}\n> +\n> +\tret = 1;\n> +done:\n> +\tstrbuf_release(&path);\n> +\treturn ret;\n> +\n> +}\n> +\n>  /*\n>   * Test if it looks like we're at a git directory.\n>   * We want to see:\n\nIt would've been helpful to move the function up in a separate commit.\nLike this it's hard to see what exactly has changed.\n\nPatrick\n"},{"id":"553725","messageId":"ar0ywAlMDuzK1Hvb@pks.im","threadId":"66383","inReplyTo":"20260929102513.712181-5-kaartic.sivaraam@gmail.com","subject":"Re: [RFC PATCH v2 4/4] setup: communicate why a directory is not a valid git directory","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-09-30T16:03:12Z","receivedAt":"2026-09-30T16:03:18Z","isPatch":true,"body":"On Tue, Sep 29, 2026 at 03:55:10PM +0530, Kaartic Sivaraam wrote:\n> At the moment, there are a few scenarios in which the error message\n> surrounding an invalid Git repository is a bit blunt:\n> \n>   $ GIT_OBJECT_DIRECTORY=/does/not/exist git --git-dir repo.git rev-parse --is-bare-repository\n>   fatal: not a git repository: 'repo.git'\n> \n> In this case, even though repo.git is a valid Git repository,\n> we get an output saying it is not since the GIT_OBJECT_DIRECTORY\n> does not point to a valid object directory. At the moment, the\n> user is on their own in figuring this out.\n> \n> Instead, make it more easy for users to figure such issues\n\ns/more easy/easier/\n\n> particularly in cases where they have explicitly specified\n> a Git directory. This intends to improve the error reporting UX\n> by clarifying why the specified repository is not considered valid.\n\nWhich I think is a good motivation.\n\n> We achieve this by means of using the new helper\n> is_git_directory_verbose() that has been introduced. With the\n> same, we get a more helpful error message as follows:\n> \n>   $ GIT_OBJECT_DIRECTORY=/does/not/exist git --git-dir repo.git rev-parse --is-bare-repository\n>   fatal: not a git repository: 'repo.git'\n>   reason: cannot access object directory '/does/not/exist' set via $GIT_OBJECT_DIRECTORY\n\nHaving a separate \"reason:\" line feels a bit off to me, but that may be\nsubjective. I'd have preferred to have it on the same line, or maybe\nfirst have \"error:\" followed by \"fatal:\".\n\n> diff --git a/setup.c b/setup.c\n> index a0fb68f7f6..a0d3c0c5bb 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -1195,6 +1195,7 @@ static void repo_discover_explicit_gitdir(struct repo_discovery *discovery,\n>  \t\t\t\t\t  int *nongit_ok)\n>  {\n>  \tconst char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);\n> +\tstruct strbuf invalid_gitdir_reason = STRBUF_INIT;\n\nNit, please feel free to ignore: I'd just have called this `errbuf`.\n\nPatrick\n"},{"id":"553740","messageId":"xmqqv77ma8ty.fsf@gitster.g","threadId":"66383","inReplyTo":"20260929102513.712181-4-kaartic.sivaraam@gmail.com","subject":"Re: [RFC PATCH v2 3/4] setup: introduce new helper 'is_git_directory_verbose'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-09-30T18:32:41Z","receivedAt":"2026-09-30T18:32:44Z","isPatch":true,"body":"Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:\n\n> Introduce a new helper is_git_directory_verbose() as a\n> counterpart to the existing is_git_directory().\n>\n> is_git_directory_verbose() also populates an optional\n> string strbuf with reasoning around why the given suspect\n> is not a valid git directory. This strbuf in turn can be\n> used to improve the error reporting which is currently blunt:\n>\n>   fatal: not a git repository\n>\n> This is not helpful as the user does not get any hint about \"why\"\n> the repository is not considered valid.\n\nI suspect that it is much less the failure to report the reason that\nmay confuse users than the failure to report which directory was\ninspected and why we picked that directory.  Once we make it clear\nwhich directory was used as the repository to run the Git operation\nthe user specified, they can visit the directory themselves and see\nthat there is no HEAD, etc.  The user may have a leftover GIT_DIR in\nthe environment that is interfering with their operation without\ntheir remembering they still had it, for example.  For this reason,\nI am not so enthused to see this much code churn to tell the user\nthe subtle differences among a dangling symlink HEAD, a HEAD\npointing outside refs, and a missing HEAD.\n\nWhat would be preferable is a much less invasive patch that reports\nwhich path was assumed to be the repository and where it came from\n(among GIT_DIR, auto-discovery stopped at GIT_CEILING_DIRECTORIES,\netc.).  It would help the user figure out why the directory they\nthought was the Git directory is not what the git binary inspected.\n\nThis patch does report which path we thought HEAD should be at in its\nmessages, but does not explain where that assumption came from,\nunlike the verification of the objects/ directory, which mentions the\nenvironment variable if it is involved.  This feels uneven.\n\nThanks.\n"},{"id":"554166","messageId":"168ac5aa-a05b-43df-9cf4-78c4295e4faa@gmail.com","threadId":"66383","inReplyTo":"ar0yutZ9ksSvaVmM@pks.im","subject":"Re: [RFC PATCH v2 3/4] setup: introduce new helper 'is_git_directory_verbose'","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-10-05T12:14:56Z","receivedAt":"2026-10-05T12:15:01Z","isPatch":true,"body":"On 9/30/26 21:33, Patrick Steinhardt wrote:\n> On Tue, Sep 29, 2026 at 03:55:09PM +0530, Kaartic Sivaraam wrote:\n>> diff --git a/setup.c b/setup.c\n>> index e9a9ecda19..a0fb68f7f6 100644\n>> --- a/setup.c\n>> +++ b/setup.c\n>> @@ -347,7 +347,7 @@ int get_common_dir_noenv(struct strbuf *sb, const char *gitdir)\n>>   \treturn ret;\n>>   }\n>>   \n>> -static int validate_headref(const char *path)\n>> +static int validate_headref(const char *path, struct strbuf *err)\n>>   {\n>>   \tstruct stat st;\n>>   \tchar buffer[256];\n> \n> If only we had structured errors.\n>\n\nIndeed.\n>> @@ -356,14 +356,23 @@ static int validate_headref(const char *path)\n>>   \tint fd;\n>>   \tssize_t len;\n>>   \n>> -\tif (lstat(path, &st) < 0)\n>> +\tif (lstat(path, &st) < 0) {\n>> +\t\tif (err)\n>> +\t\t\tstrbuf_addf(err, _(\"could not stat HEAD at '%s'\"), path);\n> \n> Shouldn't this also include `strerror(errno)`? Otherwise you're still\n> not that much wiser what the root cause of this is.\n> \n\nThat would of course be an improvement as it helps provide more context. \nWill check on it.\n  >>   \t\treturn -1;\n>> +\t}\n>>   \n>>   \t/* Make sure it is a \"refs/..\" symlink */\n>>   \tif (S_ISLNK(st.st_mode)) {\n>>   \t\tlen = readlink(path, buffer, sizeof(buffer)-1);\n>>   \t\tif (len >= 5 && !memcmp(\"refs/\", buffer, 5))\n>>   \t\t\treturn 0;\n>> +\t\tif (len == -1 && err)\n>> +\t\t\tstrbuf_addf(err, _(\"could not read the symlink HEAD at '%s'\"),\n>> +\t\t\t\t    path);\n> \n> Same here, we should include `errno`. Other sites should probably be\n> updated, too.\n> \n\nNoted.\n\n> \n> It would've been helpful to move the function up in a separate commit.\n> Like this it's hard to see what exactly has changed.\n> \n\nIndeed. I will improve it in the next iteration.\n\n-- \nSivaraam\n\n"},{"id":"554168","messageId":"b829d416-f36e-454e-a141-3fc93fd62cb3@gmail.com","threadId":"66383","inReplyTo":"ar0ywAlMDuzK1Hvb@pks.im","subject":"Re: [RFC PATCH v2 4/4] setup: communicate why a directory is not a valid git directory","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-10-05T12:29:23Z","receivedAt":"2026-10-05T12:29:28Z","isPatch":true,"body":"On 9/30/26 21:33, Patrick Steinhardt wrote:\n> On Tue, Sep 29, 2026 at 03:55:10PM +0530, Kaartic Sivaraam wrote:\n>> At the moment, there are a few scenarios in which the error message\n>> surrounding an invalid Git repository is a bit blunt:\n>>\n>>    $ GIT_OBJECT_DIRECTORY=/does/not/exist git --git-dir repo.git rev-parse --is-bare-repository\n>>    fatal: not a git repository: 'repo.git'\n>>\n>> In this case, even though repo.git is a valid Git repository,\n>> we get an output saying it is not since the GIT_OBJECT_DIRECTORY\n>> does not point to a valid object directory. At the moment, the\n>> user is on their own in figuring this out.\n>>\n>> Instead, make it more easy for users to figure such issues\n> \n> s/more easy/easier/\n>\n\nAh. Will correct.\n\n>> particularly in cases where they have explicitly specified\n>> a Git directory. This intends to improve the error reporting UX\n>> by clarifying why the specified repository is not considered valid.\n> \n> Which I think is a good motivation.\n> \n>> We achieve this by means of using the new helper\n>> is_git_directory_verbose() that has been introduced. With the\n>> same, we get a more helpful error message as follows:\n>>\n>>    $ GIT_OBJECT_DIRECTORY=/does/not/exist git --git-dir repo.git rev-parse --is-bare-repository\n>>    fatal: not a git repository: 'repo.git'\n>>    reason: cannot access object directory '/does/not/exist' set via $GIT_OBJECT_DIRECTORY\n> \n> Having a separate \"reason:\" line feels a bit off to me, but that may be\n> subjective. I'd have preferred to have it on the same line, or maybe\n> first have \"error:\" followed by \"fatal:\".\n> \n\nHmm. Let me think which of these I could adopt. Thank you for the \nsuggestion.\n\n>> diff --git a/setup.c b/setup.c\n>> index a0fb68f7f6..a0d3c0c5bb 100644\n>> --- a/setup.c\n>> +++ b/setup.c\n>> @@ -1195,6 +1195,7 @@ static void repo_discover_explicit_gitdir(struct repo_discovery *discovery,\n>>   \t\t\t\t\t  int *nongit_ok)\n>>   {\n>>   \tconst char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);\n>> +\tstruct strbuf invalid_gitdir_reason = STRBUF_INIT;\n> \n> Nit, please feel free to ignore: I'd just have called this `errbuf`.\n> \n\nNoted.\n\n-- \nSivaraam\n\n"}]}