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 <sbel...@google.com> 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).

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to