On Tue, May 3, 2016 at 11:48 AM, Jeff King <peff@xxxxxxxx> wrote: > On Tue, May 03, 2016 at 11:05:09AM -0700, Junio C Hamano wrote: > >> On Tue, May 3, 2016 at 11:02 AM, Stefan Beller <sbeller@xxxxxxxxxx> wrote: >> > >> > So I wonder if is_nonbare_repository_dir() is the culprit here. >> > (We do a chmod 0 on the .git before the `git clean` in the test to confuse Git) >> >> Ask if the test is run as root; if so, then mark the test to require >> SANITY prerequisite. > > Yeah, I can easily reproduce the failure with `sudo ./t7300-clean.sh`. > So the immediate fix is the SANITY prereq. > > Looking at Stefan's message, I wondered if the patch he came up with: > > diff --git a/setup.c b/setup.c > index 3439ec6..4cfba8f 100644 > --- a/setup.c > +++ b/setup.c > @@ -323,8 +323,7 @@ int is_nonbare_repository_dir(struct strbuf *path) > strbuf_addstr(path, ".git"); > if (read_gitfile_gently(path->buf, &gitfile_error) || > is_git_directory(path->buf)) > ret = 1; > - if (gitfile_error == READ_GITFILE_ERR_OPEN_FAILED || > - gitfile_error == READ_GITFILE_ERR_READ_FAILED) > + if (gitfile_error) > ret = 1; > strbuf_setlen(path, orig_path_len); > return ret; > > is related or worth doing on top. But I don't think so. That code is > just trying to convert some error-cases into "let's err on the side of > assuming it is a repo". Doing that for all values of gitfile_error is > definitely the wrong thing (it would treat a totally non-existent > ".git" file as "yes, it's there", which is clearly bogus). The proposed change is overly eager indeed. What if we get back a READ_GITFILE_ERR_STAT_FAILED ? I would think that is a reasonable indicator of a submodule being there? (The stat failure may be transient ENOMEM Out of memory (i.e., kernel memory).) By being overly eager I wanted to make sure the assumptions I had were wrong. I'm about to send the SANITY prerequisite patch, Thanks, Stefan > > -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html