{"thread":{"id":"65013","subject":"[PATCH v6 0/2] setup: allow cwd/.git to be a symlink to a directory","startedAt":"2026-02-18T12:46:48Z","lastAt":"2026-03-09T23:30:12Z","messageCount":45,"participants":["Tian Yuchen","Junio C Hamano","Karthik Nayak","Phillip Wood"],"isPatch":true,"patchVersion":6,"patchTotal":2},"messages":[{"id":"536276","messageId":"20260218124638.176936-1-a3205153416@gmail.com","threadId":"65013","inReplyTo":null,"subject":"[PATCH v6 0/2] setup: allow cwd/.git to be a symlink to a directory","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-18T12:46:36Z","receivedAt":"2026-02-18T12:46:48Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi Junio, Karthik,\n\nHere is the v6 reroll.\n\nChanges since v5 (and previous discussions):\n\n1. Fixed potential regression in gentle setup:\n In Patch 2/2, I fixed a logic bug where `setup_git_directory_gently()`\n could crash on generic errors (like `INVALID_FORMAT`) when `die_on_error`\n was false.\n The new logic only delegates to the die-handler for:\n - Benign cases we want to ignore (`ENOENT`, `IS_A_DIR`).\n - Security risks we MUST reject (`NOT_A_FILE`).\n - When `die_on_error` is explicitly true.\n\n2. Refinements:\n - Used `break` instead of `return` in `read_gitfile_error_die()` (Patch 1/2).\n - Updated the error message for `NOT_A_FILE` to be more precise.\n\nThanks for the guidance! \n\n\nTian Yuchen (2):\n setup: distinguish ENOENT from other stat errors\n setup: allow cwd/.git to be a symlink to a directory\n\n setup.c | 38 ++++++++++++------\n setup.h | 2 +\n t/meson.build | 1 +\n t/t0009-git-dir-validation.sh | 72 +++++++++++++++++++++++++++++++++++\n 4 files changed, 101 insertions(+), 12 deletions(-)\n create mode 100755 t/t0009-git-dir-validation.sh\n\n-- \n2.43.0\n\n"},{"id":"536277","messageId":"20260218124638.176936-2-a3205153416@gmail.com","threadId":"65013","inReplyTo":"20260218124638.176936-1-a3205153416@gmail.com","subject":"[PATCH v6 1/2] setup: distinguish ENOENT from other stat errors","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-18T12:46:37Z","receivedAt":"2026-02-18T12:46:51Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Currently, 'read_gitfile_gently()' treats all 'stat()' failures as\ngeneric errors. This prevents distinguishing between a missing file and\nreal errors like permission denied (fatal).\n\nSplit 'READ_GITFILE_ERR_STAT_FAILED' into two:\n - 'READ_GITFILE_ERR_STAT_ENOENT': The file simply does not exist;\n - 'READ_GITFILE_ERR_STAT_FAILED': Real I/O or permission errors.\n\nIntroduce 'READ_GITFILE_ERR_IS_A_DIR':\n - Used when the path exists but is a directory.\n\nUpdate 'read_gitfile_error_die()' to handle these cases:\n - Happy path ('ENOENT', 'IS_A_DIR'): Return without dying;\n - Fatal path ('STAT_FAILED', 'NOA_A_FILE'): Die immediately.\n\nThis prepares for error handling in the next commit.\n\nSigned-off-by: Tian Yuchen \n---\n setup.c | 17 +++++++++++++----\n setup.h | 2 ++\n 2 files changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex c8336eb20e..d48b6a3a3d 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -897,10 +897,13 @@ int verify_repository_format(const struct repository_format *format,\n void read_gitfile_error_die(int error_code, const char *path, const char *dir)\n {\n \tswitch (error_code) {\n+\tcase READ_GITFILE_ERR_STAT_ENOENT:\n+\tcase READ_GITFILE_ERR_IS_A_DIR:\n+\t\tbreak;\n \tcase READ_GITFILE_ERR_STAT_FAILED:\n+\t\tdie(_(\"error reading %s\"), path);\n \tcase READ_GITFILE_ERR_NOT_A_FILE:\n-\t\t/* non-fatal; follow return path */\n-\t\tbreak;\n+\t\tdie(_(\"not a regular file: %s\"), path);\n \tcase READ_GITFILE_ERR_OPEN_FAILED:\n \t\tdie_errno(_(\"error opening '%s'\"), path);\n \tcase READ_GITFILE_ERR_TOO_LARGE:\n@@ -941,8 +944,14 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n \tstatic struct strbuf realpath = STRBUF_INIT;\n \n \tif (stat(path, &st)) {\n-\t\t/* NEEDSWORK: discern between ENOENT vs other errors */\n-\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n+\t\tif (errno == ENOENT)\n+\t\t\terror_code = READ_GITFILE_ERR_STAT_ENOENT;\n+\t\telse\n+\t\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n+\t\tgoto cleanup_return;\n+\t}\n+\tif (S_ISDIR(st.st_mode)) {\n+\t\terror_code = READ_GITFILE_ERR_IS_A_DIR;\n \t\tgoto cleanup_return;\n \t}\n \tif (!S_ISREG(st.st_mode)) {\ndiff --git a/setup.h b/setup.h\nindex 0738dec244..ed4b13f061 100644\n--- a/setup.h\n+++ b/setup.h\n@@ -36,6 +36,8 @@ int is_nonbare_repository_dir(struct strbuf *path);\n #define READ_GITFILE_ERR_NO_PATH 6\n #define READ_GITFILE_ERR_NOT_A_REPO 7\n #define READ_GITFILE_ERR_TOO_LARGE 8\n+#define READ_GITFILE_ERR_STAT_ENOENT 9\n+#define READ_GITFILE_ERR_IS_A_DIR 10\n void read_gitfile_error_die(int error_code, const char *path, const char *dir);\n const char *read_gitfile_gently(const char *path, int *return_error_code);\n #define read_gitfile(path) read_gitfile_gently((path), NULL)\n-- \n2.43.0\n\n"},{"id":"536278","messageId":"20260218124638.176936-3-a3205153416@gmail.com","threadId":"65013","inReplyTo":"20260218124638.176936-1-a3205153416@gmail.com","subject":"[PATCH v6 2/2] setup: allow cwd/.git to be a symlink to a directory","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-18T12:46:38Z","receivedAt":"2026-02-18T12:46:54Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Strictly enforcing 'lstat()' prevents valid '.git' symlinks.\n\nRely on 'stat()' (via 'read_gitfile_gently()') to handle filesystem\nresolution, instead of blocking it with strict logic checks in\n'setup_git_directory_gently_1()'.\n\nTo ensure safety and correctness, we unconditionally delegate benign\ncases ('ENOENT', 'IS_A_DIR') and security risks ('NOT_A_FILE') to\n'read_gitfile_error_die()'.\n\nFor other errors (like invalid format), we only invoke the handler if\n'die_on_error' is true; otherwise, we return the error code to respect\nthe gentle fallback behavior.\n\nAdd 't/t0009-git-dir-validation.sh' to verify symlink support and FIFO\nrejection, and register it in 't/meson.build'.\n\nSigned-off-by: Tian Yuchen \n---\n setup.c | 21 ++++++----\n t/meson.build | 1 +\n t/t0009-git-dir-validation.sh | 72 +++++++++++++++++++++++++++++++++++\n 3 files changed, 86 insertions(+), 8 deletions(-)\n create mode 100755 t/t0009-git-dir-validation.sh\n\ndiff --git a/setup.c b/setup.c\nindex d48b6a3a3d..f4b9d41f78 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -1590,15 +1590,20 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n \t\tgitdirenv = read_gitfile_gently(dir->buf, die_on_error ?\n \t\t\t\t\t\tNULL : &error_code);\n \t\tif (!gitdirenv) {\n-\t\t\tif (die_on_error ||\n-\t\t\t error_code == READ_GITFILE_ERR_NOT_A_FILE) {\n-\t\t\t\t/* NEEDSWORK: fail if .git is not file nor dir */\n-\t\t\t\tif (is_git_directory(dir->buf)) {\n-\t\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n-\t\t\t\t\tgitdir_path = xstrdup(dir->buf);\n+\t\t\tif (error_code){\n+\t\t\t\tif (error_code == READ_GITFILE_ERR_STAT_ENOENT ||\n+\t\t\t\terror_code == READ_GITFILE_ERR_IS_A_DIR ||\n+\t\t\t\terror_code == READ_GITFILE_ERR_NOT_A_FILE ||\n+\t\t\t\tdie_on_error) {\n+\t\t\t\t\tread_gitfile_error_die(error_code, dir->buf, NULL);\n+\t\t\t\t} else {\n+\t\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n \t\t\t\t}\n-\t\t\t} else if (error_code != READ_GITFILE_ERR_STAT_FAILED)\n-\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n+\t\t\t}\n+\t\t\tif (is_git_directory(dir->buf)) {\n+\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n+\t\t\t\tgitdir_path = xstrdup(dir->buf);\n+\t\t\t}\n \t\t} else\n \t\t\tgitfile = xstrdup(dir->buf);\n \t\t/*\ndiff --git a/t/meson.build b/t/meson.build\nindex f80e366cff..c4afaacee5 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -80,6 +80,7 @@ integration_tests = [\n 't0006-date.sh',\n 't0007-git-var.sh',\n 't0008-ignores.sh',\n+ 't0009-git-dir-validation.sh',\n 't0010-racy-git.sh',\n 't0012-help.sh',\n 't0013-sha1dc.sh',\ndiff --git a/t/t0009-git-dir-validation.sh b/t/t0009-git-dir-validation.sh\nnew file mode 100755\nindex 0000000000..9b3925c85f\n--- /dev/null\n+++ b/t/t0009-git-dir-validation.sh\n@@ -0,0 +1,72 @@\n+#!/bin/sh\n+\n+test_description='setup: validation of .git file/directory types\n+\n+Verify that setup_git_directory() correctly handles:\n+1. Valid .git directories (including symlinks to them).\n+2. Invalid .git files (FIFOs, sockets) by erroring out.\n+3. Invalid .git files (garbage) by erroring out.\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup: create parent git repository' '\n+\tgit init parent &&\n+\ttest_commit -C parent \"root-commit\"\n+'\n+\n+test_expect_success SYMLINKS 'setup: .git as a symlink to a directory is valid' '\n+\tmkdir -p parent/link-to-dir &&\n+\t(\n+\t\tcd parent/link-to-dir &&\n+\t\tgit init real-repo &&\n+\t\tln -s real-repo/.git .git &&\n+\t\tgit rev-parse --git-dir >actual &&\n+\t\techo .git >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success PIPE 'setup: .git as a FIFO (named pipe) is rejected' '\n+\tmkdir -p parent/fifo-trap &&\n+\t(\n+\t\tcd parent/fifo-trap &&\n+\t\tmkfifo .git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"not a regular file\" stderr\n+\t)\n+'\n+\n+test_expect_success SYMLINKS,PIPE 'setup: .git as a symlink to a FIFO is rejected' '\n+\tmkdir -p parent/symlink-fifo-trap &&\n+\t(\n+\t\tcd parent/symlink-fifo-trap &&\n+\t\tmkfifo target-fifo &&\n+\t\tln -s target-fifo .git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"not a regular file\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: .git with garbage content is rejected' '\n+\tmkdir -p parent/garbage-trap &&\n+\t(\n+\t\tcd parent/garbage-trap &&\n+\t\techo \"garbage\" >.git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"invalid gitfile format\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: .git as an empty directory is ignored' '\n+\tmkdir -p parent/empty-dir &&\n+\t(\n+\t\tcd parent/empty-dir &&\n+\t\tmkdir .git &&\n+\t\tgit rev-parse --git-dir >actual &&\n+\t\techo \"$TRASH_DIRECTORY/parent/.git\" >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_done\n-- \n2.43.0\n\n"},{"id":"536375","messageId":"20260219071650.208074-1-a3205153416@gmail.com","threadId":"65013","inReplyTo":"20260218124638.176936-1-a3205153416@gmail.com","subject":"[PATCH v7] setup: allow cwd/.git to be a symlink to a directory","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-19T07:16:50Z","receivedAt":"2026-02-19T07:17:01Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Currently, `setup_git_directory_gently_1()` fails to recognize a `.git`\nsymlink pointing to a directory because `read_gitfile_gently()` strictly\nexpects a regular file and returns `READ_GITFILE_ERR_NOT_A_FILE` for\nanything else, including valid directories.\n\nFix this by distinguishing directories from regular files and other\nnon-regular file types (like FIFOs or sockets) via newly introduced\nerror_code.\n\nTo preserve the original intent of the setup process:\n1. Update `read_gitfile_error_die()` to treat `IS_A_DIR` as a no-op\n (like `ENOENT`), while still calling `die()` on true `NOT_A_FILE`\n errors.\n2. Unconditionally pass `&error_code` to `read_gitfile_gently()`. This\n eliminates an uninitialized variable hazard that occurred when\n `die_on_error` was true and `NULL` was passed.\n3. Only invoke `is_git_directory()` when we explicitly receive\n `READ_GITFILE_ERR_IS_A_DIR`, avoiding redundant filesystem checks.\n4. Correctly return `GIT_DIR_INVALID_GITFILE` on unrecognized errors\n when `die_on_error` is false.\n\nSigned-off-by: Tian Yuchen \n---\nChanges since v6:\n\n - Squashed into a single commit.\n - Fixed a hidden uninitialized variable trap. In v6:\n 'setup_git_directory_gently_1()' passed 'die_on_error ? NULL : &error_code'\n to 'read_gitfile_gently()'. When 'NULL' was passed, the local 'error_code'\n remained uninitialized. The old code survived this because of short-circuit\n evaluation ('if (die_on_error || error_code == ...)'). \n In this v7, I now unconditionally pass '&error_code'. This gives the caller\n explicit control over error routing.\n (Actually, I seem to have made the same modification back in v3 or v4, but I didn't\n realize at the time that the part I changed was originally a bug. So it was a \n happy accident. :P\n - We now only invoke 'is_git_directory()' explicitly when 'error_code ==\n READ_GITFILE_ERR_IS_A_DIR'.\n\nThanks for your patience.\n\n setup.c | 44 +++++++++++++--------\n setup.h | 2 +\n t/meson.build | 1 +\n t/t0009-git-dir-validation.sh | 72 +++++++++++++++++++++++++++++++++++\n 4 files changed, 104 insertions(+), 15 deletions(-)\n create mode 100755 t/t0009-git-dir-validation.sh\n\ndiff --git a/setup.c b/setup.c\nindex c8336eb20e..5a573e5865 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -897,10 +897,13 @@ int verify_repository_format(const struct repository_format *format,\n void read_gitfile_error_die(int error_code, const char *path, const char *dir)\n {\n \tswitch (error_code) {\n+\tcase READ_GITFILE_ERR_STAT_ENOENT:\n+\tcase READ_GITFILE_ERR_IS_A_DIR:\n+\t\tbreak;\n \tcase READ_GITFILE_ERR_STAT_FAILED:\n+\t\tdie(_(\"error reading %s\"), path);\n \tcase READ_GITFILE_ERR_NOT_A_FILE:\n-\t\t/* non-fatal; follow return path */\n-\t\tbreak;\n+\t\tdie(_(\"not a regular file: %s\"), path);\n \tcase READ_GITFILE_ERR_OPEN_FAILED:\n \t\tdie_errno(_(\"error opening '%s'\"), path);\n \tcase READ_GITFILE_ERR_TOO_LARGE:\n@@ -941,8 +944,14 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n \tstatic struct strbuf realpath = STRBUF_INIT;\n \n \tif (stat(path, &st)) {\n-\t\t/* NEEDSWORK: discern between ENOENT vs other errors */\n-\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n+\t\tif (errno == ENOENT)\n+\t\t\terror_code = READ_GITFILE_ERR_STAT_ENOENT;\n+\t\telse\n+\t\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n+\t\tgoto cleanup_return;\n+\t}\n+\tif (S_ISDIR(st.st_mode)) {\n+\t\terror_code = READ_GITFILE_ERR_IS_A_DIR;\n \t\tgoto cleanup_return;\n \t}\n \tif (!S_ISREG(st.st_mode)) {\n@@ -1578,20 +1587,25 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n \t\tif (offset > min_offset)\n \t\t\tstrbuf_addch(dir, '/');\n \t\tstrbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);\n-\t\tgitdirenv = read_gitfile_gently(dir->buf, die_on_error ?\n-\t\t\t\t\t\tNULL : &error_code);\n+\t\tgitdirenv = read_gitfile_gently(dir->buf, &error_code);\n \t\tif (!gitdirenv) {\n-\t\t\tif (die_on_error ||\n-\t\t\t error_code == READ_GITFILE_ERR_NOT_A_FILE) {\n-\t\t\t\t/* NEEDSWORK: fail if .git is not file nor dir */\n-\t\t\t\tif (is_git_directory(dir->buf)) {\n-\t\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n-\t\t\t\t\tgitdir_path = xstrdup(dir->buf);\n+\t\t\tif (error_code == READ_GITFILE_ERR_IS_A_DIR &&\n+\t\t\tis_git_directory(dir->buf)) {\n+\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n+\t\t\t\tgitdir_path = xstrdup(dir->buf);\n+\t\t\t} else {\n+\t\t\t\tif (error_code == READ_GITFILE_ERR_STAT_ENOENT ||\n+\t\t\t\terror_code == READ_GITFILE_ERR_IS_A_DIR ||\n+\t\t\t\terror_code == READ_GITFILE_ERR_NOT_A_FILE ||\n+\t\t\t\tdie_on_error) {\n+\t\t\t\t\tread_gitfile_error_die(error_code, dir->buf, NULL);\n+\t\t\t\t} else {\n+\t\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n \t\t\t\t}\n-\t\t\t} else if (error_code != READ_GITFILE_ERR_STAT_FAILED)\n-\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n-\t\t} else\n+\t\t\t}\n+\t\t} else {\n \t\t\tgitfile = xstrdup(dir->buf);\n+\t\t}\n \t\t/*\n \t\t * Earlier, we tentatively added DEFAULT_GIT_DIR_ENVIRONMENT\n \t\t * to check that directory for a repository.\ndiff --git a/setup.h b/setup.h\nindex 0738dec244..ed4b13f061 100644\n--- a/setup.h\n+++ b/setup.h\n@@ -36,6 +36,8 @@ int is_nonbare_repository_dir(struct strbuf *path);\n #define READ_GITFILE_ERR_NO_PATH 6\n #define READ_GITFILE_ERR_NOT_A_REPO 7\n #define READ_GITFILE_ERR_TOO_LARGE 8\n+#define READ_GITFILE_ERR_STAT_ENOENT 9\n+#define READ_GITFILE_ERR_IS_A_DIR 10\n void read_gitfile_error_die(int error_code, const char *path, const char *dir);\n const char *read_gitfile_gently(const char *path, int *return_error_code);\n #define read_gitfile(path) read_gitfile_gently((path), NULL)\ndiff --git a/t/meson.build b/t/meson.build\nindex f80e366cff..c4afaacee5 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -80,6 +80,7 @@ integration_tests = [\n 't0006-date.sh',\n 't0007-git-var.sh',\n 't0008-ignores.sh',\n+ 't0009-git-dir-validation.sh',\n 't0010-racy-git.sh',\n 't0012-help.sh',\n 't0013-sha1dc.sh',\ndiff --git a/t/t0009-git-dir-validation.sh b/t/t0009-git-dir-validation.sh\nnew file mode 100755\nindex 0000000000..9b3925c85f\n--- /dev/null\n+++ b/t/t0009-git-dir-validation.sh\n@@ -0,0 +1,72 @@\n+#!/bin/sh\n+\n+test_description='setup: validation of .git file/directory types\n+\n+Verify that setup_git_directory() correctly handles:\n+1. Valid .git directories (including symlinks to them).\n+2. Invalid .git files (FIFOs, sockets) by erroring out.\n+3. Invalid .git files (garbage) by erroring out.\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup: create parent git repository' '\n+\tgit init parent &&\n+\ttest_commit -C parent \"root-commit\"\n+'\n+\n+test_expect_success SYMLINKS 'setup: .git as a symlink to a directory is valid' '\n+\tmkdir -p parent/link-to-dir &&\n+\t(\n+\t\tcd parent/link-to-dir &&\n+\t\tgit init real-repo &&\n+\t\tln -s real-repo/.git .git &&\n+\t\tgit rev-parse --git-dir >actual &&\n+\t\techo .git >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success PIPE 'setup: .git as a FIFO (named pipe) is rejected' '\n+\tmkdir -p parent/fifo-trap &&\n+\t(\n+\t\tcd parent/fifo-trap &&\n+\t\tmkfifo .git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"not a regular file\" stderr\n+\t)\n+'\n+\n+test_expect_success SYMLINKS,PIPE 'setup: .git as a symlink to a FIFO is rejected' '\n+\tmkdir -p parent/symlink-fifo-trap &&\n+\t(\n+\t\tcd parent/symlink-fifo-trap &&\n+\t\tmkfifo target-fifo &&\n+\t\tln -s target-fifo .git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"not a regular file\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: .git with garbage content is rejected' '\n+\tmkdir -p parent/garbage-trap &&\n+\t(\n+\t\tcd parent/garbage-trap &&\n+\t\techo \"garbage\" >.git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"invalid gitfile format\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: .git as an empty directory is ignored' '\n+\tmkdir -p parent/empty-dir &&\n+\t(\n+\t\tcd parent/empty-dir &&\n+\t\tmkdir .git &&\n+\t\tgit rev-parse --git-dir >actual &&\n+\t\techo \"$TRASH_DIRECTORY/parent/.git\" >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_done\n-- \n2.43.0\n\n"},{"id":"536458","messageId":"xmqqh5rc13rw.fsf@gitster.g","threadId":"65013","inReplyTo":"20260219071650.208074-1-a3205153416@gmail.com","subject":"Re: [PATCH v7] setup: allow cwd/.git to be a symlink to a directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-20T03:40:19Z","receivedAt":"2026-02-20T03:40:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen writes:\n\n> diff --git a/setup.c b/setup.c\n> index c8336eb20e..5a573e5865 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -897,10 +897,13 @@ int verify_repository_format(const struct repository_format *format,\n> void read_gitfile_error_die(int error_code, const char *path, const char *dir)\n> {\n> \tswitch (error_code) {\n> +\tcase READ_GITFILE_ERR_STAT_ENOENT:\n> +\tcase READ_GITFILE_ERR_IS_A_DIR:\n> +\t\tbreak;\n> \tcase READ_GITFILE_ERR_STAT_FAILED:\n> +\t\tdie(_(\"error reading %s\"), path);\n> \tcase READ_GITFILE_ERR_NOT_A_FILE:\n> -\t\t/* non-fatal; follow return path */\n> -\t\tbreak;\n\nThis comment is now lost. Shouldn't (at least /* non-fatal */ part of)\nit be moved to those two new non-error codes we see above?\n\n> \tif (stat(path, &st)) {\n> -\t\t/* NEEDSWORK: discern between ENOENT vs other errors */\n> -\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n> +\t\tif (errno == ENOENT)\n> +\t\t\terror_code = READ_GITFILE_ERR_STAT_ENOENT;\n> +\t\telse\n> +\t\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n> +\t\tgoto cleanup_return;\n> +\t}\n> +\tif (S_ISDIR(st.st_mode)) {\n> +\t\terror_code = READ_GITFILE_ERR_IS_A_DIR;\n> \t\tgoto cleanup_return;\n> \t}\n\nOK.\n\n> @@ -1578,20 +1587,25 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n> \t\tif (offset > min_offset)\n> \t\t\tstrbuf_addch(dir, '/');\n> \t\tstrbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);\n> +\t\tgitdirenv = read_gitfile_gently(dir->buf, &error_code);\n> \t\tif (!gitdirenv) {\n> +\t\t\tif (error_code == READ_GITFILE_ERR_IS_A_DIR &&\n> +\t\t\tis_git_directory(dir->buf)) {\n> +\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n> +\t\t\t\tgitdir_path = xstrdup(dir->buf);\n> +\t\t\t} else {\n> +\t\t\t\tif (error_code == READ_GITFILE_ERR_STAT_ENOENT ||\n> +\t\t\t\terror_code == READ_GITFILE_ERR_IS_A_DIR ||\n> +\t\t\t\terror_code == READ_GITFILE_ERR_NOT_A_FILE ||\n> +\t\t\t\tdie_on_error) {\n> +\t\t\t\t\tread_gitfile_error_die(error_code, dir->buf, NULL);\n> +\t\t\t\t} else {\n> +\t\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n> \t\t\t\t}\n> +\t\t\t}\n\nIs it just me who finds the above harder to follow than necessary?\nI would have expected something like\n\n\tif (!gitdirenv) {\n\t\tswitch (error_code) {\n\t\tcase READ_GITFILE_ERR_IS_A_DIR:\n\t\t\tif (is_git_directory(dir->buf)) {\n\t\t\t\t...\n\t\t\t} else if (die_on_error) {\n\t\t\t\tdie(\"'%s' is an invalid .git directory\", dir->buf);\n\t\t\t} else {\n\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n\t\t\t}\n\t\t\tbreak;\n\t\tcase READ_GITFILE_ERR_STAT_NOENT:\n\t\t\t/* no .git in this directory, move on */\n\t\t\tbreak;\n\t\tdefault:\n\t\t\tif (die_on_error)\n\t\t\t\tread_gitfile_err_stat_noent(error_code, ...);\n\t\t\telse\n\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n\t\t}\n\t}\n\nor its equivalent, with the top-level switch rewritten into\nan if/elseif cascade.\n"},{"id":"536533","messageId":"12cb054d-71f7-4df3-b052-764b62d32f54@gmail.com","threadId":"65013","inReplyTo":"xmqqh5rc13rw.fsf@gitster.g","subject":"Re: [PATCH v7] setup: allow cwd/.git to be a symlink to a directory","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-20T16:27:42Z","receivedAt":"2026-02-20T16:27:46Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"On 2/20/26 11:40, Junio C Hamano wrote:\n\nThanks for the review!\n\n>> \tcase READ_GITFILE_ERR_NOT_A_FILE:\n>> -\t\t/* non-fatal; follow return path */\n>> -\t\tbreak;\n> \n> This comment is now lost. Shouldn't (at least /* non-fatal */ part of)\n> it be moved to those two new non-error codes we see above?\n\nIndeed. I made a mistake here.\n\n>> @@ -1578,20 +1587,25 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n>> \t\tif (offset > min_offset)\n>> \t\t\tstrbuf_addch(dir, '/');\n>> \t\tstrbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);\n>> +\t\tgitdirenv = read_gitfile_gently(dir->buf, &error_code);\n>> \t\tif (!gitdirenv) {\n>> +\t\t\tif (error_code == READ_GITFILE_ERR_IS_A_DIR &&\n>> +\t\t\tis_git_directory(dir->buf)) {\n>> +\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n>> +\t\t\t\tgitdir_path = xstrdup(dir->buf);\n>> +\t\t\t} else {\n>> +\t\t\t\tif (error_code == READ_GITFILE_ERR_STAT_ENOENT ||\n>> +\t\t\t\terror_code == READ_GITFILE_ERR_IS_A_DIR ||\n>> +\t\t\t\terror_code == READ_GITFILE_ERR_NOT_A_FILE ||\n>> +\t\t\t\tdie_on_error) {\n>> +\t\t\t\t\tread_gitfile_error_die(error_code, dir->buf, NULL);\n>> +\t\t\t\t} else {\n>> +\t\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n>> \t\t\t\t}\n>> +\t\t\t}\n> \n> Is it just me who finds the above harder to follow than necessary?\n> I would have expected something like\n\nTo be honest I can't find any reason why the above statement is \ndifficult to understand. However, replacing it with a switch statement \ndoes make it much clearer. On this point, I completely agree with you. \nWill change soon.\n\n\n> \tif (!gitdirenv) {\n> \t\tswitch (error_code) {\n> \t\tcase READ_GITFILE_ERR_IS_A_DIR:\n> \t\t\tif (is_git_directory(dir->buf)) {\n> \t\t\t\t...\n> \t\t\t} else if (die_on_error) {\n> \t\t\t\tdie(\"'%s' is an invalid .git directory\", dir->buf);\n> \t\t\t} else {\n> \t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n> \t\t\t}\n> \t\t\tbreak;\n> \t\tcase READ_GITFILE_ERR_STAT_NOENT:\n> \t\t\t/* no .git in this directory, move on */\n> \t\t\tbreak;\n> \t\tdefault:\n> \t\t\tif (die_on_error)\n> \t\t\t\tread_gitfile_err_stat_noent(error_code, ...);\n> \t\t\telse\n> \t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n> \t\t}\n> \t}\n> \n> or its equivalent, with the top-level switch rewritten into\n> an if/elseif cascade.\n\nPoint taken.\n\nRegards,\n\nYuchen\n\n"},{"id":"536536","messageId":"20260220164512.216901-1-a3205153416@gmail.com","threadId":"65013","inReplyTo":"20260218124638.176936-1-a3205153416@gmail.com","subject":"[PATCH v8] setup: allow cwd/.git to be a symlink to a directory","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-20T16:45:12Z","receivedAt":"2026-02-20T16:45:39Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Currently, `setup_git_directory_gently_1()` fails to recognize a `.git`\nsymlink pointing to a directory because `read_gitfile_gently()` strictly\nexpects a regular file and returns `READ_GITFILE_ERR_NOT_A_FILE` for\nanything else, including valid directories.\n\nFix this by distinguishing directories from regular files and other\nnon-regular file types (like FIFOs or sockets) via newly introduced\nerror_code.\n\nTo preserve the original intent of the setup process:\n1. Update `read_gitfile_error_die()` to treat `IS_A_DIR` as a no-op\n (like `ENOENT`), while still calling `die()` on true `NOT_A_FILE`\n errors.\n2. Unconditionally pass `&error_code` to `read_gitfile_gently()`. This\n eliminates an uninitialized variable hazard that occurred when\n `die_on_error` was true and `NULL` was passed.\n3. Only invoke `is_git_directory()` when we explicitly receive\n `READ_GITFILE_ERR_IS_A_DIR`, avoiding redundant filesystem checks.\n4. Correctly return `GIT_DIR_INVALID_GITFILE` on unrecognized errors\n when `die_on_error` is false.\n\nSigned-off-by: Tian Yuchen \n---\n setup.c | 42 ++++++++++++++------\n setup.h | 2 +\n t/meson.build | 1 +\n t/t0009-git-dir-validation.sh | 72 +++++++++++++++++++++++++++++++++++\n 4 files changed, 105 insertions(+), 12 deletions(-)\n create mode 100755 t/t0009-git-dir-validation.sh\n\ndiff --git a/setup.c b/setup.c\nindex c8336eb20e..2869d10669 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -897,10 +897,14 @@ int verify_repository_format(const struct repository_format *format,\n void read_gitfile_error_die(int error_code, const char *path, const char *dir)\n {\n \tswitch (error_code) {\n-\tcase READ_GITFILE_ERR_STAT_FAILED:\n-\tcase READ_GITFILE_ERR_NOT_A_FILE:\n+\tcase READ_GITFILE_ERR_STAT_ENOENT:\n+\tcase READ_GITFILE_ERR_IS_A_DIR:\n \t\t/* non-fatal; follow return path */\n \t\tbreak;\n+\tcase READ_GITFILE_ERR_STAT_FAILED:\n+\t\tdie(_(\"error reading %s\"), path);\n+\tcase READ_GITFILE_ERR_NOT_A_FILE:\n+\t\tdie(_(\"not a regular file: %s\"), path);\n \tcase READ_GITFILE_ERR_OPEN_FAILED:\n \t\tdie_errno(_(\"error opening '%s'\"), path);\n \tcase READ_GITFILE_ERR_TOO_LARGE:\n@@ -941,8 +945,14 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n \tstatic struct strbuf realpath = STRBUF_INIT;\n \n \tif (stat(path, &st)) {\n-\t\t/* NEEDSWORK: discern between ENOENT vs other errors */\n-\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n+\t\tif (errno == ENOENT)\n+\t\t\terror_code = READ_GITFILE_ERR_STAT_ENOENT;\n+\t\telse\n+\t\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n+\t\tgoto cleanup_return;\n+\t}\n+\tif (S_ISDIR(st.st_mode)) {\n+\t\terror_code = READ_GITFILE_ERR_IS_A_DIR;\n \t\tgoto cleanup_return;\n \t}\n \tif (!S_ISREG(st.st_mode)) {\n@@ -1578,20 +1588,28 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n \t\tif (offset > min_offset)\n \t\t\tstrbuf_addch(dir, '/');\n \t\tstrbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);\n-\t\tgitdirenv = read_gitfile_gently(dir->buf, die_on_error ?\n-\t\t\t\t\t\tNULL : &error_code);\n+\t\tgitdirenv = read_gitfile_gently(dir->buf, &error_code);\n \t\tif (!gitdirenv) {\n-\t\t\tif (die_on_error ||\n-\t\t\t error_code == READ_GITFILE_ERR_NOT_A_FILE) {\n-\t\t\t\t/* NEEDSWORK: fail if .git is not file nor dir */\n+\t\t\tswitch (error_code) {\n+\t\t\tcase READ_GITFILE_ERR_STAT_ENOENT:\n+\t\t\t\t/* no .git in this directory, move on */\n+\t\t\t\tbreak;\n+\t\t\tcase READ_GITFILE_ERR_IS_A_DIR:\n \t\t\t\tif (is_git_directory(dir->buf)) {\n \t\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n \t\t\t\t\tgitdir_path = xstrdup(dir->buf);\n \t\t\t\t}\n-\t\t\t} else if (error_code != READ_GITFILE_ERR_STAT_FAILED)\n-\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n-\t\t} else\n+\t\t\t\t/* otherwise, it is an empty/unrelated directory, move on */\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\tif (die_on_error || error_code == READ_GITFILE_ERR_NOT_A_FILE)\n+\t\t\t\t\tread_gitfile_error_die(error_code, dir->buf, NULL);\n+\t\t\t\telse\n+\t\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n+\t\t\t}\n+\t\t} else {\n \t\t\tgitfile = xstrdup(dir->buf);\n+\t\t}\n \t\t/*\n \t\t * Earlier, we tentatively added DEFAULT_GIT_DIR_ENVIRONMENT\n \t\t * to check that directory for a repository.\ndiff --git a/setup.h b/setup.h\nindex 0738dec244..ed4b13f061 100644\n--- a/setup.h\n+++ b/setup.h\n@@ -36,6 +36,8 @@ int is_nonbare_repository_dir(struct strbuf *path);\n #define READ_GITFILE_ERR_NO_PATH 6\n #define READ_GITFILE_ERR_NOT_A_REPO 7\n #define READ_GITFILE_ERR_TOO_LARGE 8\n+#define READ_GITFILE_ERR_STAT_ENOENT 9\n+#define READ_GITFILE_ERR_IS_A_DIR 10\n void read_gitfile_error_die(int error_code, const char *path, const char *dir);\n const char *read_gitfile_gently(const char *path, int *return_error_code);\n #define read_gitfile(path) read_gitfile_gently((path), NULL)\ndiff --git a/t/meson.build b/t/meson.build\nindex f80e366cff..c4afaacee5 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -80,6 +80,7 @@ integration_tests = [\n 't0006-date.sh',\n 't0007-git-var.sh',\n 't0008-ignores.sh',\n+ 't0009-git-dir-validation.sh',\n 't0010-racy-git.sh',\n 't0012-help.sh',\n 't0013-sha1dc.sh',\ndiff --git a/t/t0009-git-dir-validation.sh b/t/t0009-git-dir-validation.sh\nnew file mode 100755\nindex 0000000000..9b3925c85f\n--- /dev/null\n+++ b/t/t0009-git-dir-validation.sh\n@@ -0,0 +1,72 @@\n+#!/bin/sh\n+\n+test_description='setup: validation of .git file/directory types\n+\n+Verify that setup_git_directory() correctly handles:\n+1. Valid .git directories (including symlinks to them).\n+2. Invalid .git files (FIFOs, sockets) by erroring out.\n+3. Invalid .git files (garbage) by erroring out.\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup: create parent git repository' '\n+\tgit init parent &&\n+\ttest_commit -C parent \"root-commit\"\n+'\n+\n+test_expect_success SYMLINKS 'setup: .git as a symlink to a directory is valid' '\n+\tmkdir -p parent/link-to-dir &&\n+\t(\n+\t\tcd parent/link-to-dir &&\n+\t\tgit init real-repo &&\n+\t\tln -s real-repo/.git .git &&\n+\t\tgit rev-parse --git-dir >actual &&\n+\t\techo .git >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success PIPE 'setup: .git as a FIFO (named pipe) is rejected' '\n+\tmkdir -p parent/fifo-trap &&\n+\t(\n+\t\tcd parent/fifo-trap &&\n+\t\tmkfifo .git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"not a regular file\" stderr\n+\t)\n+'\n+\n+test_expect_success SYMLINKS,PIPE 'setup: .git as a symlink to a FIFO is rejected' '\n+\tmkdir -p parent/symlink-fifo-trap &&\n+\t(\n+\t\tcd parent/symlink-fifo-trap &&\n+\t\tmkfifo target-fifo &&\n+\t\tln -s target-fifo .git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"not a regular file\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: .git with garbage content is rejected' '\n+\tmkdir -p parent/garbage-trap &&\n+\t(\n+\t\tcd parent/garbage-trap &&\n+\t\techo \"garbage\" >.git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"invalid gitfile format\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: .git as an empty directory is ignored' '\n+\tmkdir -p parent/empty-dir &&\n+\t(\n+\t\tcd parent/empty-dir &&\n+\t\tmkdir .git &&\n+\t\tgit rev-parse --git-dir >actual &&\n+\t\techo \"$TRASH_DIRECTORY/parent/.git\" >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_done\n-- \n2.43.0\n\n"},{"id":"536544","messageId":"xmqqfr6vxpkn.fsf@gitster.g","threadId":"65013","inReplyTo":"20260220164512.216901-1-a3205153416@gmail.com","subject":"Re: [PATCH v8] setup: allow cwd/.git to be a symlink to a directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-20T18:00:40Z","receivedAt":"2026-02-20T18:00:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen writes:\n\n> Currently, `setup_git_directory_gently_1()` fails to recognize a `.git`\n> symlink pointing to a directory because `read_gitfile_gently()` strictly\n> expects a regular file and returns `READ_GITFILE_ERR_NOT_A_FILE` for\n> anything else, including valid directories.\n\nI have to admit that I was not paying too much attention to this\npart of the proposed log message. We kept seeing the above (and the\ntitle) for the series forever, but is it really a problem if we did\nnot allow you to make a symbolic link to somebody else's .git/\ndirectory? Besides, I do not think we have such a problem at all,\nas read_gitfile_gently() uses stat() not lstat().\n\n\tSide note: In any case, we do not want to see \"Currently\".\n We give an observation on how the current system works in\n the present tense (so no need to say \"Currently X is Y\", or\n \"Previously X was Y\" to describe the state before your\n change; just \"X is Y\" is enough), and discuss what you\n perceive as a problem in it.\n\nI just did this, and it worked just fine as expected.\n\n $ git clone https://git.kernel.org/pub/scm/git/git.git/ git\n $ mv git/.git git.git\n $ ln -s ../git.git git/.git\n $ git -C git log\n\nEven if there were a problem in making such a symbolic link work,\nthese days we have a textual \"gitdir: $there\" file as an official\nway to let you keep the repository data on a filesystem that is\ndifferent from the filesystem that hosts its working tree.\n\nAnyway, I've always thought that this topic is about addressing the\ntwo NEEDSWORK comments to allow us to give better filesystem problem\ndiagnoses.\n\n> Fix this by distinguishing directories from regular files and other\n> non-regular file types (like FIFOs or sockets) via newly introduced\n> error_code.\n\nSo, \"Fix this\" written here does not resonate with my understanding\nof what we have been discussing so far. Puzzled.\n\n> diff --git a/setup.c b/setup.c\n> index c8336eb20e..2869d10669 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -897,10 +897,14 @@ int verify_repository_format(const struct repository_format *format,\n> void read_gitfile_error_die(int error_code, const char *path, const char *dir)\n> {\n> \tswitch (error_code) {\n> -\tcase READ_GITFILE_ERR_STAT_FAILED:\n> -\tcase READ_GITFILE_ERR_NOT_A_FILE:\n> +\tcase READ_GITFILE_ERR_STAT_ENOENT:\n> +\tcase READ_GITFILE_ERR_IS_A_DIR:\n> \t\t/* non-fatal; follow return path */\n> \t\tbreak;\n> +\tcase READ_GITFILE_ERR_STAT_FAILED:\n> +\t\tdie(_(\"error reading %s\"), path);\n> +\tcase READ_GITFILE_ERR_NOT_A_FILE:\n> +\t\tdie(_(\"not a regular file: %s\"), path);\n> \tcase READ_GITFILE_ERR_OPEN_FAILED:\n> \t\tdie_errno(_(\"error opening '%s'\"), path);\n> \tcase READ_GITFILE_ERR_TOO_LARGE:\n> @@ -941,8 +945,14 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n> \tstatic struct strbuf realpath = STRBUF_INIT;\n> \n> \tif (stat(path, &st)) {\n> -\t\t/* NEEDSWORK: discern between ENOENT vs other errors */\n> -\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n> +\t\tif (errno == ENOENT)\n> +\t\t\terror_code = READ_GITFILE_ERR_STAT_ENOENT;\n> +\t\telse\n> +\t\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n> +\t\tgoto cleanup_return;\n> +\t}\n> +\tif (S_ISDIR(st.st_mode)) {\n> +\t\terror_code = READ_GITFILE_ERR_IS_A_DIR;\n> \t\tgoto cleanup_return;\n> \t}\n> \tif (!S_ISREG(st.st_mode)) {\n\nAll of the above are exactly what I expected to see. Nice.\n\n> @@ -1578,20 +1588,28 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n> \t\tif (offset > min_offset)\n> \t\t\tstrbuf_addch(dir, '/');\n> \t\tstrbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);\n> -\t\tgitdirenv = read_gitfile_gently(dir->buf, die_on_error ?\n> -\t\t\t\t\t\tNULL : &error_code);\n> +\t\tgitdirenv = read_gitfile_gently(dir->buf, &error_code);\n> \t\tif (!gitdirenv) {\n> +\t\t\tswitch (error_code) {\n> +\t\t\tcase READ_GITFILE_ERR_STAT_ENOENT:\n> +\t\t\t\t/* no .git in this directory, move on */\n> +\t\t\t\tbreak;\n> +\t\t\tcase READ_GITFILE_ERR_IS_A_DIR:\n> \t\t\t\tif (is_git_directory(dir->buf)) {\n> \t\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n> \t\t\t\t\tgitdir_path = xstrdup(dir->buf);\n> \t\t\t\t}\n> +\t\t\t\t/* otherwise, it is an empty/unrelated directory, move on */\n> +\t\t\t\tbreak;\n\nThis design decision may be debatable, but not tightening everything\nat once may be a prudent thing to do to avoid accidental regression.\n\nHaving said that.\n\nIf you have a directory \".git/\" somewhere in your working tree, and\nthe directory is somehow corrupt that is_git_directory() says \"nope,\nthat is not a valid Git directory\", wouldn't you rather want to know\nabout it as a potential problem? Perhaps there are valid use cases\nto have such a directory (you have \"empty\" listed here, which may or\nmay not have a valid use case), but it smells like falling into the\nsame bucket as \"Gee we have .git that is a fifo here---what is going\non???\", which is ...\n\n> +\t\t\tdefault:\n> +\t\t\t\tif (die_on_error || error_code == READ_GITFILE_ERR_NOT_A_FILE)\n> +\t\t\t\t\tread_gitfile_error_die(error_code, dir->buf, NULL);\n> +\t\t\t\telse\n> +\t\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n\n... what we do here.\n\n\nSo, I dunno.\n\n> +\t\t\t}\n> +\t\t} else {\n> \t\t\tgitfile = xstrdup(dir->buf);\n> +\t\t}\n"},{"id":"536577","messageId":"60e4cbcd-6dfe-4e1a-9c63-be905c815bed@gmail.com","threadId":"65013","inReplyTo":"xmqqfr6vxpkn.fsf@gitster.g","subject":"Re: [PATCH v8] setup: allow cwd/.git to be a symlink to a directory","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-21T08:10:49Z","receivedAt":"2026-02-21T08:10:53Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi junio,\n\n> So, \"Fix this\" written here does not resonate with my understanding\n> of what we have been discussing so far. Puzzled.\n\nYou are completely right to be puzzled.\n\nIn an earlier iteration of this patch, another reviewer brought up a \ncase regarding symlinks. I adjusted the logic and wrote the commit \nmessage around that \"symlinks\" narrative.\n\nThrough the subsequent refactors up to v8, the code evolved to tackle \nthe much deeper issue-addressing the two decade-old 'NEEDSWORK' \ncomments. The code structure was completely rewritten but I forgot that \nthe symlinks stuff was not relevant anymore.\n\nYou are right. The actual value of this patch is the error code \nrefactoring, not fixing symlinks.\n\n> All of the above are exactly what I expected to see. Nice.\n\nI'm glad I finally got it right ;)\n\n> This design decision may be debatable, but not tightening everything\n> at once may be a prudent thing to do to avoid accidental regression.\n> \n> Having said that.\n>\n> If you have a directory \".git/\" somewhere in your working tree, and\n> the directory is somehow corrupt that is_git_directory() says \"nope,\n> that is not a valid Git directory\", wouldn't you rather want to know\n> about it as a potential problem?\n\nGreat point. A corrupt '.git' dir is definitely a red flag. However, \nsilently ignoring it and moving on has been the historical behavior, \nhasn't it?\n\nStill, if we decide to tighten this in the future, it will be very \nsimple change within this new 'switch' structure. Nothing much to worry \nabout IMO.\n\nWill send v9 soon, with commit message rewritten.\n\nRegards,\n\nYuchen\n\n"},{"id":"536578","messageId":"20260221083001.220061-1-a3205153416@gmail.com","threadId":"65013","inReplyTo":"20260220164512.216901-1-a3205153416@gmail.com","subject":"[PATCH v9] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-21T08:30:01Z","receivedAt":"2026-02-21T08:30:08Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"'read_gitfile_gently()' treats any non-regular file as\n'READ_GITFILE_ERR_NOT_A_FILE' and fails to discern between 'ENOENT'\nand other stat failures. This flawed error reporting is noted by two\n'NEEDSWORK' comments.\n\nAddress these comments by introducing two new error codes:\n'READ_GITFILE_ERR_STAT_ENOENT' and 'READ_GITFILE_ERR_IS_A_DIR'.\n\nTo preserve the original intent of the setup process:\n1. Update 'read_gitfile_error_die()' to treat 'IS_A_DIR' as a no-op\n (like 'ENOENT'), while still calling 'die()' on true 'NOT_A_FILE'\n errors.\n2. Unconditionally pass '&error_code' to 'read_gitfile_gently()'. This\n eliminates an uninitialized variable hazard that occurred when\n 'die_on_error' was true and 'NULL' was passed.\n3. Only invoke 'is_git_directory()' when we explicitly receive\n 'READ_GITFILE_ERR_IS_A_DIR', avoiding redundant filesystem checks.\n4. Correctly return 'GIT_DIR_INVALID_GITFILE' on unrecognized errors\n when 'die_on_error' is false.\n\nSigned-off-by: Tian Yuchen \n---\n setup.c | 42 ++++++++++++++------\n setup.h | 2 +\n t/meson.build | 1 +\n t/t0009-git-dir-validation.sh | 72 +++++++++++++++++++++++++++++++++++\n 4 files changed, 105 insertions(+), 12 deletions(-)\n create mode 100755 t/t0009-git-dir-validation.sh\n\ndiff --git a/setup.c b/setup.c\nindex c8336eb20e..2869d10669 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -897,10 +897,14 @@ int verify_repository_format(const struct repository_format *format,\n void read_gitfile_error_die(int error_code, const char *path, const char *dir)\n {\n \tswitch (error_code) {\n-\tcase READ_GITFILE_ERR_STAT_FAILED:\n-\tcase READ_GITFILE_ERR_NOT_A_FILE:\n+\tcase READ_GITFILE_ERR_STAT_ENOENT:\n+\tcase READ_GITFILE_ERR_IS_A_DIR:\n \t\t/* non-fatal; follow return path */\n \t\tbreak;\n+\tcase READ_GITFILE_ERR_STAT_FAILED:\n+\t\tdie(_(\"error reading %s\"), path);\n+\tcase READ_GITFILE_ERR_NOT_A_FILE:\n+\t\tdie(_(\"not a regular file: %s\"), path);\n \tcase READ_GITFILE_ERR_OPEN_FAILED:\n \t\tdie_errno(_(\"error opening '%s'\"), path);\n \tcase READ_GITFILE_ERR_TOO_LARGE:\n@@ -941,8 +945,14 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n \tstatic struct strbuf realpath = STRBUF_INIT;\n \n \tif (stat(path, &st)) {\n-\t\t/* NEEDSWORK: discern between ENOENT vs other errors */\n-\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n+\t\tif (errno == ENOENT)\n+\t\t\terror_code = READ_GITFILE_ERR_STAT_ENOENT;\n+\t\telse\n+\t\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n+\t\tgoto cleanup_return;\n+\t}\n+\tif (S_ISDIR(st.st_mode)) {\n+\t\terror_code = READ_GITFILE_ERR_IS_A_DIR;\n \t\tgoto cleanup_return;\n \t}\n \tif (!S_ISREG(st.st_mode)) {\n@@ -1578,20 +1588,28 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n \t\tif (offset > min_offset)\n \t\t\tstrbuf_addch(dir, '/');\n \t\tstrbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);\n-\t\tgitdirenv = read_gitfile_gently(dir->buf, die_on_error ?\n-\t\t\t\t\t\tNULL : &error_code);\n+\t\tgitdirenv = read_gitfile_gently(dir->buf, &error_code);\n \t\tif (!gitdirenv) {\n-\t\t\tif (die_on_error ||\n-\t\t\t error_code == READ_GITFILE_ERR_NOT_A_FILE) {\n-\t\t\t\t/* NEEDSWORK: fail if .git is not file nor dir */\n+\t\t\tswitch (error_code) {\n+\t\t\tcase READ_GITFILE_ERR_STAT_ENOENT:\n+\t\t\t\t/* no .git in this directory, move on */\n+\t\t\t\tbreak;\n+\t\t\tcase READ_GITFILE_ERR_IS_A_DIR:\n \t\t\t\tif (is_git_directory(dir->buf)) {\n \t\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n \t\t\t\t\tgitdir_path = xstrdup(dir->buf);\n \t\t\t\t}\n-\t\t\t} else if (error_code != READ_GITFILE_ERR_STAT_FAILED)\n-\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n-\t\t} else\n+\t\t\t\t/* otherwise, it is an empty/unrelated directory, move on */\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\tif (die_on_error || error_code == READ_GITFILE_ERR_NOT_A_FILE)\n+\t\t\t\t\tread_gitfile_error_die(error_code, dir->buf, NULL);\n+\t\t\t\telse\n+\t\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n+\t\t\t}\n+\t\t} else {\n \t\t\tgitfile = xstrdup(dir->buf);\n+\t\t}\n \t\t/*\n \t\t * Earlier, we tentatively added DEFAULT_GIT_DIR_ENVIRONMENT\n \t\t * to check that directory for a repository.\ndiff --git a/setup.h b/setup.h\nindex 0738dec244..ed4b13f061 100644\n--- a/setup.h\n+++ b/setup.h\n@@ -36,6 +36,8 @@ int is_nonbare_repository_dir(struct strbuf *path);\n #define READ_GITFILE_ERR_NO_PATH 6\n #define READ_GITFILE_ERR_NOT_A_REPO 7\n #define READ_GITFILE_ERR_TOO_LARGE 8\n+#define READ_GITFILE_ERR_STAT_ENOENT 9\n+#define READ_GITFILE_ERR_IS_A_DIR 10\n void read_gitfile_error_die(int error_code, const char *path, const char *dir);\n const char *read_gitfile_gently(const char *path, int *return_error_code);\n #define read_gitfile(path) read_gitfile_gently((path), NULL)\ndiff --git a/t/meson.build b/t/meson.build\nindex f80e366cff..c4afaacee5 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -80,6 +80,7 @@ integration_tests = [\n 't0006-date.sh',\n 't0007-git-var.sh',\n 't0008-ignores.sh',\n+ 't0009-git-dir-validation.sh',\n 't0010-racy-git.sh',\n 't0012-help.sh',\n 't0013-sha1dc.sh',\ndiff --git a/t/t0009-git-dir-validation.sh b/t/t0009-git-dir-validation.sh\nnew file mode 100755\nindex 0000000000..9b3925c85f\n--- /dev/null\n+++ b/t/t0009-git-dir-validation.sh\n@@ -0,0 +1,72 @@\n+#!/bin/sh\n+\n+test_description='setup: validation of .git file/directory types\n+\n+Verify that setup_git_directory() correctly handles:\n+1. Valid .git directories (including symlinks to them).\n+2. Invalid .git files (FIFOs, sockets) by erroring out.\n+3. Invalid .git files (garbage) by erroring out.\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup: create parent git repository' '\n+\tgit init parent &&\n+\ttest_commit -C parent \"root-commit\"\n+'\n+\n+test_expect_success SYMLINKS 'setup: .git as a symlink to a directory is valid' '\n+\tmkdir -p parent/link-to-dir &&\n+\t(\n+\t\tcd parent/link-to-dir &&\n+\t\tgit init real-repo &&\n+\t\tln -s real-repo/.git .git &&\n+\t\tgit rev-parse --git-dir >actual &&\n+\t\techo .git >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success PIPE 'setup: .git as a FIFO (named pipe) is rejected' '\n+\tmkdir -p parent/fifo-trap &&\n+\t(\n+\t\tcd parent/fifo-trap &&\n+\t\tmkfifo .git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"not a regular file\" stderr\n+\t)\n+'\n+\n+test_expect_success SYMLINKS,PIPE 'setup: .git as a symlink to a FIFO is rejected' '\n+\tmkdir -p parent/symlink-fifo-trap &&\n+\t(\n+\t\tcd parent/symlink-fifo-trap &&\n+\t\tmkfifo target-fifo &&\n+\t\tln -s target-fifo .git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"not a regular file\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: .git with garbage content is rejected' '\n+\tmkdir -p parent/garbage-trap &&\n+\t(\n+\t\tcd parent/garbage-trap &&\n+\t\techo \"garbage\" >.git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"invalid gitfile format\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: .git as an empty directory is ignored' '\n+\tmkdir -p parent/empty-dir &&\n+\t(\n+\t\tcd parent/empty-dir &&\n+\t\tmkdir .git &&\n+\t\tgit rev-parse --git-dir >actual &&\n+\t\techo \"$TRASH_DIRECTORY/parent/.git\" >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_done\n-- \n2.43.0\n\n"},{"id":"536598","messageId":"xmqqqzqerp21.fsf@gitster.g","threadId":"65013","inReplyTo":"60e4cbcd-6dfe-4e1a-9c63-be905c815bed@gmail.com","subject":"Re: [PATCH v8] setup: allow cwd/.git to be a symlink to a directory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-21T17:20:38Z","receivedAt":"2026-02-21T17:20:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen writes:\n\n>> This design decision may be debatable, but not tightening everything\n>> at once may be a prudent thing to do to avoid accidental regression.\n>> \n>> Having said that.\n>>\n>> If you have a directory \".git/\" somewhere in your working tree, and\n>> the directory is somehow corrupt that is_git_directory() says \"nope,\n>> that is not a valid Git directory\", wouldn't you rather want to know\n>> about it as a potential problem?\n>\n> Great point. A corrupt '.git' dir is definitely a red flag. However, \n> silently ignoring it and moving on has been the historical behavior, \n> hasn't it?\n\nExactly. That is where my reference to \"not tightening everything\nat once\" comes from.\n\n> Still, if we decide to tighten this in the future, it will be very \n> simple change within this new 'switch' structure. Nothing much to worry \n> about IMO.\n\nYup. Perhaps it would deserve a new \"/* NEEDSWORK: should we catch a\ndirectory .git that is not a git directory here? */\" comment there.\n\n> Will send v9 soon, with commit message rewritten.\n\nThanks.\n"},{"id":"536627","messageId":"b2f0153e-8401-437c-bec5-333b3375782b@gmail.com","threadId":"65013","inReplyTo":"xmqqqzqerp21.fsf@gitster.g","subject":"Re: [PATCH v8] setup: allow cwd/.git to be a symlink to a directory","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-22T03:22:17Z","receivedAt":"2026-02-22T03:22:21Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi Junio,\n\nThanks for the review.\n> Exactly. That is where my reference to \"not tightening everything\n> at once\" comes from.\n\nHappy that I'm on the right track.\n\n> Yup. Perhaps it would deserve a new \"/* NEEDSWORK: should we catch a\n> directory .git that is not a git directory here? */\" comment there.\n\nIndeed. Will add soon.\n\nRegards,\n\nYuchen\n\n"},{"id":"536631","messageId":"xmqqseatqqpr.fsf@gitster.g","threadId":"65013","inReplyTo":"20260221083001.220061-1-a3205153416@gmail.com","subject":"Re: [PATCH v9] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-22T05:42:24Z","receivedAt":"2026-02-22T05:42:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen writes:\n\n> void read_gitfile_error_die(int error_code, const char *path, const char *dir)\n> {\n> \tswitch (error_code) {\n> -\tcase READ_GITFILE_ERR_STAT_FAILED:\n> -\tcase READ_GITFILE_ERR_NOT_A_FILE:\n> +\tcase READ_GITFILE_ERR_STAT_ENOENT:\n> +\tcase READ_GITFILE_ERR_IS_A_DIR:\n> \t\t/* non-fatal; follow return path */\n> \t\tbreak;\n> +\tcase READ_GITFILE_ERR_STAT_FAILED:\n> +\t\tdie(_(\"error reading %s\"), path);\n> +\tcase READ_GITFILE_ERR_NOT_A_FILE:\n> +\t\tdie(_(\"not a regular file: %s\"), path);\n> \tcase READ_GITFILE_ERR_OPEN_FAILED:\n> \t\tdie_errno(_(\"error opening '%s'\"), path);\n> \tcase READ_GITFILE_ERR_TOO_LARGE:\n\nThe changes to these two functions require us to audit callers of\nthem that are outside the call graph of the main focus of this\npatch. For example, we see the following code in submodule.c:\n\n void absorb_git_dir_into_superproject(const char *path,\n const char *super_prefix)\n {\n int err_code;\n const char *sub_git_dir;\n struct strbuf gitdir = STRBUF_INIT;\n\n if (validate_submodule_path(path) < 0)\n exit(128);\n\n strbuf_addf(&gitdir, \"%s/.git\", path);\n sub_git_dir = resolve_gitdir_gently(gitdir.buf, &err_code);\n\n /* Not populated? */\n if (!sub_git_dir) {\n const struct submodule *sub;\n struct strbuf sub_gitdir = STRBUF_INIT;\n\n if (err_code == READ_GITFILE_ERR_STAT_FAILED) {\n /* unpopulated as expected */\n strbuf_release(&gitdir);\n return;\n }\n\n if (err_code != READ_GITFILE_ERR_NOT_A_REPO)\n /* We don't know what broke here. */\n read_gitfile_error_die(err_code, path, NULL);\n\nLet's take a look.\n\nERR_STAT_FAILED used to be \"If we got ENOENT, it is an unpopulated\nsubmodule so it is OK; it is so unlikely that we get other kinds of\nfailure and we cannot deal with them anyway\".\n\nWith the patch we have been looking at, shouldn't the above code\ncheck for ERR_STAT_ENOENT instead, to do the same \"unpopulated,\nnothing to do here, happy!\" return?\n\nThis is merely just one example. All hits from \"git grep\" for the\nfunctions whose semantics have been updated by the patch must be\nlooked at in a similar way, but I didn't look at how many releavnt\ncode paths there are. It could be an easier way to find code paths\nthat may need to be updated to grep for ERR_STAT_FAILED and\nERR_NOT_A_FILE, the two symbols whose meaning have changed.\n\nThanks.\n\n"},{"id":"536640","messageId":"9d79bff0-eb97-4332-8260-236761c9c3da@gmail.com","threadId":"65013","inReplyTo":"xmqqseatqqpr.fsf@gitster.g","subject":"Re: [PATCH v9] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-22T10:28:41Z","receivedAt":"2026-02-22T10:28:45Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi Junio,\n\nOn 2/22/26 13:42, Junio C Hamano wrote:\n\n> The changes to these two functions require us to audit callers of\n> them that are outside the call graph of the main focus of this\n> patch. For example, we see the following code in submodule.c:\n\nOops, I was so focused on setup.c that I completely overlooked the \nexternal callers of the API. Thank you for pointing out!\n\nI ran git grep for READ_GITFILE_ERR_STAT_FAILED and \nREAD_GITFILE_ERR_NOT_A_FILE across the tree and audited the hits. \nBesides the expected changes in setup.c, here is what I found and fixed:\n\n - In submodule.c, exactly as you pointed out.\n - In worktree.c there were two places relying on NOT_A_FILE to print \nthe specific \".git is not a file\" error. I added \nREAD_GITFILE_ERR_IS_A_DIR to those conditions to prevent the error from \ndegrading to the generic \".git file broken\".\n\nI have also added the NEEDSWORK comment in the switch statedment, as you \nmentioned earlier.\n\nThanks,\n\nYuchen\n\n\n"},{"id":"536641","messageId":"20260222102928.377519-1-a3205153416@gmail.com","threadId":"65013","inReplyTo":"20260221083001.220061-1-a3205153416@gmail.com","subject":"[PATCH v10] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-22T10:29:28Z","receivedAt":"2026-02-22T10:29:34Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"'read_gitfile_gently()' treats any non-regular file as\n'READ_GITFILE_ERR_NOT_A_FILE' and fails to discern between 'ENOENT'\nand other stat failures. This flawed error reporting is noted by two\n'NEEDSWORK' comments.\n\nAddress these comments by introducing two new error codes:\n'READ_GITFILE_ERR_STAT_ENOENT' and 'READ_GITFILE_ERR_IS_A_DIR'.\n\nTo preserve the original intent of the setup process:\n1. Update 'read_gitfile_error_die()' to treat 'IS_A_DIR' as a no-op\n (like 'ENOENT'), while still calling 'die()' on true 'NOT_A_FILE'\n errors.\n2. Unconditionally pass '&error_code' to 'read_gitfile_gently()'. This\n eliminates an uninitialized variable hazard that occurred when\n 'die_on_error' was true and 'NULL' was passed.\n3. Only invoke 'is_git_directory()' when we explicitly receive\n 'READ_GITFILE_ERR_IS_A_DIR', avoiding redundant filesystem checks.\n4. Correctly return 'GIT_DIR_INVALID_GITFILE' on unrecognized errors\n when 'die_on_error' is false.\n\nAdditionally, audit external callers of 'read_gitfile_gently()' in\n'submodule.c' and 'worktree.c' to accommodate the refined error codes.\n\nSigned-off-by: Tian Yuchen \n---\n setup.c | 42 ++++++++++++++------\n setup.h | 2 +\n submodule.c | 2 +-\n t/meson.build | 1 +\n t/t0009-git-dir-validation.sh | 72 +++++++++++++++++++++++++++++++++++\n worktree.c | 6 ++-\n 6 files changed, 110 insertions(+), 15 deletions(-)\n create mode 100755 t/t0009-git-dir-validation.sh\n\ndiff --git a/setup.c b/setup.c\nindex c8336eb20e..9d49b9ae53 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -897,10 +897,14 @@ int verify_repository_format(const struct repository_format *format,\n void read_gitfile_error_die(int error_code, const char *path, const char *dir)\n {\n \tswitch (error_code) {\n-\tcase READ_GITFILE_ERR_STAT_FAILED:\n-\tcase READ_GITFILE_ERR_NOT_A_FILE:\n+\tcase READ_GITFILE_ERR_STAT_ENOENT:\n+\tcase READ_GITFILE_ERR_IS_A_DIR:\n \t\t/* non-fatal; follow return path */\n \t\tbreak;\n+\tcase READ_GITFILE_ERR_STAT_FAILED:\n+\t\tdie(_(\"error reading %s\"), path);\n+\tcase READ_GITFILE_ERR_NOT_A_FILE:\n+\t\tdie(_(\"not a regular file: %s\"), path);\n \tcase READ_GITFILE_ERR_OPEN_FAILED:\n \t\tdie_errno(_(\"error opening '%s'\"), path);\n \tcase READ_GITFILE_ERR_TOO_LARGE:\n@@ -941,8 +945,14 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n \tstatic struct strbuf realpath = STRBUF_INIT;\n \n \tif (stat(path, &st)) {\n-\t\t/* NEEDSWORK: discern between ENOENT vs other errors */\n-\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n+\t\tif (errno == ENOENT)\n+\t\t\terror_code = READ_GITFILE_ERR_STAT_ENOENT;\n+\t\telse\n+\t\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n+\t\tgoto cleanup_return;\n+\t}\n+\tif (S_ISDIR(st.st_mode)) {\n+\t\terror_code = READ_GITFILE_ERR_IS_A_DIR;\n \t\tgoto cleanup_return;\n \t}\n \tif (!S_ISREG(st.st_mode)) {\n@@ -1578,20 +1588,28 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n \t\tif (offset > min_offset)\n \t\t\tstrbuf_addch(dir, '/');\n \t\tstrbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);\n-\t\tgitdirenv = read_gitfile_gently(dir->buf, die_on_error ?\n-\t\t\t\t\t\tNULL : &error_code);\n+\t\tgitdirenv = read_gitfile_gently(dir->buf, &error_code);\n \t\tif (!gitdirenv) {\n-\t\t\tif (die_on_error ||\n-\t\t\t error_code == READ_GITFILE_ERR_NOT_A_FILE) {\n-\t\t\t\t/* NEEDSWORK: fail if .git is not file nor dir */\n+\t\t\tswitch (error_code) {\n+\t\t\tcase READ_GITFILE_ERR_STAT_ENOENT:\n+\t\t\t\t/* no .git in this directory, move on */\n+\t\t\t\tbreak;\n+\t\t\tcase READ_GITFILE_ERR_IS_A_DIR:\n \t\t\t\tif (is_git_directory(dir->buf)) {\n \t\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n \t\t\t\t\tgitdir_path = xstrdup(dir->buf);\n \t\t\t\t}\n-\t\t\t} else if (error_code != READ_GITFILE_ERR_STAT_FAILED)\n-\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n-\t\t} else\n+\t\t\t\t/* NEEDSWORK: should we catch a directory .git that is not a git directory here? */\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\tif (die_on_error || error_code == READ_GITFILE_ERR_NOT_A_FILE)\n+\t\t\t\t\tread_gitfile_error_die(error_code, dir->buf, NULL);\n+\t\t\t\telse\n+\t\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n+\t\t\t}\n+\t\t} else {\n \t\t\tgitfile = xstrdup(dir->buf);\n+\t\t}\n \t\t/*\n \t\t * Earlier, we tentatively added DEFAULT_GIT_DIR_ENVIRONMENT\n \t\t * to check that directory for a repository.\ndiff --git a/setup.h b/setup.h\nindex 0738dec244..ed4b13f061 100644\n--- a/setup.h\n+++ b/setup.h\n@@ -36,6 +36,8 @@ int is_nonbare_repository_dir(struct strbuf *path);\n #define READ_GITFILE_ERR_NO_PATH 6\n #define READ_GITFILE_ERR_NOT_A_REPO 7\n #define READ_GITFILE_ERR_TOO_LARGE 8\n+#define READ_GITFILE_ERR_STAT_ENOENT 9\n+#define READ_GITFILE_ERR_IS_A_DIR 10\n void read_gitfile_error_die(int error_code, const char *path, const char *dir);\n const char *read_gitfile_gently(const char *path, int *return_error_code);\n #define read_gitfile(path) read_gitfile_gently((path), NULL)\ndiff --git a/submodule.c b/submodule.c\nindex 508938e4da..b179f952fb 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2559,7 +2559,7 @@ void absorb_git_dir_into_superproject(const char *path,\n \t\tconst struct submodule *sub;\n \t\tstruct strbuf sub_gitdir = STRBUF_INIT;\n \n-\t\tif (err_code == READ_GITFILE_ERR_STAT_FAILED) {\n+\t\tif (err_code == READ_GITFILE_ERR_STAT_ENOENT) {\n \t\t\t/* unpopulated as expected */\n \t\t\tstrbuf_release(&gitdir);\n \t\t\treturn;\ndiff --git a/t/meson.build b/t/meson.build\nindex f80e366cff..c4afaacee5 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -80,6 +80,7 @@ integration_tests = [\n 't0006-date.sh',\n 't0007-git-var.sh',\n 't0008-ignores.sh',\n+ 't0009-git-dir-validation.sh',\n 't0010-racy-git.sh',\n 't0012-help.sh',\n 't0013-sha1dc.sh',\ndiff --git a/t/t0009-git-dir-validation.sh b/t/t0009-git-dir-validation.sh\nnew file mode 100755\nindex 0000000000..9b3925c85f\n--- /dev/null\n+++ b/t/t0009-git-dir-validation.sh\n@@ -0,0 +1,72 @@\n+#!/bin/sh\n+\n+test_description='setup: validation of .git file/directory types\n+\n+Verify that setup_git_directory() correctly handles:\n+1. Valid .git directories (including symlinks to them).\n+2. Invalid .git files (FIFOs, sockets) by erroring out.\n+3. Invalid .git files (garbage) by erroring out.\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup: create parent git repository' '\n+\tgit init parent &&\n+\ttest_commit -C parent \"root-commit\"\n+'\n+\n+test_expect_success SYMLINKS 'setup: .git as a symlink to a directory is valid' '\n+\tmkdir -p parent/link-to-dir &&\n+\t(\n+\t\tcd parent/link-to-dir &&\n+\t\tgit init real-repo &&\n+\t\tln -s real-repo/.git .git &&\n+\t\tgit rev-parse --git-dir >actual &&\n+\t\techo .git >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success PIPE 'setup: .git as a FIFO (named pipe) is rejected' '\n+\tmkdir -p parent/fifo-trap &&\n+\t(\n+\t\tcd parent/fifo-trap &&\n+\t\tmkfifo .git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"not a regular file\" stderr\n+\t)\n+'\n+\n+test_expect_success SYMLINKS,PIPE 'setup: .git as a symlink to a FIFO is rejected' '\n+\tmkdir -p parent/symlink-fifo-trap &&\n+\t(\n+\t\tcd parent/symlink-fifo-trap &&\n+\t\tmkfifo target-fifo &&\n+\t\tln -s target-fifo .git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"not a regular file\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: .git with garbage content is rejected' '\n+\tmkdir -p parent/garbage-trap &&\n+\t(\n+\t\tcd parent/garbage-trap &&\n+\t\techo \"garbage\" >.git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"invalid gitfile format\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: .git as an empty directory is ignored' '\n+\tmkdir -p parent/empty-dir &&\n+\t(\n+\t\tcd parent/empty-dir &&\n+\t\tmkdir .git &&\n+\t\tgit rev-parse --git-dir >actual &&\n+\t\techo \"$TRASH_DIRECTORY/parent/.git\" >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_done\ndiff --git a/worktree.c b/worktree.c\nindex 9308389cb6..d1165e1d1c 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -653,7 +653,8 @@ static void repair_gitfile(struct worktree *wt,\n \t\t}\n \t}\n \n-\tif (err == READ_GITFILE_ERR_NOT_A_FILE)\n+\tif (err == READ_GITFILE_ERR_NOT_A_FILE ||\n+\t\terr == READ_GITFILE_ERR_IS_A_DIR)\n \t\tfn(1, wt->path, _(\".git is not a file\"), cb_data);\n \telse if (err)\n \t\trepair = _(\".git file broken\");\n@@ -833,7 +834,8 @@ void repair_worktree_at_path(const char *path,\n \t\t\tstrbuf_addstr(&backlink, dotgit_contents);\n \t\t\tstrbuf_realpath_forgiving(&backlink, backlink.buf, 0);\n \t\t}\n-\t} else if (err == READ_GITFILE_ERR_NOT_A_FILE) {\n+\t} else if (err == READ_GITFILE_ERR_NOT_A_FILE ||\n+\t\t\terr == READ_GITFILE_ERR_IS_A_DIR) {\n \t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git is not a file\"), cb_data);\n \t\tgoto done;\n \t} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {\n-- \n2.43.0\n\n"},{"id":"536649","messageId":"CAOLa=ZTePRR05M5VBxxk0OA=_RyNd0pLe=Bq6xwnE3MyZBjBAw@mail.gmail.com","threadId":"65013","inReplyTo":"20260222102928.377519-1-a3205153416@gmail.com","subject":"Re: [PATCH v10] setup: improve error diagnosis for invalid .git files","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-02-22T16:53:35Z","receivedAt":"2026-02-22T16:53:37Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Tian Yuchen writes:\n\n> 'read_gitfile_gently()' treats any non-regular file as\n> 'READ_GITFILE_ERR_NOT_A_FILE' and fails to discern between 'ENOENT'\n> and other stat failures. This flawed error reporting is noted by two\n> 'NEEDSWORK' comments.\n\nOkay.\n\n> Address these comments by introducing two new error codes:\n> 'READ_GITFILE_ERR_STAT_ENOENT' and 'READ_GITFILE_ERR_IS_A_DIR'.\n>\n\nNit: This is much better, we seem to talk about the issues and the new\nerrors introduced. I wonder if we can tie the errors to the issues.\n\nPerhaps\n\n The 'read_gitfile_gently()' is used to obtain the location of a git\n directory by parsing a '.git' file.\n\n When parsing the file with 'stat(2)', it fails to differentiate\n between a 'ENOENT' and other errors. Introduce\n 'READ_GITFILE_ERR_STAT_ENOENT' to make this differentiation.\n\n The function also marks directories as\n 'READ_GITFILE_ERR_NOT_A_FILE', introduce 'READ_GITFILE_ERR_IS_A_DIR'\n to specifically make this distinction.\n\n\n> To preserve the original intent of the setup process:\n> 1. Update 'read_gitfile_error_die()' to treat 'IS_A_DIR' as a no-op\n> (like 'ENOENT'), while still calling 'die()' on true 'NOT_A_FILE'\n> errors.\n\nNice, shouldn't we also mention READ_GITFILE_ERR_STAT_ENOENT is now\ntreated as a no-op, while its counterpart is not.\n\n> 2. Unconditionally pass '&error_code' to 'read_gitfile_gently()'. This\n> eliminates an uninitialized variable hazard that occurred when\n> 'die_on_error' was true and 'NULL' was passed.\n\nWhere is the 'uninitialized variable hazard'? The function says:\n\n If return_error_code is NULL the function will die instead\n\n> 3. Only invoke 'is_git_directory()' when we explicitly receive\n> 'READ_GITFILE_ERR_IS_A_DIR', avoiding redundant filesystem checks.\n\nNice.\n\n> 4. Correctly return 'GIT_DIR_INVALID_GITFILE' on unrecognized errors\n> when 'die_on_error' is false.\n>\n> Additionally, audit external callers of 'read_gitfile_gently()' in\n> 'submodule.c' and 'worktree.c' to accommodate the refined error codes.\n>\n> Signed-off-by: Tian Yuchen \n> ---\n> setup.c | 42 ++++++++++++++------\n> setup.h | 2 +\n> submodule.c | 2 +-\n> t/meson.build | 1 +\n> t/t0009-git-dir-validation.sh | 72 +++++++++++++++++++++++++++++++++++\n> worktree.c | 6 ++-\n> 6 files changed, 110 insertions(+), 15 deletions(-)\n> create mode 100755 t/t0009-git-dir-validation.sh\n>\n\nI couldn't find a discussion, why did we merge the commits?\n\n> diff --git a/setup.c b/setup.c\n> index c8336eb20e..9d49b9ae53 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -897,10 +897,14 @@ int verify_repository_format(const struct repository_format *format,\n> void read_gitfile_error_die(int error_code, const char *path, const char *dir)\n> {\n> \tswitch (error_code) {\n> -\tcase READ_GITFILE_ERR_STAT_FAILED:\n> -\tcase READ_GITFILE_ERR_NOT_A_FILE:\n> +\tcase READ_GITFILE_ERR_STAT_ENOENT:\n> +\tcase READ_GITFILE_ERR_IS_A_DIR:\n> \t\t/* non-fatal; follow return path */\n> \t\tbreak;\n> +\tcase READ_GITFILE_ERR_STAT_FAILED:\n> +\t\tdie(_(\"error reading %s\"), path);\n> +\tcase READ_GITFILE_ERR_NOT_A_FILE:\n> +\t\tdie(_(\"not a regular file: %s\"), path);\n\nNot your fault, but some of the errors quote the path and some don't, it\nwould be nice to be uniform here.\n\n> \tcase READ_GITFILE_ERR_OPEN_FAILED:\n> \t\tdie_errno(_(\"error opening '%s'\"), path);\n> \tcase READ_GITFILE_ERR_TOO_LARGE:\n> @@ -941,8 +945,14 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n> \tstatic struct strbuf realpath = STRBUF_INIT;\n>\n> \tif (stat(path, &st)) {\n> -\t\t/* NEEDSWORK: discern between ENOENT vs other errors */\n> -\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n> +\t\tif (errno == ENOENT)\n> +\t\t\terror_code = READ_GITFILE_ERR_STAT_ENOENT;\n> +\t\telse\n> +\t\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n> +\t\tgoto cleanup_return;\n> +\t}\n> +\tif (S_ISDIR(st.st_mode)) {\n> +\t\terror_code = READ_GITFILE_ERR_IS_A_DIR;\n> \t\tgoto cleanup_return;\n> \t}\n\nThis block makes sense.\n\n> \tif (!S_ISREG(st.st_mode)) {\n> @@ -1578,20 +1588,28 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n> \t\tif (offset > min_offset)\n> \t\t\tstrbuf_addch(dir, '/');\n> \t\tstrbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);\n> -\t\tgitdirenv = read_gitfile_gently(dir->buf, die_on_error ?\n> -\t\t\t\t\t\tNULL : &error_code);\n> +\t\tgitdirenv = read_gitfile_gently(dir->buf, &error_code);\n\nSo we now ask the error code to be provided, even if `die_on_error` is\nset. I assume, we will manually handle `die_on_error`.\n\n> \t\tif (!gitdirenv) {\n> -\t\t\tif (die_on_error ||\n> -\t\t\t error_code == READ_GITFILE_ERR_NOT_A_FILE) {\n> -\t\t\t\t/* NEEDSWORK: fail if .git is not file nor dir */\n> +\t\t\tswitch (error_code) {\n> +\t\t\tcase READ_GITFILE_ERR_STAT_ENOENT:\n> +\t\t\t\t/* no .git in this directory, move on */\n> +\t\t\t\tbreak;\n> +\t\t\tcase READ_GITFILE_ERR_IS_A_DIR:\n> \t\t\t\tif (is_git_directory(dir->buf)) {\n> \t\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n> \t\t\t\t\tgitdir_path = xstrdup(dir->buf);\n> \t\t\t\t}\n> -\t\t\t} else if (error_code != READ_GITFILE_ERR_STAT_FAILED)\n> -\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n> -\t\t} else\n> +\t\t\t\t/* NEEDSWORK: should we catch a directory .git that is not a git directory here? */\n\nNit: we should probably wrap this.\n\n> +\t\t\t\tbreak;\n> +\t\t\tdefault:\n> +\t\t\t\tif (die_on_error || error_code == READ_GITFILE_ERR_NOT_A_FILE)\n> +\t\t\t\t\tread_gitfile_error_die(error_code, dir->buf, NULL);\n> +\t\t\t\telse\n> +\t\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n> +\t\t\t}\n> +\t\t} else {\n> \t\t\tgitfile = xstrdup(dir->buf);\n> +\t\t}\n\nThe changes themselves make sense to me.\n\n> \t\t/*\n> \t\t * Earlier, we tentatively added DEFAULT_GIT_DIR_ENVIRONMENT\n> \t\t * to check that directory for a repository.\n> diff --git a/setup.h b/setup.h\n> index 0738dec244..ed4b13f061 100644\n> --- a/setup.h\n> +++ b/setup.h\n> @@ -36,6 +36,8 @@ int is_nonbare_repository_dir(struct strbuf *path);\n> #define READ_GITFILE_ERR_NO_PATH 6\n> #define READ_GITFILE_ERR_NOT_A_REPO 7\n> #define READ_GITFILE_ERR_TOO_LARGE 8\n> +#define READ_GITFILE_ERR_STAT_ENOENT 9\n> +#define READ_GITFILE_ERR_IS_A_DIR 10\n> void read_gitfile_error_die(int error_code, const char *path, const char *dir);\n> const char *read_gitfile_gently(const char *path, int *return_error_code);\n> #define read_gitfile(path) read_gitfile_gently((path), NULL)\n> diff --git a/submodule.c b/submodule.c\n> index 508938e4da..b179f952fb 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -2559,7 +2559,7 @@ void absorb_git_dir_into_superproject(const char *path,\n> \t\tconst struct submodule *sub;\n> \t\tstruct strbuf sub_gitdir = STRBUF_INIT;\n>\n> -\t\tif (err_code == READ_GITFILE_ERR_STAT_FAILED) {\n> +\t\tif (err_code == READ_GITFILE_ERR_STAT_ENOENT) {\n> \t\t\t/* unpopulated as expected */\n> \t\t\tstrbuf_release(&gitdir);\n> \t\t\treturn;\n> diff --git a/t/meson.build b/t/meson.build\n> index f80e366cff..c4afaacee5 100644\n> --- a/t/meson.build\n> +++ b/t/meson.build\n> @@ -80,6 +80,7 @@ integration_tests = [\n> 't0006-date.sh',\n> 't0007-git-var.sh',\n> 't0008-ignores.sh',\n> + 't0009-git-dir-validation.sh',\n> 't0010-racy-git.sh',\n> 't0012-help.sh',\n> 't0013-sha1dc.sh',\n> diff --git a/t/t0009-git-dir-validation.sh b/t/t0009-git-dir-validation.sh\n> new file mode 100755\n> index 0000000000..9b3925c85f\n> --- /dev/null\n> +++ b/t/t0009-git-dir-validation.sh\n> @@ -0,0 +1,72 @@\n> +#!/bin/sh\n> +\n> +test_description='setup: validation of .git file/directory types\n> +\n> +Verify that setup_git_directory() correctly handles:\n> +1. Valid .git directories (including symlinks to them).\n> +2. Invalid .git files (FIFOs, sockets) by erroring out.\n> +3. Invalid .git files (garbage) by erroring out.\n> +'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'setup: create parent git repository' '\n> +\tgit init parent &&\n> +\ttest_commit -C parent \"root-commit\"\n> +'\n> +\n> +test_expect_success SYMLINKS 'setup: .git as a symlink to a directory is valid' '\n\nNit: should we also cleanup? with a 'test_when_finished \"rm -rf\nparent/link-to-dir\"'.\nShould apply for all the tests.\n\n> +\tmkdir -p parent/link-to-dir &&\n> +\t(\n> +\t\tcd parent/link-to-dir &&\n> +\t\tgit init real-repo &&\n> +\t\tln -s real-repo/.git .git &&\n> +\t\tgit rev-parse --git-dir >actual &&\n> +\t\techo .git >expect &&\n> +\t\ttest_cmp expect actual\n> +\t)\n> +'\n> +\n> +test_expect_success PIPE 'setup: .git as a FIFO (named pipe) is rejected' '\n> +\tmkdir -p parent/fifo-trap &&\n> +\t(\n> +\t\tcd parent/fifo-trap &&\n> +\t\tmkfifo .git &&\n> +\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n> +\t\tgrep \"not a regular file\" stderr\n> +\t)\n> +'\n> +\n> +test_expect_success SYMLINKS,PIPE 'setup: .git as a symlink to a FIFO is rejected' '\n> +\tmkdir -p parent/symlink-fifo-trap &&\n> +\t(\n> +\t\tcd parent/symlink-fifo-trap &&\n> +\t\tmkfifo target-fifo &&\n> +\t\tln -s target-fifo .git &&\n> +\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n> +\t\tgrep \"not a regular file\" stderr\n> +\t)\n> +'\n> +\n> +test_expect_success 'setup: .git with garbage content is rejected' '\n> +\tmkdir -p parent/garbage-trap &&\n> +\t(\n> +\t\tcd parent/garbage-trap &&\n> +\t\techo \"garbage\" >.git &&\n> +\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n> +\t\tgrep \"invalid gitfile format\" stderr\n> +\t)\n> +'\n> +\n> +test_expect_success 'setup: .git as an empty directory is ignored' '\n> +\tmkdir -p parent/empty-dir &&\n> +\t(\n> +\t\tcd parent/empty-dir &&\n> +\t\tmkdir .git &&\n> +\t\tgit rev-parse --git-dir >actual &&\n> +\t\techo \"$TRASH_DIRECTORY/parent/.git\" >expect &&\n> +\t\ttest_cmp expect actual\n> +\t)\n> +'\n> +\n> +test_done\n> diff --git a/worktree.c b/worktree.c\n> index 9308389cb6..d1165e1d1c 100644\n> --- a/worktree.c\n> +++ b/worktree.c\n> @@ -653,7 +653,8 @@ static void repair_gitfile(struct worktree *wt,\n> \t\t}\n> \t}\n>\n> -\tif (err == READ_GITFILE_ERR_NOT_A_FILE)\n> +\tif (err == READ_GITFILE_ERR_NOT_A_FILE ||\n> +\t\terr == READ_GITFILE_ERR_IS_A_DIR)\n> \t\tfn(1, wt->path, _(\".git is not a file\"), cb_data);\n> \telse if (err)\n> \t\trepair = _(\".git file broken\");\n> @@ -833,7 +834,8 @@ void repair_worktree_at_path(const char *path,\n> \t\t\tstrbuf_addstr(&backlink, dotgit_contents);\n> \t\t\tstrbuf_realpath_forgiving(&backlink, backlink.buf, 0);\n> \t\t}\n> -\t} else if (err == READ_GITFILE_ERR_NOT_A_FILE) {\n> +\t} else if (err == READ_GITFILE_ERR_NOT_A_FILE ||\n> +\t\t\terr == READ_GITFILE_ERR_IS_A_DIR) {\n> \t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git is not a file\"), cb_data);\n> \t\tgoto done;\n> \t} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {\n> --\n> 2.43.0\n\nThe rest looks good. Thanks!\n"},{"id":"536670","messageId":"xmqq4in8quxn.fsf@gitster.g","threadId":"65013","inReplyTo":"20260222102928.377519-1-a3205153416@gmail.com","subject":"Re: [PATCH v10] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-22T22:23:32Z","receivedAt":"2026-02-22T22:23:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen writes:\n\n> 'read_gitfile_gently()' treats any non-regular file as\n> 'READ_GITFILE_ERR_NOT_A_FILE' and fails to discern between 'ENOENT'\n> and other stat failures. This flawed error reporting is noted by two\n> 'NEEDSWORK' comments.\n>\n> Address these comments by introducing two new error codes:\n> 'READ_GITFILE_ERR_STAT_ENOENT' and 'READ_GITFILE_ERR_IS_A_DIR'.\n>\n> To preserve the original intent of the setup process:\n> 1. Update 'read_gitfile_error_die()' to treat 'IS_A_DIR' as a no-op\n> (like 'ENOENT'), while still calling 'die()' on true 'NOT_A_FILE'\n> errors.\n> 2. Unconditionally pass '&error_code' to 'read_gitfile_gently()'. This\n> eliminates an uninitialized variable hazard that occurred when\n> 'die_on_error' was true and 'NULL' was passed.\n> 3. Only invoke 'is_git_directory()' when we explicitly receive\n> 'READ_GITFILE_ERR_IS_A_DIR', avoiding redundant filesystem checks.\n> 4. Correctly return 'GIT_DIR_INVALID_GITFILE' on unrecognized errors\n> when 'die_on_error' is false.\n>\n> Additionally, audit external callers of 'read_gitfile_gently()' in\n> 'submodule.c' and 'worktree.c' to accommodate the refined error codes.\n>\n> Signed-off-by: Tian Yuchen \n> ---\n> setup.c | 42 ++++++++++++++------\n> setup.h | 2 +\n> submodule.c | 2 +-\n> t/meson.build | 1 +\n> t/t0009-git-dir-validation.sh | 72 +++++++++++++++++++++++++++++++++++\n> worktree.c | 6 ++-\n> 6 files changed, 110 insertions(+), 15 deletions(-)\n> create mode 100755 t/t0009-git-dir-validation.sh\n\nWe'd probably need to treat ENOTDIR the same way as ENOENT to deal\nwith cases where we expect a directory \"sm1\" to be the root of a\nsubmodule working tree, and we have a modification that removes the\nsubmodule directory and replace it with a regular file \"sm1\". In\nthe code path touched by this patch in submodule.c, we would ask \"is\nsm1/.git a git directory?\" and the stat(2) call on that path in\nread_gitfile_gently() used to say \"Ah, a failure, that means we\ncannot positively say that 'sm1/.git' is a git directory or a gitdir\nfile.\" Now we inspect the error code in an attempt to tell if it is\na system failure (e.g., a corrupt filesystem), but catching only\nENOENT is probably a bit too tight. In the above scenario, asking\nabout 'sm1/.git' when 'sm1' is a regular file will not result in\nENOENT but in ENOTDIR (i.e., \"the leading 'sm1' is not a directory so\nit makes no sense to ask about 'sm1/.git'\").\n\nIs it always sensible to treat ENOTDIR and ENOENT as two equivalent\nerrors for the purpose of read_gitfile_gently()? I have no clear\nanswer offhand myself. This is part of what we need to think about\nand resolve while addressing the original \"NEEDSWORK:\" comment.\n\n"},{"id":"536676","messageId":"xmqqqzqcpatz.fsf@gitster.g","threadId":"65013","inReplyTo":"xmqq4in8quxn.fsf@gitster.g","subject":"Re: [PATCH v10] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-23T00:23:04Z","receivedAt":"2026-02-23T00:23:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> setup.c | 42 ++++++++++++++------\n>> setup.h | 2 +\n>> submodule.c | 2 +-\n>> t/meson.build | 1 +\n>> t/t0009-git-dir-validation.sh | 72 +++++++++++++++++++++++++++++++++++\n>> worktree.c | 6 ++-\n>> 6 files changed, 110 insertions(+), 15 deletions(-)\n>> create mode 100755 t/t0009-git-dir-validation.sh\n>\n> We'd probably need to treat ENOTDIR the same way as ENOENT to deal\n> with cases where we expect a directory \"sm1\" to be the root of a\n> submodule working tree, and we have a modification that removes the\n> submodule directory and replace it with a regular file \"sm1\". In\n> the code path touched by this patch in submodule.c, we would ask \"is\n> sm1/.git a git directory?\" and the stat(2) call on that path in\n> read_gitfile_gently() used to say \"Ah, a failure, that means we\n> cannot positively say that 'sm1/.git' is a git directory or a gitdir\n> file.\" Now we inspect the error code in an attempt to tell if it is\n> a system failure (e.g., a corrupt filesystem), but catching only\n> ENOENT is probably a bit too tight. In the above scenario, asking\n> about 'sm1/.git' when 'sm1' is a regular file will not result in\n> ENOENT but in ENOTDIR (i.e., \"the leading 'sm1' is not a directory so\n> it makes no sense to ask about 'sm1/.git'\").\n>\n> Is it always sensible to treat ENOTDIR and ENOENT as two equivalent\n> errors for the purpose of read_gitfile_gently()? I have no clear\n> answer offhand myself. This is part of what we need to think about\n> and resolve while addressing the original \"NEEDSWORK:\" comment.\n\n\n----- >8 -----\nSubject: [PATCH] read_gitfile(): group ENOENT and ENOTDIR into a\n single MISSING error\n\nThe code from the previous step does not deal wellwith a case where\nwe check if \"sm/.git\" is a good directory after replacing \"sm\" with\na regular file. ENOTDIR is returned when we ask about \"sm/.git\",\nnot ENOENT, and the code would want to handle both.\n\nI am not convinced if this is a good change, though. Outside the\nsubmodule caller, the story might be different and we may want to\ntreat ENOTDIR differently from ENOENT. I dunno. That is why this\nis not squashed into the patch (yet).\n\nSigned-off-by: Junio C Hamano \n---\n setup.c | 8 ++++----\n setup.h | 2 +-\n submodule.c | 2 +-\n 3 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex b79a9233f5..d4afbed3eb 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -895,7 +895,7 @@ int verify_repository_format(const struct repository_format *format,\n void read_gitfile_error_die(int error_code, const char *path, const char *dir)\n {\n \tswitch (error_code) {\n-\tcase READ_GITFILE_ERR_STAT_ENOENT:\n+\tcase READ_GITFILE_ERR_STAT_MISSING:\n \tcase READ_GITFILE_ERR_IS_A_DIR:\n \t\t/* non-fatal; follow return path */\n \t\tbreak;\n@@ -943,8 +943,8 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n \tstatic struct strbuf realpath = STRBUF_INIT;\n \n \tif (stat(path, &st)) {\n-\t\tif (errno == ENOENT)\n-\t\t\terror_code = READ_GITFILE_ERR_STAT_ENOENT;\n+\t\tif (errno == ENOENT || errno == ENOTDIR)\n+\t\t\terror_code = READ_GITFILE_ERR_STAT_MISSING;\n \t\telse\n \t\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n \t\tgoto cleanup_return;\n@@ -1589,7 +1589,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n \t\tgitdirenv = read_gitfile_gently(dir->buf, &error_code);\n \t\tif (!gitdirenv) {\n \t\t\tswitch (error_code) {\n-\t\t\tcase READ_GITFILE_ERR_STAT_ENOENT:\n+\t\t\tcase READ_GITFILE_ERR_STAT_MISSING:\n \t\t\t\t/* no .git in this directory, move on */\n \t\t\t\tbreak;\n \t\t\tcase READ_GITFILE_ERR_IS_A_DIR:\ndiff --git a/setup.h b/setup.h\nindex c23629cb4f..cc45f962fa 100644\n--- a/setup.h\n+++ b/setup.h\n@@ -36,7 +36,7 @@ int is_nonbare_repository_dir(struct strbuf *path);\n #define READ_GITFILE_ERR_NO_PATH 6\n #define READ_GITFILE_ERR_NOT_A_REPO 7\n #define READ_GITFILE_ERR_TOO_LARGE 8\n-#define READ_GITFILE_ERR_STAT_ENOENT 9\n+#define READ_GITFILE_ERR_STAT_MISSING 9\n #define READ_GITFILE_ERR_IS_A_DIR 10\n void read_gitfile_error_die(int error_code, const char *path, const char *dir);\n const char *read_gitfile_gently(const char *path, int *return_error_code);\ndiff --git a/submodule.c b/submodule.c\nindex 52a7cf0e43..fc85a7a1d8 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2413,7 +2413,7 @@ void absorb_git_dir_into_superproject(const char *path,\n \t\tconst struct submodule *sub;\n \t\tstruct strbuf sub_gitdir = STRBUF_INIT;\n \n-\t\tif (err_code == READ_GITFILE_ERR_STAT_ENOENT) {\n+\t\tif (err_code == READ_GITFILE_ERR_STAT_MISSING) {\n \t\t\t/* unpopulated as expected */\n \t\t\tstrbuf_release(&gitdir);\n \t\t\treturn;\n-- \n2.53.0-455-g62fcd67e6e\n\n"},{"id":"536686","messageId":"5263825f-163c-43af-bac7-152d670919d9@gmail.com","threadId":"65013","inReplyTo":"xmqqqzqcpatz.fsf@gitster.g","subject":"Re: [PATCH v10] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-23T03:35:46Z","receivedAt":"2026-02-23T03:35:50Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi Junio,\n\n >> We'd probably need to treat ENOTDIR the same way as ENOENT to deal\n >> with cases where we expect a directory \"sm1\" to be the root of a\n >> submodule working tree, and we have a modification that removes the\n >> submodule directory and replace it with a regular file \"sm1\". In\n >> the code path touched by this patch in submodule.c, we would ask \"is\n >> sm1/.git a git directory?\" and the stat(2) call on that path in\n >> read_gitfile_gently() used to say \"Ah, a failure, that means we\n >> cannot positively say that 'sm1/.git' is a git directory or a gitdir\n >> file.\" Now we inspect the error code in an attempt to tell if it is\n >> a system failure (e.g., a corrupt filesystem), but catching only\n >> ENOENT is probably a bit too tight. In the above scenario, asking\n >> about 'sm1/.git' when 'sm1' is a regular file will not result in\n >> ENOENT but in ENOTDIR (i.e., \"the leading 'sm1' is not a directory so\n >> it makes no sense to ask about 'sm1/.git'\").\n\nI must admit I hadn't considered this edge case at all. Thank you for \npointing it out :]\n\n>> Is it always sensible to treat ENOTDIR and ENOENT as two equivalent\n>> errors for the purpose of read_gitfile_gently()? I have no clear\n>> answer offhand myself. This is part of what we need to think about\n>> and resolve while addressing the original \"NEEDSWORK:\" comment.\n\nHummm, I believe it is safe. From the perspective of \n'read_gitfile_gently()', the sole purpose is to locate and read a \nrepository file. Whether 'stat()' returns 'ENOENT' (the file physically \ndoes not exist) or 'ENOTDIR' (a component of the path is not a \ndirectory, making it impossible for the file to exist there), the \nfunctional result is exactly the same: the '.git' file is missing.\n\nBut I must say I can't 100% guarantee its safety. Anyway, lemme just do \nwhat I can for now.\n\nI will:\n - squash your diff into my patch\n - rename the error code to `READ_GITFILE_ERR_STAT_MISSING`\n - combine this with the commit message refinements and test cleanups \nsuggested by Karthik in the previous thread.\n\nThank you for the patch and for walking me through this edge case. I'll \nsend out v11 soon! (2~3 hours later)\n\nRegards,\n\nYuchen\n"},{"id":"536689","messageId":"xmqqfr6soxjq.fsf@gitster.g","threadId":"65013","inReplyTo":"5263825f-163c-43af-bac7-152d670919d9@gmail.com","subject":"Re: [PATCH v10] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-23T05:10:01Z","receivedAt":"2026-02-23T05:10:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen writes:\n\n> I must admit I hadn't considered this edge case at all. Thank you for \n> pointing it out :]\n\nIt is easy to see, if you run the tests, though ;-)\n\n> I will:\n> - squash your diff into my patch\n> - rename the error code to `READ_GITFILE_ERR_STAT_MISSING`\n\nI think MISSING is more appropriate than STAT_MISSING. Our stat(2)\ncall positively identified that the given path does not exist on the\nfilesystem.\n\n> - combine this with the commit message refinements and test cleanups \n> suggested by Karthik in the previous thread.\n\nYeah, cleaning after yourselves in the tests is a good idea.\n"},{"id":"536693","messageId":"2008a0a2-7e68-463c-9790-498f4bf7b779@gmail.com","threadId":"65013","inReplyTo":"CAOLa=ZTePRR05M5VBxxk0OA=_RyNd0pLe=Bq6xwnE3MyZBjBAw@mail.gmail.com","subject":"Re: [PATCH v10] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-23T07:00:49Z","receivedAt":"2026-02-23T07:00:53Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi Karthik,\n\nThank you so much for the detailed review.\n\n\n> Where is the 'uninitialized variable hazard'?\n\nHummm...seems that 'die_on_error ? NULL : &error_code' would just \nimmediately 'die()' internally if NULL was passed. So there was no \n*real* hazard of uninitialized error_code being evaluated externally. \nAnyway, this change hardly qualifies as a major focus, and I will remove \nit from the commit message.\n\n> I couldn't find a discussion, why did we merge the commits?\n\nIt was suggested by Junio, ensuring that every commit in the history \nremains strictly atomic and bisectable.\n\n> Not your fault, but some of the errors quote the path and some don't, it\n> would be nice to be uniform here.\n> Nit: should we also cleanup? with a 'test_when_finished \"rm -rf\n> parent/link-to-dir\"'.\n> Should apply for all the tests.\n\nGreat catch. Will change in v11.\n\n> The rest looks good. Thanks!\n\nThank you again for the guidance ;)\n\nRegards,\n\nyuchen\n\n"},{"id":"536695","messageId":"20260223074410.917523-1-a3205153416@gmail.com","threadId":"65013","inReplyTo":"20260222102928.377519-1-a3205153416@gmail.com","subject":"[PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-23T07:44:10Z","receivedAt":"2026-02-23T07:44:17Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"'read_gitfile_gently()' treats any non-regular file as\n'READ_GITFILE_ERR_NOT_A_FILE' and fails to discern between 'ENOENT'\nand other stat failures. This flawed error reporting is noted by two\n'NEEDSWORK' comments.\n\nAddress these comments by introducing two new error codes:\n'READ_GITFILE_ERR_MISSING'(which groups the \"file missing\" scenarios\ntogether) and 'READ_GITFILE_ERR_IS_A_DIR'.\n\nTo preserve the original intent of the setup process:\n1. Update 'read_gitfile_error_die()' to treat both 'IS_A_DIR' and\n 'MISSING' as no-ops, while continuing to call 'die()' on true\n 'NOT_A_FILE' errors to prevent security hazards (like FIFOs).\n2. Unconditionally pass '&error_code' to 'read_gitfile_gently()'.\n3. Only invoke 'is_git_directory()' when we explicitly receive\n 'READ_GITFILE_ERR_IS_A_DIR', avoiding redundant filesystem checks.\n4. Correctly return 'GIT_DIR_INVALID_GITFILE' on unrecognized errors\n when 'die_on_error' is false.\n\nAdditionally, audit external callers of 'read_gitfile_gently()' in\n'submodule.c' and 'worktree.c' to accommodate the refined error codes.\n\nSigned-off-by: Tian Yuchen \n---\n setup.c | 45 ++++++++++++++------\n setup.h | 2 +\n submodule.c | 2 +-\n t/meson.build | 1 +\n t/t0009-git-dir-validation.sh | 77 +++++++++++++++++++++++++++++++++++\n worktree.c | 6 ++-\n 6 files changed, 118 insertions(+), 15 deletions(-)\n create mode 100755 t/t0009-git-dir-validation.sh\n\ndiff --git a/setup.c b/setup.c\nindex c8336eb20e..015088119c 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -897,10 +897,14 @@ int verify_repository_format(const struct repository_format *format,\n void read_gitfile_error_die(int error_code, const char *path, const char *dir)\n {\n \tswitch (error_code) {\n-\tcase READ_GITFILE_ERR_STAT_FAILED:\n-\tcase READ_GITFILE_ERR_NOT_A_FILE:\n+\tcase READ_GITFILE_ERR_MISSING:\n+\tcase READ_GITFILE_ERR_IS_A_DIR:\n \t\t/* non-fatal; follow return path */\n \t\tbreak;\n+\tcase READ_GITFILE_ERR_STAT_FAILED:\n+\t\tdie(_(\"error reading '%s'\"), path);\n+\tcase READ_GITFILE_ERR_NOT_A_FILE:\n+\t\tdie(_(\"not a regular file: '%s'\"), path);\n \tcase READ_GITFILE_ERR_OPEN_FAILED:\n \t\tdie_errno(_(\"error opening '%s'\"), path);\n \tcase READ_GITFILE_ERR_TOO_LARGE:\n@@ -941,8 +945,14 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n \tstatic struct strbuf realpath = STRBUF_INIT;\n \n \tif (stat(path, &st)) {\n-\t\t/* NEEDSWORK: discern between ENOENT vs other errors */\n-\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n+\t\tif (errno == ENOENT || errno == ENOTDIR)\n+\t\t\terror_code = READ_GITFILE_ERR_MISSING;\n+\t\telse\n+\t\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n+\t\tgoto cleanup_return;\n+\t}\n+\tif (S_ISDIR(st.st_mode)) {\n+\t\terror_code = READ_GITFILE_ERR_IS_A_DIR;\n \t\tgoto cleanup_return;\n \t}\n \tif (!S_ISREG(st.st_mode)) {\n@@ -1578,20 +1588,31 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n \t\tif (offset > min_offset)\n \t\t\tstrbuf_addch(dir, '/');\n \t\tstrbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);\n-\t\tgitdirenv = read_gitfile_gently(dir->buf, die_on_error ?\n-\t\t\t\t\t\tNULL : &error_code);\n+\t\tgitdirenv = read_gitfile_gently(dir->buf, &error_code);\n \t\tif (!gitdirenv) {\n-\t\t\tif (die_on_error ||\n-\t\t\t error_code == READ_GITFILE_ERR_NOT_A_FILE) {\n-\t\t\t\t/* NEEDSWORK: fail if .git is not file nor dir */\n+\t\t\tswitch (error_code) {\n+\t\t\tcase READ_GITFILE_ERR_MISSING:\n+\t\t\t\t/* no .git in this directory, move on */\n+\t\t\t\tbreak;\n+\t\t\tcase READ_GITFILE_ERR_IS_A_DIR:\n \t\t\t\tif (is_git_directory(dir->buf)) {\n \t\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n \t\t\t\t\tgitdir_path = xstrdup(dir->buf);\n \t\t\t\t}\n-\t\t\t} else if (error_code != READ_GITFILE_ERR_STAT_FAILED)\n-\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n-\t\t} else\n+\t\t\t\t/* \n+\t\t\t\t * NEEDSWORK: should we catch a directory .git \n+\t\t\t\t * that is not a git directory here? \n+\t\t\t\t */\n+\t\t\t\tbreak;\n+\t\t\tdefault:\n+\t\t\t\tif (die_on_error || error_code == READ_GITFILE_ERR_NOT_A_FILE)\n+\t\t\t\t\tread_gitfile_error_die(error_code, dir->buf, NULL);\n+\t\t\t\telse\n+\t\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n+\t\t\t}\n+\t\t} else {\n \t\t\tgitfile = xstrdup(dir->buf);\n+\t\t}\n \t\t/*\n \t\t * Earlier, we tentatively added DEFAULT_GIT_DIR_ENVIRONMENT\n \t\t * to check that directory for a repository.\ndiff --git a/setup.h b/setup.h\nindex 0738dec244..76fb260c20 100644\n--- a/setup.h\n+++ b/setup.h\n@@ -36,6 +36,8 @@ int is_nonbare_repository_dir(struct strbuf *path);\n #define READ_GITFILE_ERR_NO_PATH 6\n #define READ_GITFILE_ERR_NOT_A_REPO 7\n #define READ_GITFILE_ERR_TOO_LARGE 8\n+#define READ_GITFILE_ERR_MISSING 9\n+#define READ_GITFILE_ERR_IS_A_DIR 10\n void read_gitfile_error_die(int error_code, const char *path, const char *dir);\n const char *read_gitfile_gently(const char *path, int *return_error_code);\n #define read_gitfile(path) read_gitfile_gently((path), NULL)\ndiff --git a/submodule.c b/submodule.c\nindex 508938e4da..767d4c3c35 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2559,7 +2559,7 @@ void absorb_git_dir_into_superproject(const char *path,\n \t\tconst struct submodule *sub;\n \t\tstruct strbuf sub_gitdir = STRBUF_INIT;\n \n-\t\tif (err_code == READ_GITFILE_ERR_STAT_FAILED) {\n+\t\tif (err_code == READ_GITFILE_ERR_MISSING) {\n \t\t\t/* unpopulated as expected */\n \t\t\tstrbuf_release(&gitdir);\n \t\t\treturn;\ndiff --git a/t/meson.build b/t/meson.build\nindex f80e366cff..c4afaacee5 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -80,6 +80,7 @@ integration_tests = [\n 't0006-date.sh',\n 't0007-git-var.sh',\n 't0008-ignores.sh',\n+ 't0009-git-dir-validation.sh',\n 't0010-racy-git.sh',\n 't0012-help.sh',\n 't0013-sha1dc.sh',\ndiff --git a/t/t0009-git-dir-validation.sh b/t/t0009-git-dir-validation.sh\nnew file mode 100755\nindex 0000000000..7e2c711a63\n--- /dev/null\n+++ b/t/t0009-git-dir-validation.sh\n@@ -0,0 +1,77 @@\n+#!/bin/sh\n+\n+test_description='setup: validation of .git file/directory types\n+\n+Verify that setup_git_directory() correctly handles:\n+1. Valid .git directories (including symlinks to them).\n+2. Invalid .git files (FIFOs, sockets) by erroring out.\n+3. Invalid .git files (garbage) by erroring out.\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup: create parent git repository' '\n+\tgit init parent &&\n+\ttest_commit -C parent \"root-commit\"\n+'\n+\n+test_expect_success SYMLINKS 'setup: .git as a symlink to a directory is valid' '\n+\ttest_when_finished \"rm -rf parent/link-to-dir\" &&\n+\tmkdir -p parent/link-to-dir &&\n+\t(\n+\t\tcd parent/link-to-dir &&\n+\t\tgit init real-repo &&\n+\t\tln -s real-repo/.git .git &&\n+\t\tgit rev-parse --git-dir >actual &&\n+\t\techo .git >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success PIPE 'setup: .git as a FIFO (named pipe) is rejected' '\n+\ttest_when_finished \"rm -rf parent/fifo-trap\" &&\n+\tmkdir -p parent/fifo-trap &&\n+\t(\n+\t\tcd parent/fifo-trap &&\n+\t\tmkfifo .git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"not a regular file\" stderr\n+\t)\n+'\n+\n+test_expect_success SYMLINKS,PIPE 'setup: .git as a symlink to a FIFO is rejected' '\n+\ttest_when_finished \"rm -rf parent/symlink-fifo-trap\" &&\n+\tmkdir -p parent/symlink-fifo-trap &&\n+\t(\n+\t\tcd parent/symlink-fifo-trap &&\n+\t\tmkfifo target-fifo &&\n+\t\tln -s target-fifo .git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"not a regular file\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: .git with garbage content is rejected' '\n+\ttest_when_finished \"rm -rf parent/garbage-trap\" &&\n+\tmkdir -p parent/garbage-trap &&\n+\t(\n+\t\tcd parent/garbage-trap &&\n+\t\techo \"garbage\" >.git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"invalid gitfile format\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: .git as an empty directory is ignored' '\n+\ttest_when_finished \"rm -rf parent/empty-dir\" &&\n+\tmkdir -p parent/empty-dir &&\n+\t(\n+\t\tcd parent/empty-dir &&\n+\t\tmkdir .git &&\n+\t\tgit rev-parse --git-dir >actual &&\n+\t\techo \"$TRASH_DIRECTORY/parent/.git\" >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_done\ndiff --git a/worktree.c b/worktree.c\nindex 9308389cb6..d1165e1d1c 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -653,7 +653,8 @@ static void repair_gitfile(struct worktree *wt,\n \t\t}\n \t}\n \n-\tif (err == READ_GITFILE_ERR_NOT_A_FILE)\n+\tif (err == READ_GITFILE_ERR_NOT_A_FILE ||\n+\t\terr == READ_GITFILE_ERR_IS_A_DIR)\n \t\tfn(1, wt->path, _(\".git is not a file\"), cb_data);\n \telse if (err)\n \t\trepair = _(\".git file broken\");\n@@ -833,7 +834,8 @@ void repair_worktree_at_path(const char *path,\n \t\t\tstrbuf_addstr(&backlink, dotgit_contents);\n \t\t\tstrbuf_realpath_forgiving(&backlink, backlink.buf, 0);\n \t\t}\n-\t} else if (err == READ_GITFILE_ERR_NOT_A_FILE) {\n+\t} else if (err == READ_GITFILE_ERR_NOT_A_FILE ||\n+\t\t\terr == READ_GITFILE_ERR_IS_A_DIR) {\n \t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git is not a file\"), cb_data);\n \t\tgoto done;\n \t} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {\n-- \n2.43.0\n\n"},{"id":"536813","messageId":"xmqq7bs3piz7.fsf@gitster.g","threadId":"65013","inReplyTo":"xmqqfr6soxjq.fsf@gitster.g","subject":"Re: [PATCH v10] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-23T15:39:24Z","receivedAt":"2026-02-23T15:39:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> I think MISSING is more appropriate than STAT_MISSING. Our stat(2)\n> call positively identified that the given path does not exist on the\n> filesystem.\n\nIf you prefer to have STAT_ there, I do not mind too much. But at\nsome point, we may want to drop _ERR in those two new \"these are not\nerrors\" return values.\n"},{"id":"536856","messageId":"a2b2e581-18ba-42ad-9bf1-a3e16b85f4e9@gmail.com","threadId":"65013","inReplyTo":"xmqq7bs3piz7.fsf@gitster.g","subject":"Re: [PATCH v10] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-23T17:17:52Z","receivedAt":"2026-02-23T17:17:56Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi Junio,\n\n > But at some point, we may want to drop _ERR in those two new \"these \nare not\n > errors\" return values.\n\nFor the v11 I sent earlier, I kept `READ_GITFILE_ERR_MISSING` to \nmaintain namespace consistency with the existing `READ_GITFILE_ERR_*` \nmacros. However, I can't deny that decoupling these into neutral status \ncodes (e.g., `READ_GITFILE_MISSING` vs actual fatal errors) also makes \nsense.\n\nI'd be more than happy to drop the `_ERR` prefix in a v12 if you think \nit's better to address this cleanup right now, or we can definitely \nleave it for a future cleanup patch as you suggested.\n\nThanks,\n\nTian Yuchen\n"},{"id":"536870","messageId":"xmqqwm03mfax.fsf@gitster.g","threadId":"65013","inReplyTo":"a2b2e581-18ba-42ad-9bf1-a3e16b85f4e9@gmail.com","subject":"Re: [PATCH v10] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-23T19:27:02Z","receivedAt":"2026-02-23T19:27:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen writes:\n\n> Hi Junio,\n>\n> > But at some point, we may want to drop _ERR in those two new \"these \n> are not\n> > errors\" return values.\n>\n> For the v11 I sent earlier, I kept `READ_GITFILE_ERR_MISSING` to \n> maintain namespace consistency with the existing `READ_GITFILE_ERR_*` \n> macros. However, I can't deny that decoupling these into neutral status \n> codes (e.g., `READ_GITFILE_MISSING` vs actual fatal errors) also makes \n> sense.\n>\n> I'd be more than happy to drop the `_ERR` prefix in a v12 if you think \n> it's better to address this cleanup right now, or we can definitely \n> leave it for a future cleanup patch as you suggested.\n\nNah, we are quickly approaching the point of diminishing return.\nLet's see if we need to kill more fundamental bugs before\nbikeshedding the constant names.\n\nThanks.\n"},{"id":"536961","messageId":"c0b31daf-0997-46a0-95d3-e1f608b23888@gmail.com","threadId":"65013","inReplyTo":"xmqqwm03mfax.fsf@gitster.g","subject":"Re: [PATCH v10] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-24T10:23:42Z","receivedAt":"2026-02-24T10:23:45Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi Junio,\n\nUnderstood.\n\nI'll leave the patch series as it is. Please reply to me any time\nwhen new bugs are observed.\n\nRegards,\n\nYuchen\n\n"},{"id":"536981","messageId":"e48c68ce-de45-4d45-8bd2-1307686a8910@gmail.com","threadId":"65013","inReplyTo":"xmqqwm03mfax.fsf@gitster.g","subject":"Re: [PATCH v10] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-24T17:01:36Z","receivedAt":"2026-02-24T17:01:39Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi Junio,\n\nI believe we have made some progress so far. However, after reviewing \nthe code again just now, I still have a few questions I'd like to ask:\n\n 1. Perhaps I took it literally, but as far as the original intent \ngoes, shouldn't 'read_gitfile_gently()' be solely responsible for \n*opening a .git file and parsing the gitdir: format*? However, it \ncurrently executes 'stat()', checks 'S_ISDIR', checks 'S_ISREG', handles \nmissing components, and *then* attempts to parse. Do we need an 'enum \ngit_componet_type git_componet(cost char *path)' which returns pure \nfilesystem states, then parse it with 'parse_gitfile_format(const char \n*path)'? I don't know.\n\n 2. Let's say, when stat() encounters a EACCES when cheaking a \nrestricted sub-folder. Git funnels this into STAT_FAILED and \nsubsequently invokes die(), which calls exit(). I'm thinking of \nlibification: if a long running multi threaded git server encounters a \npermission-denied directory, is killing the entire process the expected \nbehavior? Should 'permission-denied' really be considered as an 'error'? \nDoes a library has the authority to terminate the application?\n\nI won't touch any of this right now to keep the current scope focused, \nbut I plan to incorporate thoughts into my GSoC proposal for the global \nstate/libification project.\n\nIf you have more important things to do, feel free to ignore this email. \nAfter all, I consider it a minor issue.\n\nThis is my first patch at my nineteen, and I'm more than delighted to \nspend my birthday reviewing git code ;)\n\nRegards,\n\nYuchen\n\n"},{"id":"537066","messageId":"xmqq8qchcz9w.fsf@gitster.g","threadId":"65013","inReplyTo":"e48c68ce-de45-4d45-8bd2-1307686a8910@gmail.com","subject":"Re: [PATCH v10] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-25T02:50:19Z","receivedAt":"2026-02-25T02:50:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen writes:\n\n> 1. Perhaps I took it literally, but as far as the original intent \n> goes, shouldn't 'read_gitfile_gently()' be solely responsible for \n> *opening a .git file and parsing the gitdir: format*? However, it \n> currently executes 'stat()', checks 'S_ISDIR', checks 'S_ISREG', handles \n> missing components, and *then* attempts to parse. Do we need an 'enum \n> git_componet_type git_componet(cost char *path)' which returns pure \n> filesystem states, then parse it with 'parse_gitfile_format(const char \n> *path)'? I don't know.\n\nSorry, but I do not see what such a change buys us.\n\n> 2. Let's say, when stat() encounters a EACCES when cheaking a \n> restricted sub-folder. Git funnels this into STAT_FAILED and \n> subsequently invokes die(), which calls exit().\n\nYes all this happens in repository set-up which should happen very\nearly in the process, no?\n\n\n> I'm thinking of \n> libification: if a long running multi threaded git server encounters a \n> permission-denied directory, is killing the entire process the expected \n> behavior?\n\nIt is a bug in the way you wrote that multi-threaded git server, no?\n\nWe have the \"_gently\" variant for such a use case, and I do not\nthink we expect the normal single-process git start-up sequence\nshould be reused there.\n"},{"id":"537096","messageId":"28bf5b66-8c08-43b1-b472-acb97c8f0eb7@gmail.com","threadId":"65013","inReplyTo":"xmqq8qchcz9w.fsf@gitster.g","subject":"Re: [PATCH v10] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-25T16:03:18Z","receivedAt":"2026-02-25T16:03:24Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi Junio,\n\n> Sorry, but I do not see what such a change buys us.\n\nMy primary concern was the function's semantics, though it is indeed \nredundant.\n\n> Yes all this happens in repository set-up which should happen very\n> early in the process, no?\n\nShould be like this.\n\n> It is a bug in the way you wrote that multi-threaded git server, no?\n> \n> We have the \"_gently\" variant for such a use case, and I do not\n> think we expect the normal single-process git start-up sequence\n> should be reused there.\n\nI see. Thank you for the insights. I will drop these overly abstract \nideas and keep my focus on practical fixes.\n\nRegards,\n\nYuchen\n"},{"id":"537254","messageId":"xmqqpl5rumy0.fsf@gitster.g","threadId":"65013","inReplyTo":"20260223074410.917523-1-a3205153416@gmail.com","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-26T23:03:51Z","receivedAt":"2026-02-26T23:03:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen writes:\n\n> 'read_gitfile_gently()' treats any non-regular file as\n> 'READ_GITFILE_ERR_NOT_A_FILE' and fails to discern between 'ENOENT'\n> and other stat failures. This flawed error reporting is noted by two\n> 'NEEDSWORK' comments.\n>\n> Address these comments by introducing two new error codes:\n> 'READ_GITFILE_ERR_MISSING'(which groups the \"file missing\" scenarios\n> together) and 'READ_GITFILE_ERR_IS_A_DIR'.\n>\n> To preserve the original intent of the setup process:\n> 1. Update 'read_gitfile_error_die()' to treat both 'IS_A_DIR' and\n> 'MISSING' as no-ops, while continuing to call 'die()' on true\n> 'NOT_A_FILE' errors to prevent security hazards (like FIFOs).\n> 2. Unconditionally pass '&error_code' to 'read_gitfile_gently()'.\n> 3. Only invoke 'is_git_directory()' when we explicitly receive\n> 'READ_GITFILE_ERR_IS_A_DIR', avoiding redundant filesystem checks.\n> 4. Correctly return 'GIT_DIR_INVALID_GITFILE' on unrecognized errors\n> when 'die_on_error' is false.\n>\n> Additionally, audit external callers of 'read_gitfile_gently()' in\n> 'submodule.c' and 'worktree.c' to accommodate the refined error codes.\n>\n> Signed-off-by: Tian Yuchen \n> ---\n> setup.c | 45 ++++++++++++++------\n> setup.h | 2 +\n> submodule.c | 2 +-\n> t/meson.build | 1 +\n> t/t0009-git-dir-validation.sh | 77 +++++++++++++++++++++++++++++++++++\n> worktree.c | 6 ++-\n> 6 files changed, 118 insertions(+), 15 deletions(-)\n> create mode 100755 t/t0009-git-dir-validation.sh\n\nUnfortunately this seems to break almost all the tests, not just the\ntest the patch adds, on Windows (which I almost know nothing about,\nbut I can observe that CI jobs die).\n\nhttps://github.com/git/git/actions/runs/22464017037 is a CI run that\nmerged this patch on top of the commit that corresponds to the tip\nof 'next' as of today. We can see \"win test (N)\" jobs dying all\nover. I cancelled the workflow before seeing everything die,\nthough.\n\nhttps://github.com/git/git/actions/runs/22464479533 is a CI run that\ntests this patch applied directly on v2.53.0 in isolation.\n\nAs I said, I do not know Windows well, so this may be a red-herring,\nbut in this CI run, we see \"GIT_DIR=/dev/null git diff --no-index ...\"\nresults in \"fatal: error reading 'nul'\":\n\n https://github.com/git/git/actions/runs/22464479533/job/65067515458#step:5:95419\n\nwhich is an expected thing to happen, but we probably used to ignore\nit as a non-error?\n\nFor now, I'll kick this topic out of my tree to give other topics a\nbit more test exposure so that we can notice new bugs in them (not\nin this topic) that causes the tests fail. With this topic in 'seen',\nsuch bugs in other topics are all masked.\n\nCan folks equipped with knowledge and environment to debug Windows\nonly breakage lend a hand to figure out what this patch gets wrong\nto help it move forward?\n\n\nThanks.\n\n\n\n\n"},{"id":"537269","messageId":"bcf64540-fe84-4fcc-a969-6927f348608e@gmail.com","threadId":"65013","inReplyTo":"xmqqpl5rumy0.fsf@gitster.g","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-27T05:26:51Z","receivedAt":"2026-02-27T05:26:56Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi Junio,\n\nThank you for the error reporting.\n\nIt seems that failure \"fatal: error reading 'nul'\" matches the \n'die(_(\"error reading %s\"), path)', if my understanding is correct?\n\nSo during 'git diff --no-index', the test passes 'GIT_DIR=/dev/null'. I \nhighly suspect that 'stat(\"nul\")' on Windows fails with an 'errno' other \nthan 'ENOENT', so it falls into the 'STAT_FAILED' branch...\n\n...which can be simply fixed by reverting 'READ_GITFILE_ERR_STAT_FAILED' \n(and probably 'READ_GITFILE_ERR_NOT_A_FILE') back to being non-fatal \ninside 'read_gitfile_error_die()', if I'm correct? In that case, the \nlogic of the test script should also be changed, shouldn't it?\n\nHowever, I'm sure that this change runs counter to our previous \ndiscussions. I can't think of any good ideas since I'm no windows expert \neither. So you can pretend I never said anything :P\n\nRegards,\n\nYuchen\n"},{"id":"537345","messageId":"xmqq4in1q152.fsf@gitster.g","threadId":"65013","inReplyTo":"bcf64540-fe84-4fcc-a969-6927f348608e@gmail.com","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-02-27T22:20:41Z","receivedAt":"2026-02-27T22:20:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen writes:\n\n> It seems that failure \"fatal: error reading 'nul'\" matches the \n> 'die(_(\"error reading %s\"), path)', if my understanding is correct?\n\nThe message is likely to be with its own pair of single quotes,\ni.e., \"error reading '%s'\".\n\n> So during 'git diff --no-index', the test passes 'GIT_DIR=/dev/null'. I \n> highly suspect that 'stat(\"nul\")' on Windows fails with an 'errno' other \n> than 'ENOENT', so it falls into the 'STAT_FAILED' branch...\n\nIn the expected case, how would\n\n $ GIT_DIR=/dev/null git diff --no-index . .\n\nstart up? setup_git_directory_gently() is called with a non-NULL\n&nongit in turn it calls setup_git_directory_gently_1() that gives\nus GIT_DIR_EXPLICIT, which makes us call setup_explicit_git_dir().\n\nIt would make us call is_git_diretory() on \"/dev/null\", and a\nnon-NULL nongit_ok pointer causes us to return NULL from there.\n\nI am not sure how the result of calling stat() gets in the picture.\n\n> ...which can be simply fixed by reverting 'READ_GITFILE_ERR_STAT_FAILED' \n> (and probably 'READ_GITFILE_ERR_NOT_A_FILE') back to being non-fatal \n> inside 'read_gitfile_error_die()', if I'm correct? In that case, the \n> logic of the test script should also be changed, shouldn't it?\n>\n> However, I'm sure that this change runs counter to our previous \n> discussions. I can't think of any good ideas since I'm no windows expert \n> either. So you can pretend I never said anything :P\n\nIt sounds like saying \"oh, this is too hard, let's punt and roll all\nfailures from stat() into 'nothing there, move on'\", which is how\nthe code used to work before this patch X-<.\n\n\n"},{"id":"537374","messageId":"e6e7e272-4aec-461e-aebd-33ec0a324770@gmail.com","threadId":"65013","inReplyTo":"xmqq4in1q152.fsf@gitster.g","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-02-28T04:38:54Z","receivedAt":"2026-02-28T04:38:58Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi Junio,\n\nThanks for pushing back on my previous suggestion.\n\n> It sounds like saying \"oh, this is too hard, let's punt and roll all\n> failures from stat() into 'nothing there, move on'\", which is how\n> the code used to work before this patch X-<.\n\nYeah, that's why I said this idea was pretty dumb. XD\n\nRegarding your question about how 'stat()' gets into the picture, I \ntried GDB debugging and simulated Windows behavior by 'GIT_DIR=nul' (I'm \nNOT sure if it's correct). Here is the back trace caught:\n\n(gdb) print path\n$1 = 0x555555a86dd0 \"nul\"\n(gdb) bt\n#0 read_gitfile_gently (path=0x555555a86dd0 \"nul\", return_error_code=0x0)\n at setup.c:936\n#1 0x000055555589dea1 in setup_explicit_git_dir (\n gitdirenv=0x555555a86dd0 \"nul\", cwd=0x555555a58070 ,\n repo_fmt=0x7fffffffcbe0, nongit_ok=0x7fffffffccc4) at setup.c:1107\n#2 0x000055555589fd53 in setup_git_directory_gently \n(nongit_ok=0x7fffffffccc4)\n at setup.c:1867\n#3 0x00005555555c1383 in cmd_diff (argc=4, argv=0x555555a86ce0, \nprefix=0x0,\n repo=0x0) at builtin/diff.c:458\n#4 0x0000555555575e30 in run_builtin (p=0x555555a48368 ,\n argc=4, argv=0x555555a86ce0, repo=0x555555a7bdc0 ) at \ngit.c:506\n#5 0x0000555555576354 in handle_builtin (args=0x7fffffffdc10) at git.c:780\n#6 0x000055555557667e in run_argv (args=0x7fffffffdc10) at git.c:863\n#7 0x0000555555576b44 in cmd_main (argc=4, argv=0x7fffffffdda0) at \ngit.c:985\n#8 0x00005555556a73bf in main (argc=5, argv=0x7fffffffdd98) at \ncommon-main.c:9\n\nSorry it might be a bit messy. It turns out that \n'setup_explicit_git_dir()' calls 'read_gitfile_gently()' right away to \ncheck whether GIT_DIR is actually a gitfile, right?\n\nSince stat(\"nul\") fails on windows, STAT_FAILED was caught and \nimmediately triggered die(), blowing up corresponding CI tests.\n\nStill, I don't know Windows well. These thoughts are just to stimulate \ndiscussions. （　´_ゝ`）\n\nThanks,\n\nYuchen\n\n\n"},{"id":"537546","messageId":"xmqqjyvu42pw.fsf@gitster.g","threadId":"65013","inReplyTo":"xmqqpl5rumy0.fsf@gitster.g","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-02T16:26:35Z","receivedAt":"2026-03-02T16:26:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Let's try again.\n\nCan folks equipped with knowledge and environment to debug breakages\nthat hapepns only on Windows lend a hand to figure out what this\npatch gets wrong to help the topic move forward?\n\nThanks.\n\nJunio C Hamano writes:\n\n> Tian Yuchen writes:\n>\n>> 'read_gitfile_gently()' treats any non-regular file as\n>> 'READ_GITFILE_ERR_NOT_A_FILE' and fails to discern between 'ENOENT'\n>> and other stat failures. This flawed error reporting is noted by two\n>> 'NEEDSWORK' comments.\n>>\n>> Address these comments by introducing two new error codes:\n>> 'READ_GITFILE_ERR_MISSING'(which groups the \"file missing\" scenarios\n>> together) and 'READ_GITFILE_ERR_IS_A_DIR'.\n>>\n>> To preserve the original intent of the setup process:\n>> 1. Update 'read_gitfile_error_die()' to treat both 'IS_A_DIR' and\n>> 'MISSING' as no-ops, while continuing to call 'die()' on true\n>> 'NOT_A_FILE' errors to prevent security hazards (like FIFOs).\n>> 2. Unconditionally pass '&error_code' to 'read_gitfile_gently()'.\n>> 3. Only invoke 'is_git_directory()' when we explicitly receive\n>> 'READ_GITFILE_ERR_IS_A_DIR', avoiding redundant filesystem checks.\n>> 4. Correctly return 'GIT_DIR_INVALID_GITFILE' on unrecognized errors\n>> when 'die_on_error' is false.\n>>\n>> Additionally, audit external callers of 'read_gitfile_gently()' in\n>> 'submodule.c' and 'worktree.c' to accommodate the refined error codes.\n>>\n>> Signed-off-by: Tian Yuchen \n>> ---\n>> setup.c | 45 ++++++++++++++------\n>> setup.h | 2 +\n>> submodule.c | 2 +-\n>> t/meson.build | 1 +\n>> t/t0009-git-dir-validation.sh | 77 +++++++++++++++++++++++++++++++++++\n>> worktree.c | 6 ++-\n>> 6 files changed, 118 insertions(+), 15 deletions(-)\n>> create mode 100755 t/t0009-git-dir-validation.sh\n>\n> Unfortunately this seems to break almost all the tests, not just the\n> test the patch adds, on Windows (which I almost know nothing about,\n> but I can observe that CI jobs die).\n>\n> https://github.com/git/git/actions/runs/22464017037 is a CI run that\n> merged this patch on top of the commit that corresponds to the tip\n> of 'next' as of today. We can see \"win test (N)\" jobs dying all\n> over. I cancelled the workflow before seeing everything die,\n> though.\n>\n> https://github.com/git/git/actions/runs/22464479533 is a CI run that\n> tests this patch applied directly on v2.53.0 in isolation.\n>\n> As I said, I do not know Windows well, so this may be a red-herring,\n> but in this CI run, we see \"GIT_DIR=/dev/null git diff --no-index ...\"\n> results in \"fatal: error reading 'nul'\":\n>\n> https://github.com/git/git/actions/runs/22464479533/job/65067515458#step:5:95419\n>\n> which is an expected thing to happen, but we probably used to ignore\n> it as a non-error?\n>\n> For now, I'll kick this topic out of my tree to give other topics a\n> bit more test exposure so that we can notice new bugs in them (not\n> in this topic) that causes the tests fail. With this topic in 'seen',\n> such bugs in other topics are all masked.\n>\n>\n>\n> Thanks.\n"},{"id":"537723","messageId":"460f00d5-97b4-4a6c-be45-6f60a17cd33e@gmail.com","threadId":"65013","inReplyTo":"xmqqjyvu42pw.fsf@gitster.g","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-03T19:31:04Z","receivedAt":"2026-03-03T19:31:11Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 02/03/2026 16:26, Junio C Hamano wrote:\n> Let's try again.\n> \n> Can folks equipped with knowledge and environment to debug breakages\n> that hapepns only on Windows lend a hand to figure out what this\n> patch gets wrong to help the topic move forward?\n\nLooking at the test failures the tests are failing because\n\n GIT_DIR=/dev/null git diff --no-index ...\n\nwhich is used by test_cmp() on Windows is dying. That happens because \nstat(\"nul\", &st) fails and this series makes that an error (somewhere \nalong the line \"/dev/null\" is rewritten to \"nul\" on Windows). I'm afraid \nI don't know enough about Windows to be sure how to fix it but maybe we \nshould special case \"nul\" in the setup code or mingw_stat(). We should \nalso check what happens when GIT_DIR=/dev/null on linux and other POSIX \nplatforms.\n\nThanks\n\nPhillip\n\n\n> Thanks.\n> \n> Junio C Hamano writes:\n> \n>> Tian Yuchen writes:\n>>\n>>> 'read_gitfile_gently()' treats any non-regular file as\n>>> 'READ_GITFILE_ERR_NOT_A_FILE' and fails to discern between 'ENOENT'\n>>> and other stat failures. This flawed error reporting is noted by two\n>>> 'NEEDSWORK' comments.\n>>>\n>>> Address these comments by introducing two new error codes:\n>>> 'READ_GITFILE_ERR_MISSING'(which groups the \"file missing\" scenarios\n>>> together) and 'READ_GITFILE_ERR_IS_A_DIR'.\n>>>\n>>> To preserve the original intent of the setup process:\n>>> 1. Update 'read_gitfile_error_die()' to treat both 'IS_A_DIR' and\n>>> 'MISSING' as no-ops, while continuing to call 'die()' on true\n>>> 'NOT_A_FILE' errors to prevent security hazards (like FIFOs).\n>>> 2. Unconditionally pass '&error_code' to 'read_gitfile_gently()'.\n>>> 3. Only invoke 'is_git_directory()' when we explicitly receive\n>>> 'READ_GITFILE_ERR_IS_A_DIR', avoiding redundant filesystem checks.\n>>> 4. Correctly return 'GIT_DIR_INVALID_GITFILE' on unrecognized errors\n>>> when 'die_on_error' is false.\n>>>\n>>> Additionally, audit external callers of 'read_gitfile_gently()' in\n>>> 'submodule.c' and 'worktree.c' to accommodate the refined error codes.\n>>>\n>>> Signed-off-by: Tian Yuchen \n>>> ---\n>>> setup.c | 45 ++++++++++++++------\n>>> setup.h | 2 +\n>>> submodule.c | 2 +-\n>>> t/meson.build | 1 +\n>>> t/t0009-git-dir-validation.sh | 77 +++++++++++++++++++++++++++++++++++\n>>> worktree.c | 6 ++-\n>>> 6 files changed, 118 insertions(+), 15 deletions(-)\n>>> create mode 100755 t/t0009-git-dir-validation.sh\n>>\n>> Unfortunately this seems to break almost all the tests, not just the\n>> test the patch adds, on Windows (which I almost know nothing about,\n>> but I can observe that CI jobs die).\n>>\n>> https://github.com/git/git/actions/runs/22464017037 is a CI run that\n>> merged this patch on top of the commit that corresponds to the tip\n>> of 'next' as of today. We can see \"win test (N)\" jobs dying all\n>> over. I cancelled the workflow before seeing everything die,\n>> though.\n>>\n>> https://github.com/git/git/actions/runs/22464479533 is a CI run that\n>> tests this patch applied directly on v2.53.0 in isolation.\n>>\n>> As I said, I do not know Windows well, so this may be a red-herring,\n>> but in this CI run, we see \"GIT_DIR=/dev/null git diff --no-index ...\"\n>> results in \"fatal: error reading 'nul'\":\n>>\n>> https://github.com/git/git/actions/runs/22464479533/job/65067515458#step:5:95419\n>>\n>> which is an expected thing to happen, but we probably used to ignore\n>> it as a non-error?\n>>\n>> For now, I'll kick this topic out of my tree to give other topics a\n>> bit more test exposure so that we can notice new bugs in them (not\n>> in this topic) that causes the tests fail. With this topic in 'seen',\n>> such bugs in other topics are all masked.\n>>\n>>\n>>\n>> Thanks.\n> \n\n"},{"id":"537749","messageId":"xmqqo6l49mrt.fsf@gitster.g","threadId":"65013","inReplyTo":"460f00d5-97b4-4a6c-be45-6f60a17cd33e@gmail.com","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-04T05:39:02Z","receivedAt":"2026-03-04T05:39:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> Looking at the test failures the tests are failing because\n>\n> GIT_DIR=/dev/null git diff --no-index ...\n>\n> which is used by test_cmp() on Windows is dying. That happens because \n> stat(\"nul\", &st) fails and this series makes that an error (somewhere \n> along the line \"/dev/null\" is rewritten to \"nul\" on Windows). I'm afraid \n> I don't know enough about Windows to be sure how to fix it but maybe we \n> should special case \"nul\" in the setup code or mingw_stat().\n\nWhile I do not think we want to special case \"nul\", I think we need\nto make the tightening of error checking conditional to who is\nasking to know. The _intent_ behind the NEEDSWORK comment was to\nallow us to be more strict when we see a fishy \".git\" filesystem\nentity when we are in a directory /a/b/c/d and are trying to find\nout if we are in a Git working tree and where our .git directory is.\nWe may not see /a/b/c/d/.git, go up one level and find /a/b/c/.git\nand stat(2) it. In the current code, we take any and all stat(2)\nfailures as if it failed because ENOENT i.e., as if /a/b/c/.git did\nnot exist, and we go upwards. The NEEDSWORK comment wonders if we\nwant to be noticing that /a/b/c/.git did exist but we failed to\nstat(2) for some other reason, and if we would want to let the user\nknow.\n\nBut the \"GIT_DIR=/dev/null git ...\" use case is vastly different.\nWe are not doing a discovery and we are not interested in going\nupwards when the thing we check for \"git-dir-ness\" fails to be a\ngit-dir. It may have worked around the \"nul cannot be stat'ed\"\nlimitation if we used \"GIT_DIR=no-such-directory git ...\", but I\nthink anything that we positively know is not a .git directory or a\n\"gitdir: over-there\" file is given as GIT_DIR, we would want to say\n\"nope, we do not have .git dir and have to work outside any\nrepository\", instead of dying with \"whoa, what is that garbage you\nare giving me as GIT_DIR???\".\n\nWIth an explicitly given GIT_DIR, setup_explicit_git_dir() is called\nand the function calls read_gitfile_gently(path, NULL), that lets it\ndie when the thing turns out not to be a proper \".git\", except when\nstat(2) failed (for any reason) or stat(2) tells that the thing is\nnot a file. If we use GIT_DIR=/dev/null on POSIX systems, this\n\"not-a-file is fine\" leniency allows us to proceed without dying,\nsaying \"We were given an invalid GIT_DIR, we are not doing\ndiscovery, hence we are operating without a repository\". On systems\nwhere stat(2) fails for \"/dev/null\" or its equivalent \"NUL:\", the\nsame \"stat failed for any reason\" leniency allows us to do the same.\n\nBut with the patches we have been looking at, that leniency is\ntightened. I think it is a good thing to do during the repository\ndiscovery. But it is not if we are checking the explicitly given\nGIT_DIR. All calls to read_gitfile_gently(path, NULL) need to be\naudited and then we need to decide which ones to leave lenient, and\nwhich ones are OK to tighten together with the call used during the\nrepository discovery.\n\nThanks.\n\n"},{"id":"537769","messageId":"99c6a437-3fc3-4d9a-9465-4c47a9777776@gmail.com","threadId":"65013","inReplyTo":"xmqqo6l49mrt.fsf@gitster.g","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-03-04T11:03:05Z","receivedAt":"2026-03-04T11:03:09Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi Junio and Phillip,\n\nThanks for the detailed reply!\n\nAfter reading through your discussion, I believe the most crucial point is:\n\n> \"We were given an invalid GIT_DIR, we are not doing\n> discovery, hence we are operating without a repository\"\n\nIf I understand correctly, the expected behavior should be: when a user \nexplicitly passes 'GIT_DIR=/dev/null git diff', Git should no longer \nneed to \"search\" or \"guess\" anything. Instead, if it's a trash file (or \nsomething similar) rather a repository, Git should simply act as if no \nrepository exists. Is that correct?\n\nSo what I'm doing next is:\n\n> All calls to read_gitfile_gently(path, NULL) need to be\n> audited and then we need to decide which ones to leave lenient, and\n> which ones are OK to tighten together with the call used during the\n> repository discovery.\n\nWill be working on it in the next few days.\n\nRegards,\n\nYuchen\n"},{"id":"537780","messageId":"20260304141526.37764-1-a3205153416@gmail.com","threadId":"65013","inReplyTo":"20260223074410.917523-1-a3205153416@gmail.com","subject":"[PATCH v12] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-03-04T14:15:26Z","receivedAt":"2026-03-04T14:15:39Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"'read_gitfile_gently()' treats any non-regular file as\n'READ_GITFILE_ERR_NOT_A_FILE' and fails to discern between 'ENOENT'\nand other stat failures. This flawed error reporting is noted by two\n'NEEDSWORK' comments.\n\nAddress these comments by introducing two new error codes:\n'READ_GITFILE_ERR_MISSING'(which groups the \"file missing\" scenarios\ntogether) and 'READ_GITFILE_ERR_IS_A_DIR':\n\n1. Update 'read_gitfile_error_die()' to treat 'IS_A_DIR', 'MISSING',\n'NOT_A_FILE' and 'STAT_FAILED' as non-fatal no-ops. This accommodates\nintentional non-repo scenarios (e.g., GIT_DIR=/dev/null).\n\n2. Explicitly catch 'NOT_A_FILE' and 'STAT_FAILED' during\ndiscovery and call 'die()' if 'die_on_error' is set.\n\n3. Unconditionally pass '&error_code' to 'read_gitfile_gently()'.\n\n4. Only invoke 'is_git_directory()' when we explicitly receive\n 'READ_GITFILE_ERR_IS_A_DIR', avoiding redundant checks.\n\nAdditionally, audit external callers of 'read_gitfile_gently()' in\n'submodule.c' and 'worktree.c' to accommodate the refined error codes.\n\nSigned-off-by: Tian Yuchen \n---\nTo be honest, I've really gotten myself all tangled up.\nSkill issue :(\nFeel free to point out all the stupid mistakes I made.\n\nI'm very uncertain about whether my changes in \nsetup_git_directory_gently_1() are appropriate.\nBut least all CI tests passed.\n\nBy the way, the replies in my email inbox look particularly messy.\nWhen sending a new patch, which email should I reply to? Should I\nreply to the previous patch, or, start a new thread?\n\n setup.c | 47 ++++++++++++++++-----\n setup.h | 2 +\n submodule.c | 2 +-\n t/meson.build | 1 +\n t/t0009-git-dir-validation.sh | 77 +++++++++++++++++++++++++++++++++++\n worktree.c | 6 ++-\n 6 files changed, 121 insertions(+), 14 deletions(-)\n create mode 100755 t/t0009-git-dir-validation.sh\n\ndiff --git a/setup.c b/setup.c\nindex c8336eb20e..3bf96516ba 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -897,8 +897,10 @@ int verify_repository_format(const struct repository_format *format,\n void read_gitfile_error_die(int error_code, const char *path, const char *dir)\n {\n \tswitch (error_code) {\n-\tcase READ_GITFILE_ERR_STAT_FAILED:\n \tcase READ_GITFILE_ERR_NOT_A_FILE:\n+\tcase READ_GITFILE_ERR_STAT_FAILED:\n+\tcase READ_GITFILE_ERR_MISSING:\n+\tcase READ_GITFILE_ERR_IS_A_DIR:\n \t\t/* non-fatal; follow return path */\n \t\tbreak;\n \tcase READ_GITFILE_ERR_OPEN_FAILED:\n@@ -941,8 +943,14 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n \tstatic struct strbuf realpath = STRBUF_INIT;\n \n \tif (stat(path, &st)) {\n-\t\t/* NEEDSWORK: discern between ENOENT vs other errors */\n-\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n+\t\tif (errno == ENOENT || errno == ENOTDIR)\n+\t\t\terror_code = READ_GITFILE_ERR_MISSING;\n+\t\telse\n+\t\t\terror_code = READ_GITFILE_ERR_STAT_FAILED;\n+\t\tgoto cleanup_return;\n+\t}\n+\tif (S_ISDIR(st.st_mode)) {\n+\t\terror_code = READ_GITFILE_ERR_IS_A_DIR;\n \t\tgoto cleanup_return;\n \t}\n \tif (!S_ISREG(st.st_mode)) {\n@@ -1578,20 +1586,37 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,\n \t\tif (offset > min_offset)\n \t\t\tstrbuf_addch(dir, '/');\n \t\tstrbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);\n-\t\tgitdirenv = read_gitfile_gently(dir->buf, die_on_error ?\n-\t\t\t\t\t\tNULL : &error_code);\n+\t\tgitdirenv = read_gitfile_gently(dir->buf, &error_code);\n \t\tif (!gitdirenv) {\n-\t\t\tif (die_on_error ||\n-\t\t\t error_code == READ_GITFILE_ERR_NOT_A_FILE) {\n-\t\t\t\t/* NEEDSWORK: fail if .git is not file nor dir */\n+\t\t\tswitch (error_code) {\n+\t\t\tcase READ_GITFILE_ERR_MISSING:\n+\t\t\t\t/* no .git in this directory, move on */\n+\t\t\t\tbreak;\n+\t\t\tcase READ_GITFILE_ERR_IS_A_DIR:\n \t\t\t\tif (is_git_directory(dir->buf)) {\n \t\t\t\t\tgitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;\n \t\t\t\t\tgitdir_path = xstrdup(dir->buf);\n \t\t\t\t}\n-\t\t\t} else if (error_code != READ_GITFILE_ERR_STAT_FAILED)\n-\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n-\t\t} else\n+\t\t\t\tbreak;\n+\t\t\tcase READ_GITFILE_ERR_STAT_FAILED:\n+\t\t\t\tif (die_on_error)\n+\t\t\t\t\tdie(_(\"error reading '%s'\"), dir->buf);\n+\t\t\t\telse\n+\t\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n+\t\t\tcase READ_GITFILE_ERR_NOT_A_FILE:\n+\t\t\t\tif (die_on_error)\n+\t\t\t\t\tdie(_(\"not a regular file: '%s'\"), dir->buf);\n+\t\t\t\telse\n+\t\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n+\t\t\tdefault:\n+\t\t\t\tif (die_on_error)\n+\t\t\t\t\tread_gitfile_error_die(error_code, dir->buf, NULL);\n+\t\t\t\telse\n+\t\t\t\t\treturn GIT_DIR_INVALID_GITFILE;\n+\t\t\t}\n+\t\t} else {\n \t\t\tgitfile = xstrdup(dir->buf);\n+\t\t}\n \t\t/*\n \t\t * Earlier, we tentatively added DEFAULT_GIT_DIR_ENVIRONMENT\n \t\t * to check that directory for a repository.\ndiff --git a/setup.h b/setup.h\nindex 0738dec244..76fb260c20 100644\n--- a/setup.h\n+++ b/setup.h\n@@ -36,6 +36,8 @@ int is_nonbare_repository_dir(struct strbuf *path);\n #define READ_GITFILE_ERR_NO_PATH 6\n #define READ_GITFILE_ERR_NOT_A_REPO 7\n #define READ_GITFILE_ERR_TOO_LARGE 8\n+#define READ_GITFILE_ERR_MISSING 9\n+#define READ_GITFILE_ERR_IS_A_DIR 10\n void read_gitfile_error_die(int error_code, const char *path, const char *dir);\n const char *read_gitfile_gently(const char *path, int *return_error_code);\n #define read_gitfile(path) read_gitfile_gently((path), NULL)\ndiff --git a/submodule.c b/submodule.c\nindex 508938e4da..767d4c3c35 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2559,7 +2559,7 @@ void absorb_git_dir_into_superproject(const char *path,\n \t\tconst struct submodule *sub;\n \t\tstruct strbuf sub_gitdir = STRBUF_INIT;\n \n-\t\tif (err_code == READ_GITFILE_ERR_STAT_FAILED) {\n+\t\tif (err_code == READ_GITFILE_ERR_MISSING) {\n \t\t\t/* unpopulated as expected */\n \t\t\tstrbuf_release(&gitdir);\n \t\t\treturn;\ndiff --git a/t/meson.build b/t/meson.build\nindex f80e366cff..c4afaacee5 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -80,6 +80,7 @@ integration_tests = [\n 't0006-date.sh',\n 't0007-git-var.sh',\n 't0008-ignores.sh',\n+ 't0009-git-dir-validation.sh',\n 't0010-racy-git.sh',\n 't0012-help.sh',\n 't0013-sha1dc.sh',\ndiff --git a/t/t0009-git-dir-validation.sh b/t/t0009-git-dir-validation.sh\nnew file mode 100755\nindex 0000000000..33d21ed9ea\n--- /dev/null\n+++ b/t/t0009-git-dir-validation.sh\n@@ -0,0 +1,77 @@\n+#!/bin/sh\n+\n+test_description='setup: validation of .git file/directory types\n+\n+Verify that setup_git_directory() correctly handles:\n+1. Valid .git directories (including symlinks to them).\n+2. Invalid .git files (FIFOs, sockets) by erroring out.\n+3. Invalid .git files (garbage) by erroring out.\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup: create parent git repository' '\n+\tgit init parent &&\n+\ttest_commit -C parent \"root-commit\"\n+'\n+\n+test_expect_success SYMLINKS 'setup: .git as a symlink to a directory is valid' '\n+\ttest_when_finished \"rm -rf parent/link-to-dir\" &&\n+\tmkdir -p parent/link-to-dir &&\n+\t(\n+\t\tcd parent/link-to-dir &&\n+\t\tgit init real-repo &&\n+\t\tln -s real-repo/.git .git &&\n+\t\tgit rev-parse --git-dir >actual &&\n+\t\techo .git >expect &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success PIPE 'setup: .git as a FIFO (named pipe) is rejected' '\n+\ttest_when_finished \"rm -rf parent/fifo-trap\" &&\n+\tmkdir -p parent/fifo-trap &&\n+\t(\n+\t\tcd parent/fifo-trap &&\n+\t\tmkfifo .git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"not a regular file\" stderr\n+\t)\n+'\n+\n+test_expect_success SYMLINKS,PIPE 'setup: .git as a symlink to a FIFO is rejected' '\n+\ttest_when_finished \"rm -rf parent/symlink-fifo-trap\" &&\n+\tmkdir -p parent/symlink-fifo-trap &&\n+\t(\n+\t\tcd parent/symlink-fifo-trap &&\n+\t\tmkfifo target-fifo &&\n+\t\tln -s target-fifo .git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"not a regular file\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: .git with garbage content is rejected' '\n+\ttest_when_finished \"rm -rf parent/garbage-trap\" &&\n+\tmkdir -p parent/garbage-trap &&\n+\t(\n+\t\tcd parent/garbage-trap &&\n+\t\techo \"garbage\" >.git &&\n+\t\ttest_must_fail git rev-parse --git-dir 2>stderr &&\n+\t\tgrep \"invalid gitfile format\" stderr\n+\t)\n+'\n+\n+test_expect_success 'setup: .git as an empty directory is ignored' '\n+\ttest_when_finished \"rm -rf parent/empty-dir\" &&\n+\tmkdir -p parent/empty-dir &&\n+\t(\n+\t\tcd parent/empty-dir &&\n+\t\tgit rev-parse --git-dir >expect &&\n+\t\tmkdir .git &&\n+\t\tgit rev-parse --git-dir >actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_done\ndiff --git a/worktree.c b/worktree.c\nindex 9308389cb6..d1165e1d1c 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -653,7 +653,8 @@ static void repair_gitfile(struct worktree *wt,\n \t\t}\n \t}\n \n-\tif (err == READ_GITFILE_ERR_NOT_A_FILE)\n+\tif (err == READ_GITFILE_ERR_NOT_A_FILE ||\n+\t\terr == READ_GITFILE_ERR_IS_A_DIR)\n \t\tfn(1, wt->path, _(\".git is not a file\"), cb_data);\n \telse if (err)\n \t\trepair = _(\".git file broken\");\n@@ -833,7 +834,8 @@ void repair_worktree_at_path(const char *path,\n \t\t\tstrbuf_addstr(&backlink, dotgit_contents);\n \t\t\tstrbuf_realpath_forgiving(&backlink, backlink.buf, 0);\n \t\t}\n-\t} else if (err == READ_GITFILE_ERR_NOT_A_FILE) {\n+\t} else if (err == READ_GITFILE_ERR_NOT_A_FILE ||\n+\t\t\terr == READ_GITFILE_ERR_IS_A_DIR) {\n \t\tfn(1, dotgit.buf, _(\"unable to locate repository; .git is not a file\"), cb_data);\n \t\tgoto done;\n \t} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {\n-- \n2.43.0\n\n"},{"id":"537789","messageId":"xmqqfr6fa63h.fsf@gitster.g","threadId":"65013","inReplyTo":"99c6a437-3fc3-4d9a-9465-4c47a9777776@gmail.com","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-04T16:53:54Z","receivedAt":"2026-03-04T16:53:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen writes:\n\n> Hi Junio and Phillip,\n>\n> Thanks for the detailed reply!\n>\n> After reading through your discussion, I believe the most crucial point is:\n>\n>> \"We were given an invalid GIT_DIR, we are not doing\n>> discovery, hence we are operating without a repository\"\n>\n> If I understand correctly, the expected behavior should be: when a user \n> explicitly passes 'GIT_DIR=/dev/null git diff', Git should no longer \n> need to \"search\" or \"guess\" anything. Instead, if it's a trash file (or \n> something similar) rather a repository, Git should simply act as if no \n> repository exists. Is that correct?\n\nThat is one of the things. The broken test highlighted that\nGIT_DIR_EXPLICIT case needs more thought than what we have discussed\nso far, but there may be other cases that we need to also think\nabout. See what different cases are in the big switch statement in\nsetup_git_directory_gently().\n\n> So what I'm doing next is:\n>\n>> All calls to read_gitfile_gently(path, NULL) need to be\n>> audited and then we need to decide which ones to leave lenient, and\n>> which ones are OK to tighten together with the call used during the\n>> repository discovery.\n>\n> Will be working on it in the next few days.\n\nThanks.\n"},{"id":"537795","messageId":"fc2aaed9-ecc3-4efa-bdef-e6ac951c1d5b@gmail.com","threadId":"65013","inReplyTo":"xmqqfr6fa63h.fsf@gitster.g","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-03-04T17:35:11Z","receivedAt":"2026-03-04T17:35:15Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi Junio,\n\n> That is one of the things. The broken test highlighted that\n> GIT_DIR_EXPLICIT case needs more thought than what we have discussed\n> so far, but there may be other cases that we need to also think\n> about. See what different cases are in the big switch statement in\n> setup_git_directory_gently().\n\nShortly before your email I had already sent the v12 patch.\n\nI did make changes in setup_git_directory_gently_1(), and logically it \nshouldn't cause major issues.\n\nUnfortunately, looking back now, my implementation barely qualifies as \n“functional” and actually undermines the purpose of setup_git..() \nitself. It's a mess — I rushed into it without properly reviewing the \ncontext (like how other cases are handled) :(((\n\nI'll polish it thoroughly before releasing v13.\n\nRegards,\n\nYuchen\n"},{"id":"537798","messageId":"xmqqcy1j8o5r.fsf@gitster.g","threadId":"65013","inReplyTo":"fc2aaed9-ecc3-4efa-bdef-e6ac951c1d5b@gmail.com","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-04T18:06:40Z","receivedAt":"2026-03-04T18:06:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen writes:\n\n> Unfortunately, looking back now, my implementation barely qualifies as \n> “functional” and actually undermines the purpose of setup_git..() \n> itself. It's a mess — I rushed into it without properly reviewing the \n> context (like how other cases are handled) :(((\n\nv12 does work differently from what we have been aiming for, but I\nfind that it arguably is a much safer approach.\n\nEven though the updated read_gitfile_gently() returns finer-grained\nREAD_GITFILE_ERR_* codes than the original, read_gitfile_error_die()\ndoes not change behaviour from the original. Any caller that use\nread_gitfile(path), which is read_gitfile_gently(path, NULL), like\nthe setup_explicit_git_dir() codepath we have been looking at lately,\nlets read_gitfile_error_die() react to the error code, which is to\nbehave exactly as what the code did before this patch.\n\nSo, I dunno. After all, these two NEEDSWORK comments have been with\nus for quite some time, and reminded us that we may want to consider\nif we need to do anything differently. I do not think we mind if we\nconclude negatively, taking \"no, it is of dubious value to tighten\nerror checking in these code paths\" as an answer to these NEEDSWORK\ncomments. v12 is slightly less defeatest than that stance in that\nwe are only allowing the callers that care about what kind of errors\nthey are getting and and want to decide how to react to them, while\nkeeping the default error behaviour the same for those who do not\nask with &error_code what kind of errors we saw.\n\nThe patch makes the behaviour change for callers that pass an\n&error_code pointer to read_gitfile_gently() and act on the returned\nerror code itself, like the discovery code path. As long as these\ncallers are audited and adjusted as necessary, we have very little\nrisk of regression.\n\nI won't be doing a full audit in this message, but just to give\ntaste of what is expected ...\n\n$ git grep -n -e 'read_gitfile_gently('\n\nbuiltin/init-db.c:212:\t\tp = read_gitfile_gently(git_dir, &err);\n\nThis caller gives &err but it never looks at what is in it after the\ncall returns, so there shouldn't be any behaviour change.\n\nsetup.c:465:\tif (read_gitfile_gently(path->buf, &gitfile_error) || is_git_directory(path->buf))\n\nThis is followed by \n\n\t\tret = 1;\n if (gitfile_error == READ_GITFILE_ERR_OPEN_FAILED ||\n gitfile_error == READ_GITFILE_ERR_READ_FAILED)\n ret = 1;\n\nI do not offhand know if this list of \"error codes that should\nresult in returning 1 from this function\" needs to be tweaked to\nadjust for the change in this patch.\n\nworktree.c:390:\tpath = xstrdup_or_null(read_gitfile_gently(wt_path.buf, &err));\n\nThis is followed by code that reacts to path being NULL and shows\nthe contents of err in an error message. Should be benign.\n\nThanks.\n\n"},{"id":"537824","messageId":"00f6d468-7d00-4edc-886d-723322420539@gmail.com","threadId":"65013","inReplyTo":"xmqqcy1j8o5r.fsf@gitster.g","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-03-04T18:41:46Z","receivedAt":"2026-03-04T18:41:50Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hi Junio,\n\nOn 3/5/26 02:06, Junio C Hamano wrote:\n\n> v12 does work differently from what we have been aiming for, but I\n> find that it arguably is a much safer approach.\n\nMaybe, but my main concern was that adding 'die()' in \n'setup_git_directory_gently_1()' might not be the best choice. \nConsidering the implementations of the preceding functions, I though \nlocating 'die()' in 'setup_explicit_git_dir()' might be a better choice? \nBy the way, I noticed there's a '(read_gitfile(path))' macro that \nexpands to 'read_gitfile(path, NULL)'. I was planning to pass \n'error_code' here, essentially moving the logic from the original \n'setup_git_directory_gently_1()' to this location, where the former \nwould only be responsible for returning the error status... The changes \nwould be a bit too extensive if I did it that way.\n\nSince you say the v12 patch is safer, I have no objections. I'm \ncompletely unsure about the overall structure of this part. Skill issue.\n\n> Even though the updated read_gitfile_gently() returns finer-grained\n> READ_GITFILE_ERR_* codes than the original, read_gitfile_error_die()\n> does not change behaviour from the original. Any caller that use\n> read_gitfile(path), which is read_gitfile_gently(path, NULL), like\n> the setup_explicit_git_dir() codepath we have been looking at lately,\n> lets read_gitfile_error_die() react to the error code, which is to\n> behave exactly as what the code did before this patch.\n\nI should have considered this. I feel that the changes in v12 are \nactually more like a band-aid fix compared to the previous ones.\n\nYou mentioned this change didn't touch 'read_gitfile_error_die()', and I \nthink that's one of the benefits of such simple modifications. I just \ncan't be entirely sure what the trade-offs might be — perhaps it's my \nlack of experience, but I always worry about digging a deep hole for \nfuture developers to fill. I'll keep learning.\n\n> So, I dunno. After all, these two NEEDSWORK comments have been with\n> us for quite some time, and reminded us that we may want to consider\n> if we need to do anything differently. I do not think we mind if we\n> conclude negatively, taking \"no, it is of dubious value to tighten\n> error checking in these code paths\" as an answer to these NEEDSWORK\n> comments. v12 is slightly less defeatest than that stance in that\n> we are only allowing the callers that care about what kind of errors\n> they are getting and and want to decide how to react to them, while\n> keeping the default error behaviour the same for those who do not\n> ask with &error_code what kind of errors we saw.\n\nI see.\n\n> The patch makes the behaviour change for callers that pass an\n> &error_code pointer to read_gitfile_gently() and act on the returned\n> error code itself, like the discovery code path. As long as these\n> callers are audited and adjusted as necessary, we have very little\n> risk of regression.\n> \n> I won't be doing a full audit in this message, but just to give\n> taste of what is expected ...\n> \n> $ git grep -n -e 'read_gitfile_gently('\n> \n> builtin/init-db.c:212:\t\tp = read_gitfile_gently(git_dir, &err);\n> \n> This caller gives &err but it never looks at what is in it after the\n> call returns, so there shouldn't be any behaviour change.\n> \n> setup.c:465:\tif (read_gitfile_gently(path->buf, &gitfile_error) || is_git_directory(path->buf))\n> \n> This is followed by\n> \n> \t\tret = 1;\n> if (gitfile_error == READ_GITFILE_ERR_OPEN_FAILED ||\n> gitfile_error == READ_GITFILE_ERR_READ_FAILED)\n> ret = 1;\n> \n> I do not offhand know if this list of \"error codes that should\n> result in returning 1 from this function\" needs to be tweaked to\n> adjust for the change in this patch.\n> \n> worktree.c:390:\tpath = xstrdup_or_null(read_gitfile_gently(wt_path.buf, &err));\n> \n> This is followed by code that reacts to path being NULL and shows\n> the contents of err in an error message. Should be benign.\n\nI don't think this needs to be changed. If a new error_code \nREAD_GITFILE_MISSING is encountered, then this isn't a repository at \nall, so it won't enter the if statement, and ret = 0.\n\nIf encountering the READ_GITFILE_ERR_IS_A_DIR, the \nis_git_directory(path->buf) check takes precedence. If it detects a \nvalid .git directory, it returns 1. Therefore, the gitfile_error \nfunction doesn't need to match IS_A_DIR at all.\n\nThank you for taking the time to review my (roughly made) patch.\n\nGoodnight,\n\nYuchen\n"},{"id":"537852","messageId":"xmqqbjh35hvv.fsf@gitster.g","threadId":"65013","inReplyTo":"00f6d468-7d00-4edc-886d-723322420539@gmail.com","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-04T22:50:28Z","receivedAt":"2026-03-04T22:50:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen writes:\n\n> Maybe, but my main concern was that adding 'die()' in \n> 'setup_git_directory_gently_1()' might not be the best choice. \n> Considering the implementations of the preceding functions, I though \n> locating 'die()' in 'setup_explicit_git_dir()' might be a better choice? \n\nYou may be right. I didn't take a careful enough look to comment.\n\n> By the way, I noticed there's a '(read_gitfile(path))' macro that \n> expands to 'read_gitfile(path, NULL)'. I was planning to pass \n> 'error_code' here, essentially moving the logic from the original \n> 'setup_git_directory_gently_1()' to this location, where the former \n> would only be responsible for returning the error status... The changes \n> would be a bit too extensive if I did it that way.\n\nTrue. It would be a lot more invasive change. I do not know if it\nis worth our time _right_ _now_, or if it is better to be left for\nfuture iterations.\n"},{"id":"537931","messageId":"a5e41bd1-af10-49cd-85dc-8e668f1d8970@gmail.com","threadId":"65013","inReplyTo":"xmqqbjh35hvv.fsf@gitster.g","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-03-05T12:40:08Z","receivedAt":"2026-03-05T12:40:12Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"On 3/5/26 06:50, Junio C Hamano wrote:\n> Tian Yuchen writes:\n> \n>> Maybe, but my main concern was that adding 'die()' in\n>> 'setup_git_directory_gently_1()' might not be the best choice.\n>> Considering the implementations of the preceding functions, I though\n>> locating 'die()' in 'setup_explicit_git_dir()' might be a better choice?\n> \n> You may be right. I didn't take a careful enough look to comment.\n> \n>> By the way, I noticed there's a '(read_gitfile(path))' macro that\n>> expands to 'read_gitfile(path, NULL)'. I was planning to pass\n>> 'error_code' here, essentially moving the logic from the original\n>> 'setup_git_directory_gently_1()' to this location, where the former\n>> would only be responsible for returning the error status... The changes\n>> would be a bit too extensive if I did it that way.\n> \n> True. It would be a lot more invasive change. I do not know if it\n> is worth our time _right_ _now_, or if it is better to be left for\n> future iterations.\n\nI will hold off on any further iterations and leave v12 as is, unless \nyou or others spot any specific details in it that still need tweaking.\n\nThank you so much for the patience and guidance throughout this entire \nseries! I really learned a lot from it.\n\nRegards,\n\nYuchen\n"},{"id":"538337","messageId":"xmqqcy1c1szh.fsf@gitster.g","threadId":"65013","inReplyTo":"a5e41bd1-af10-49cd-85dc-8e668f1d8970@gmail.com","subject":"Re: [PATCH v11] setup: improve error diagnosis for invalid .git files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-09T23:30:10Z","receivedAt":"2026-03-09T23:30:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen writes:\n\n>> True. It would be a lot more invasive change. I do not know if it\n>> is worth our time _right_ _now_, or if it is better to be left for\n>> future iterations.\n>\n> I will hold off on any further iterations and leave v12 as is, unless \n> you or others spot any specific details in it that still need tweaking.\n>\n> Thank you so much for the patience and guidance throughout this entire \n> series! I really learned a lot from it.\n\nSounds good. Let me mark the topic for 'next'.\n"}]}