setup: improve error diagnosis for invalid .git files
'read_gitfile_gently()' treats any non-regular file as 'READ_GITFILE_ERR_NOT_A_FILE' and fails to discern between 'ENOENT' and other stat failures. This flawed error reporting is noted by two 'NEEDSWORK' comments. Address these comments by introducing two new error codes: 'READ_GITFILE_ERR_MISSING'(which groups the "file missing" scenarios together) and 'READ_GITFILE_ERR_IS_A_DIR': 1. Update 'read_gitfile_error_die()' to treat 'IS_A_DIR', 'MISSING', 'NOT_A_FILE' and 'STAT_FAILED' as non-fatal no-ops. This accommodates intentional non-repo scenarios (e.g., GIT_DIR=/dev/null). 2. Explicitly catch 'NOT_A_FILE' and 'STAT_FAILED' during discovery and call 'die()' if 'die_on_error' is set. 3. Unconditionally pass '&error_code' to 'read_gitfile_gently()'. 4. Only invoke 'is_git_directory()' when we explicitly receive 'READ_GITFILE_ERR_IS_A_DIR', avoiding redundant checks. Additionally, audit external callers of 'read_gitfile_gently()' in 'submodule.c' and 'worktree.c' to accommodate the refined error codes. Signed-off-by: Tian Yuchen <a3205153416@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Tian Yuchen committed
Mar 4, 2026 at 22:15 UTC
1dd27bfbfdc0f3b2071ecb8b505476f4caa56a13
6 files changed
+121
-14
setup.c
+36
-11
@@ -895,8 +895,10 @@ int verify_repository_format(const struct repository_format *format,
895
void read_gitfile_error_die(int error_code, const char *path, const char *dir)
896
{
897
switch (error_code) {
898
- case READ_GITFILE_ERR_STAT_FAILED:
898
case READ_GITFILE_ERR_NOT_A_FILE:
899
+ case READ_GITFILE_ERR_STAT_FAILED:
900
+ case READ_GITFILE_ERR_MISSING:
901
+ case READ_GITFILE_ERR_IS_A_DIR:
902
/* non-fatal; follow return path */
903
break;
904
case READ_GITFILE_ERR_OPEN_FAILED:
@@ -939,8 +941,14 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
941
static struct strbuf realpath = STRBUF_INIT;
942
943
if (stat(path, &st)) {
942
- /* NEEDSWORK: discern between ENOENT vs other errors */
943
- error_code = READ_GITFILE_ERR_STAT_FAILED;
944
+ if (errno == ENOENT || errno == ENOTDIR)
945
+ error_code = READ_GITFILE_ERR_MISSING;
946
+ else
947
+ error_code = READ_GITFILE_ERR_STAT_FAILED;
948
+ goto cleanup_return;
949
+ }
950
+ if (S_ISDIR(st.st_mode)) {
951
+ error_code = READ_GITFILE_ERR_IS_A_DIR;
952
goto cleanup_return;
953
}
954
if (!S_ISREG(st.st_mode)) {
@@ -1576,20 +1584,37 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir,
1584
if (offset > min_offset)
1585
strbuf_addch(dir, '/');
1586
strbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);
1579
- gitdirenv = read_gitfile_gently(dir->buf, die_on_error ?
1580
- NULL : &error_code);
1587
+ gitdirenv = read_gitfile_gently(dir->buf, &error_code);
1588
if (!gitdirenv) {
1582
- if (die_on_error ||
1583
- error_code == READ_GITFILE_ERR_NOT_A_FILE) {
1584
- /* NEEDSWORK: fail if .git is not file nor dir */
1589
+ switch (error_code) {
1590
+ case READ_GITFILE_ERR_MISSING:
1591
+ /* no .git in this directory, move on */
1592
+ break;
1593
+ case READ_GITFILE_ERR_IS_A_DIR:
1594
if (is_git_directory(dir->buf)) {
1595
gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
1596
gitdir_path = xstrdup(dir->buf);
1597
}
1589
- } else if (error_code != READ_GITFILE_ERR_STAT_FAILED)
1590
- return GIT_DIR_INVALID_GITFILE;
1591
- } else
1598
+ break;
1599
+ case READ_GITFILE_ERR_STAT_FAILED:
1600
+ if (die_on_error)
1601
+ die(_("error reading '%s'"), dir->buf);
1602
+ else
1603
+ return GIT_DIR_INVALID_GITFILE;
1604
+ case READ_GITFILE_ERR_NOT_A_FILE:
1605
+ if (die_on_error)
1606
+ die(_("not a regular file: '%s'"), dir->buf);
1607
+ else
1608
+ return GIT_DIR_INVALID_GITFILE;
1609
+ default:
1610
+ if (die_on_error)
1611
+ read_gitfile_error_die(error_code, dir->buf, NULL);
1612
+ else
1613
+ return GIT_DIR_INVALID_GITFILE;
1614
+ }
1615
+ } else {
1616
gitfile = xstrdup(dir->buf);
1617
+ }
1618
/*
1619
* Earlier, we tentatively added DEFAULT_GIT_DIR_ENVIRONMENT
1620
* to check that directory for a repository.
setup.h
+2
@@ -36,6 +36,8 @@ int is_nonbare_repository_dir(struct strbuf *path);
36
#define READ_GITFILE_ERR_NO_PATH 6
37
#define READ_GITFILE_ERR_NOT_A_REPO 7
38
#define READ_GITFILE_ERR_TOO_LARGE 8
39
+#define READ_GITFILE_ERR_MISSING 9
40
+#define READ_GITFILE_ERR_IS_A_DIR 10
41
void read_gitfile_error_die(int error_code, const char *path, const char *dir);
42
const char *read_gitfile_gently(const char *path, int *return_error_code);
43
#define read_gitfile(path) read_gitfile_gently((path), NULL)
submodule.c
+1
-1
@@ -2413,7 +2413,7 @@ void absorb_git_dir_into_superproject(const char *path,
2413
const struct submodule *sub;
2414
struct strbuf sub_gitdir = STRBUF_INIT;
2415
2416
- if (err_code == READ_GITFILE_ERR_STAT_FAILED) {
2416
+ if (err_code == READ_GITFILE_ERR_MISSING) {
2417
/* unpopulated as expected */
2418
strbuf_release(&gitdir);
2419
return;
t/meson.build
+1
@@ -80,6 +80,7 @@ integration_tests = [
80
't0006-date.sh',
81
't0007-git-var.sh',
82
't0008-ignores.sh',
83
+ 't0009-git-dir-validation.sh',
84
't0010-racy-git.sh',
85
't0012-help.sh',
86
't0013-sha1dc.sh',
t/t0009-git-dir-validation.sh
new
+77
@@ -0,0 +1,77 @@
1
+#!/bin/sh
2
+
3
+test_description='setup: validation of .git file/directory types
4
+
5
+Verify that setup_git_directory() correctly handles:
6
+1. Valid .git directories (including symlinks to them).
7
+2. Invalid .git files (FIFOs, sockets) by erroring out.
8
+3. Invalid .git files (garbage) by erroring out.
9
+'
10
+
11
+. ./test-lib.sh
12
+
13
+test_expect_success 'setup: create parent git repository' '
14
+ git init parent &&
15
+ test_commit -C parent "root-commit"
16
+'
17
+
18
+test_expect_success SYMLINKS 'setup: .git as a symlink to a directory is valid' '
19
+ test_when_finished "rm -rf parent/link-to-dir" &&
20
+ mkdir -p parent/link-to-dir &&
21
+ (
22
+ cd parent/link-to-dir &&
23
+ git init real-repo &&
24
+ ln -s real-repo/.git .git &&
25
+ git rev-parse --git-dir >actual &&
26
+ echo .git >expect &&
27
+ test_cmp expect actual
28
+ )
29
+'
30
+
31
+test_expect_success PIPE 'setup: .git as a FIFO (named pipe) is rejected' '
32
+ test_when_finished "rm -rf parent/fifo-trap" &&
33
+ mkdir -p parent/fifo-trap &&
34
+ (
35
+ cd parent/fifo-trap &&
36
+ mkfifo .git &&
37
+ test_must_fail git rev-parse --git-dir 2>stderr &&
38
+ grep "not a regular file" stderr
39
+ )
40
+'
41
+
42
+test_expect_success SYMLINKS,PIPE 'setup: .git as a symlink to a FIFO is rejected' '
43
+ test_when_finished "rm -rf parent/symlink-fifo-trap" &&
44
+ mkdir -p parent/symlink-fifo-trap &&
45
+ (
46
+ cd parent/symlink-fifo-trap &&
47
+ mkfifo target-fifo &&
48
+ ln -s target-fifo .git &&
49
+ test_must_fail git rev-parse --git-dir 2>stderr &&
50
+ grep "not a regular file" stderr
51
+ )
52
+'
53
+
54
+test_expect_success 'setup: .git with garbage content is rejected' '
55
+ test_when_finished "rm -rf parent/garbage-trap" &&
56
+ mkdir -p parent/garbage-trap &&
57
+ (
58
+ cd parent/garbage-trap &&
59
+ echo "garbage" >.git &&
60
+ test_must_fail git rev-parse --git-dir 2>stderr &&
61
+ grep "invalid gitfile format" stderr
62
+ )
63
+'
64
+
65
+test_expect_success 'setup: .git as an empty directory is ignored' '
66
+ test_when_finished "rm -rf parent/empty-dir" &&
67
+ mkdir -p parent/empty-dir &&
68
+ (
69
+ cd parent/empty-dir &&
70
+ git rev-parse --git-dir >expect &&
71
+ mkdir .git &&
72
+ git rev-parse --git-dir >actual &&
73
+ test_cmp expect actual
74
+ )
75
+'
76
+
77
+test_done
worktree.c
+4
-2
@@ -653,7 +653,8 @@ static void repair_gitfile(struct worktree *wt,
653
}
654
}
655
656
- if (err == READ_GITFILE_ERR_NOT_A_FILE)
656
+ if (err == READ_GITFILE_ERR_NOT_A_FILE ||
657
+ err == READ_GITFILE_ERR_IS_A_DIR)
658
fn(1, wt->path, _(".git is not a file"), cb_data);
659
else if (err)
660
repair = _(".git file broken");
@@ -833,7 +834,8 @@ void repair_worktree_at_path(const char *path,
834
strbuf_addstr(&backlink, dotgit_contents);
835
strbuf_realpath_forgiving(&backlink, backlink.buf, 0);
836
}
836
- } else if (err == READ_GITFILE_ERR_NOT_A_FILE) {
837
+ } else if (err == READ_GITFILE_ERR_NOT_A_FILE ||
838
+ err == READ_GITFILE_ERR_IS_A_DIR) {
839
fn(1, dotgit.buf, _("unable to locate repository; .git is not a file"), cb_data);
840
goto done;
841
} else if (err == READ_GITFILE_ERR_NOT_A_REPO) {