git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Stefan Beller" <sbeller@google.com>,
	"Jan Keromnes" <janx@linux.com>,
	"Erik Elfström" <erik.elfstrom@gmail.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build
Date: Tue, 3 May 2016 14:48:41 -0400	[thread overview]
Message-ID: <20160503184841.GA30530@sigill.intra.peff.net> (raw)
In-Reply-To: <CAPc5daX4noqHaOZCrZ78s0ytFBkeU7znXs0QY1LXEw8w3RnQ1A@mail.gmail.com>

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@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

  reply	other threads:[~2016-05-03 18:49 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAA6PgK7b=ithSYREV5axaE3fmRG5Vp06UtWiZXD-aJuZKfEVYA@mail.gmail.com>
2016-04-29 12:53 ` Fwd: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build Jan Keromnes
2016-04-29 12:58   ` Johannes Schindelin
2016-04-29 17:06   ` Stefan Beller
2016-05-03  9:19     ` Jan Keromnes
2016-05-03 18:02       ` Stefan Beller
2016-05-03 18:05         ` Junio C Hamano
2016-05-03 18:48           ` Jeff King [this message]
2016-05-03 18:53             ` Stefan Beller
2016-05-03 19:00               ` Jeff King
2016-05-03 21:28                 ` erik elfström

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160503184841.GA30530@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=erik.elfstrom@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=janx@linux.com \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).