Re: [PATCH v10] setup: improve error diagnosis for invalid .git files
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 23, 2026, 00:23 UTC
- Message-ID
- <xmqqqzqcpatz.fsf@gitster.g>
- In-Reply-To
- <xmqq4in8quxn.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
Show 28 quoted lines
>> setup.c | 42 ++++++++++++++------ >> setup.h | 2 + >> submodule.c | 2 +- >> t/meson.build | 1 + >> t/t0009-git-dir-validation.sh | 72 +++++++++++++++++++++++++++++++++++ >> worktree.c | 6 ++- >> 6 files changed, 110 insertions(+), 15 deletions(-) >> create mode 100755 t/t0009-git-dir-validation.sh > > We'd probably need to treat ENOTDIR the same way as ENOENT to deal > with cases where we expect a directory "sm1" to be the root of a > submodule working tree, and we have a modification that removes the > submodule directory and replace it with a regular file "sm1". In > the code path touched by this patch in submodule.c, we would ask "is > sm1/.git a git directory?" and the stat(2) call on that path in > read_gitfile_gently() used to say "Ah, a failure, that means we > cannot positively say that 'sm1/.git' is a git directory or a gitdir > file." Now we inspect the error code in an attempt to tell if it is > a system failure (e.g., a corrupt filesystem), but catching only > ENOENT is probably a bit too tight. In the above scenario, asking > about 'sm1/.git' when 'sm1' is a regular file will not result in > ENOENT but in ENOTDIR (i.e., "the leading 'sm1' is not a directory so > it makes no sense to ask about 'sm1/.git'"). > > Is it always sensible to treat ENOTDIR and ENOENT as two equivalent > errors for the purpose of read_gitfile_gently()? I have no clear > answer offhand myself. This is part of what we need to think about > and resolve while addressing the original "NEEDSWORK:" comment.
----- >8 ----- Subject: [PATCH] read_gitfile(): group ENOENT and ENOTDIR into a single MISSING error
The code from the previous step does not deal wellwith a case where we check if "sm/.git" is a good directory after replacing "sm" with a regular file. ENOTDIR is returned when we ask about "sm/.git", not ENOENT, and the code would want to handle both.
I am not convinced if this is a good change, though. Outside the submodule caller, the story might be different and we may want to treat ENOTDIR differently from ENOENT. I dunno. That is why this is not squashed into the patch (yet).
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- setup.c | 8 ++++---- setup.h | 2 +- submodule.c | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/setup.c b/setup.c index b79a9233f5..d4afbed3eb 100644 --- a/setup.c +++ b/setup.c @@ -895,7 +895,7 @@ int verify_repository_format(const struct repository_format *format, void read_gitfile_error_die(int error_code, const char *path, const char *dir) { switch (error_code) { - case READ_GITFILE_ERR_STAT_ENOENT: + case READ_GITFILE_ERR_STAT_MISSING: case READ_GITFILE_ERR_IS_A_DIR: /* non-fatal; follow return path */ break; @@ -943,8 +943,8 @@ const char *read_gitfile_gently(const char *path, int *return_error_code) static struct strbuf realpath = STRBUF_INIT; if (stat(path, &st)) { - if (errno == ENOENT) - error_code = READ_GITFILE_ERR_STAT_ENOENT; + if (errno == ENOENT || errno == ENOTDIR) + error_code = READ_GITFILE_ERR_STAT_MISSING; else error_code = READ_GITFILE_ERR_STAT_FAILED; goto cleanup_return; @@ -1589,7 +1589,7 @@ static enum discovery_result setup_git_directory_gently_1(struct strbuf *dir, gitdirenv = read_gitfile_gently(dir->buf, &error_code); if (!gitdirenv) { switch (error_code) { - case READ_GITFILE_ERR_STAT_ENOENT: + case READ_GITFILE_ERR_STAT_MISSING: /* no .git in this directory, move on */ break; case READ_GITFILE_ERR_IS_A_DIR: diff --git a/setup.h b/setup.h index c23629cb4f..cc45f962fa 100644 --- a/setup.h +++ b/setup.h @@ -36,7 +36,7 @@ int is_nonbare_repository_dir(struct strbuf *path); #define READ_GITFILE_ERR_NO_PATH 6 #define READ_GITFILE_ERR_NOT_A_REPO 7 #define READ_GITFILE_ERR_TOO_LARGE 8 -#define READ_GITFILE_ERR_STAT_ENOENT 9 +#define READ_GITFILE_ERR_STAT_MISSING 9 #define READ_GITFILE_ERR_IS_A_DIR 10 void read_gitfile_error_die(int error_code, const char *path, const char *dir); const char *read_gitfile_gently(const char *path, int *return_error_code); diff --git a/submodule.c b/submodule.c index 52a7cf0e43..fc85a7a1d8 100644 --- a/submodule.c +++ b/submodule.c @@ -2413,7 +2413,7 @@ void absorb_git_dir_into_superproject(const char *path, const struct submodule *sub; struct strbuf sub_gitdir = STRBUF_INIT; - if (err_code == READ_GITFILE_ERR_STAT_ENOENT) { + if (err_code == READ_GITFILE_ERR_STAT_MISSING) { /* unpopulated as expected */ strbuf_release(&gitdir); return;
-- 2.53.0-455-g62fcd67e6e