git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build

From
Stefan Beller <sbeller@google.com>
Date
May 3, 2016, 18:53 UTC
Message-ID
<CAGZ79kYxSCsXGLBoBTAOLfe_pOu7YNBQoccs_fA0btEzpnU=-w@mail.gmail.com>
In-Reply-To
<20160503184841.GA30530@sigill.intra.peff.net>
On Tue, May 3, 2016 at 11:48 AM, Jeff King <peff@peff.net> wrote:
Show 36 quoted lines
> 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).

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
Previous: Jeff KingNext: Jeff King
Message 8 of 10 in “Fwd: Git 2.8.1 fails test 32 of t7300-clean.sh, breaks profile build”
  1. Jan KeromnesApr 29, 2016
  2. Johannes SchindelinApr 29, 2016
  3. Stefan BellerApr 29, 2016
  4. Jan KeromnesMay 3, 2016
  5. Stefan BellerMay 3, 2016
  6. Junio C HamanoMay 3, 2016
  7. Jeff KingMay 3, 2016
  8. Stefan BellerMay 3, 2016
  9. Jeff KingMay 3, 2016
  10. erik elfströmMay 3, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.