Re: [PATCH v3] setup: fail if .git is not a file or directory
diff --git a/setup.c b/setup.c
index 3a6a048620..269aa9faaa 100644
--- a/setup.c
+++ b/setup.c
@@ -939,8 +939,14 @@ const char *read_gitfile_gently(const char *path,
int *return_error_code)
static struct strbuf realpath = STRBUF_INIT;
if (stat(path, &st)) {
- /* NEEDSWORK: discern between ENOENT vs other errors */
- error_code = READ_GITFILE_ERR_STAT_FAILED;
+ if (errno == ENOENT)
+ error_code = READ_GITFILE_ERR_STAT_ENOENT;
+ else
+ error_code = READ_GITFILE_ERR_STAT_FAILED;
+ goto cleanup_return;
+ }
+ if (S_ISDIR(st.st_mode)) {
+ error_code = READ_GITFILE_ERR_IS_A_DIR;
goto cleanup_return;
}
if (!S_ISREG(st.st_mode)) {
@@ -994,7 +1000,9 @@ const char *read_gitfile_gently(const char *path,
int *return_error_code)
cleanup_return:
if (return_error_code)
*return_error_code = error_code;
- else if (error_code)
+ else if (error_code &&
+ error_code != READ_GITFILE_ERR_STAT_ENOENT &&
+ error_code != READ_GITFILE_ERR_IS_A_DIR)
read_gitfile_error_die(error_code, path, dir);
free(buf);
@@ -1576,20 +1584,27 @@ static enum discovery_result
setup_git_directory_gently_1(struct strbuf *dir,
if (offset > min_offset)
strbuf_addch(dir, '/');
strbuf_addstr(dir, DEFAULT_GIT_DIR_ENVIRONMENT);
- gitdirenv = read_gitfile_gently(dir->buf, die_on_error ?
- NULL : &error_code);
+ gitdirenv = read_gitfile_gently(dir->buf, &error_code);
if (!gitdirenv) {
- if (die_on_error ||
- error_code == READ_GITFILE_ERR_NOT_A_FILE) {
- /* NEEDSWORK: fail if .git is not file nor dir */
+ if (error_code == READ_GITFILE_ERR_STAT_ENOENT) {
+ ;
+ } else if (error_code == READ_GITFILE_ERR_IS_A_DIR) {
if (is_git_directory(dir->buf)) {
gitdirenv = DEFAULT_GIT_DIR_ENVIRONMENT;
gitdir_path = xstrdup(dir->buf);
}
- } else if (error_code != READ_GITFILE_ERR_STAT_FAILED)
- return GIT_DIR_INVALID_GITFILE;
- } else
- gitfile = xstrdup(dir->buf);
+ } else if (error_code == READ_GITFILE_ERR_NOT_A_FILE) {
+ if (die_on_error)
+ die(_("Invalid %s: not a regular file or directory"), dir->buf);
+ else
+ return GIT_DIR_INVALID_GITFILE;
+ } else if (error_code != READ_GITFILE_ERR_STAT_FAILED) {
+ if (die_on_error)
+ read_gitfile_error_die(error_code, dir->buf, NULL);
+ else
+ return GIT_DIR_INVALID_GITFILE;
+ }
+ }
/*
* Earlier, we tentatively added DEFAULT_GIT_DIR_ENVIRONMENT
* to check that directory for a repository.
diff --git a/setup.h b/setup.h
index d55dcc6608..0271cc8f93 100644
--- a/setup.h
+++ b/setup.h
@@ -36,6 +36,9 @@ 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_IS_A_DIR 10
+
Still working on it.
Looks clearer, right? I just wanna confirm whether I'm on the right
track. Notice that I didn't change the following part: (though you
mentioned earlier)
> cleanup_return:
> if (return_error_code)
> *return_error_code = error_code;
>- else if (error_code)
>+ else if (error_code &&
>+ error_code != READ_GITFILE_ERR_STAT_ENOENT &&
>+ error_code != READ_GITFILE_ERR_IS_A_DIR)
> read_gitfile_error_die(error_code, path, dir);
My understanding is that we haven't distinguished between “unexpected”
(ENOENT, IS_A_DIR) and “errors” (NOT_A_FILE, format error) in the code
using a categorical approach. Instead, we differentiate them solely
based on whether they are listed in `read_gitfile_error_die()` (if I
haven't missed anything). Consequently, retaining this code here seems
to be the easiest and most readable approach to ensure that it doesn't
return prematurely. However, I believe this check shouldn't reside here.
That's why I'm replying to you again.
(By the way, I've always been a bit curious about how your name is
pronounced :/)
Regards,
Yuchen