Homebrew and Git

Homebrew and Git

[Jonas Thiel]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

Dear Sir or Madame,
 
A while ago I have described my problem with Homebrew at the following GitHub channel (https://github.com/Homebrew/homebrew-core/issues/2970). In the meanwhile, I believe that I my problem with Homebrew is based on an issues with my Git. I have found the attached Git Crash reports on my Mac and because I am not familiar with reading/analysing Crash Reports, it would be great if someone could give me some feedback on it.
 
If you have any question, please do not hesitate to contact me.
 
Thanks a lot in advance and best regards,
Jonas

[-- Attachment #2: Git.Crash.report.zip --]
[-- Type: application/zip, Size: 13686 bytes --]

Re: Homebrew and Git

[Heiko Voigt]

Hi,

On Sun, Sep 18, 2016 at 05:50:28PM +0200, Jonas Thiel wrote:
> A while ago I have described my problem with Homebrew at the following
> GitHub channel
> (https://github.com/Homebrew/homebrew-core/issues/2970). In the
> meanwhile, I believe that I my problem with Homebrew is based on an
> issues with my Git. I have found the attached Git Crash reports on my
> Mac and because I am not familiar with reading/analysing Crash
> Reports, it would be great if someone could give me some feedback on
> it.
>  
> If you have any question, please do not hesitate to contact me.

From your crash reports I see that git is apparently crashing in a
strchr() call from within ident_default_email() which is a function that
tries to assemble a name and email to put into your commits.

Can you post us the output of

	hostname -f

and

	whoami

?

Since it seems you are using an Apple git can you also give us the
output of

	git version

Since it seems that Apple is compiling its own git (and not publishing
the changes they made conveniently via git). Have you tried
installing a vanilla git via homebrew and seeing whether that also
produces the issue?

In your bugreport you are talking about modifications you do to your
system after which the issue occurred. I would suggest to exactly find
out which step lead to git crashing (if it actually is the issue). First
to identify an issue we need something that is reproduceable.

Cheers Heiko

Re: Re: Homebrew and Git

[Heiko Voigt]

On Tue, Sep 20, 2016 at 01:02:28PM +0200, Heiko Voigt wrote:
> Hi,
> 
> On Sun, Sep 18, 2016 at 05:50:28PM +0200, Jonas Thiel wrote:
> > A while ago I have described my problem with Homebrew at the following
> > GitHub channel
> > (https://github.com/Homebrew/homebrew-core/issues/2970). In the
> > meanwhile, I believe that I my problem with Homebrew is based on an
> > issues with my Git. I have found the attached Git Crash reports on my
> > Mac and because I am not familiar with reading/analysing Crash
> > Reports, it would be great if someone could give me some feedback on
> > it.
> >  
> > If you have any question, please do not hesitate to contact me.
> 
> From your crash reports I see that git is apparently crashing in a
> strchr() call from within ident_default_email() which is a function that
> tries to assemble a name and email to put into your commits.

BTW, here is the callstack inlined from the crashreport:

bsystem_platform.dylib      	0x00007fff840db41c _platform_strchr$VARIANT$Haswell + 28
1   git                           	0x000000010ba1d3f4 ident_default_email + 801
2   git                           	0x000000010ba1d68f fmt_ident + 66
3   git                           	0x000000010ba4b495 files_log_ref_write + 175
4   git                           	0x000000010ba4b0a6 commit_ref_update + 106
5   git                           	0x000000010ba4c3a8 ref_transaction_commit + 468
6   git                           	0x000000010b994dd8 s_update_ref + 271
7   git                           	0x000000010b994556 fetch_refs + 1969
8   git                           	0x000000010b9935f2 fetch_one + 1913
9   git                           	0x000000010b992bc4 cmd_fetch + 549
10  git                           	0x000000010b9666c4 handle_builtin + 478
11  git                           	0x000000010b96602f main + 376
12  libdyld.dylib                 	0x00007fff834ef5ad start + 1

Maybe someone else has an idea what might be causing this...

Cheers Heiko

Aw: Re: Homebrew and Git

[Jonas Thiel]

Hi Heiko,

thanks for your reply on my issue. 

Here are the following outputs you asked for:

git version
git version 2.7.4 (Apple Git-66)

hostname -f
0x6A6E73

whoami
jns

I have tried the Homebrew version of git, but no change in case of the issue.

I just customized my system with the Mathias Bynens dotfiles (https://github.com/mathiasbynens/dotfiles) as well as with drduhs OS X Security Guide (https://github.com/drduh/OS-X-Security-and-Privacy-Guide). Because I reinstall my Mac on a regularly basis, wherefore I can say that one year ago these tweaks have not caused any issue.

Thanks a lot. I really appreciate your help.

Cheers,
Jonas

> Gesendet: Dienstag, 20. September 2016 um 13:02 Uhr
> Von: "Heiko Voigt" <hvoigt@hvoigt.net>
> An: "Jonas Thiel" <jonas.lierschied@gmx.de>
> Cc: git@vger.kernel.org
> Betreff: Re: Homebrew and Git
>
> Hi,
> 
> On Sun, Sep 18, 2016 at 05:50:28PM +0200, Jonas Thiel wrote:
> > A while ago I have described my problem with Homebrew at the following
> > GitHub channel
> > (https://github.com/Homebrew/homebrew-core/issues/2970). In the
> > meanwhile, I believe that I my problem with Homebrew is based on an
> > issues with my Git. I have found the attached Git Crash reports on my
> > Mac and because I am not familiar with reading/analysing Crash
> > Reports, it would be great if someone could give me some feedback on
> > it.
> >  
> > If you have any question, please do not hesitate to contact me.
> 
> From your crash reports I see that git is apparently crashing in a
> strchr() call from within ident_default_email() which is a function that
> tries to assemble a name and email to put into your commits.
> 
> Can you post us the output of
> 
> 	hostname -f
> 
> and
> 
> 	whoami
> 
> ?
> 
> Since it seems you are using an Apple git can you also give us the
> output of
> 
> 	git version
> 
> Since it seems that Apple is compiling its own git (and not publishing
> the changes they made conveniently via git). Have you tried
> installing a vanilla git via homebrew and seeing whether that also
> produces the issue?
> 
> In your bugreport you are talking about modifications you do to your
> system after which the issue occurred. I would suggest to exactly find
> out which step lead to git crashing (if it actually is the issue). First
> to identify an issue we need something that is reproduceable.
> 
> Cheers Heiko
> 

Re: Re: Homebrew and Git

[John Keeping]

On Tue, Sep 20, 2016 at 01:07:00PM +0200, Heiko Voigt wrote:
> On Tue, Sep 20, 2016 at 01:02:28PM +0200, Heiko Voigt wrote:
> > Hi,
> > 
> > On Sun, Sep 18, 2016 at 05:50:28PM +0200, Jonas Thiel wrote:
> > > A while ago I have described my problem with Homebrew at the following
> > > GitHub channel
> > > (https://github.com/Homebrew/homebrew-core/issues/2970). In the
> > > meanwhile, I believe that I my problem with Homebrew is based on an
> > > issues with my Git. I have found the attached Git Crash reports on my
> > > Mac and because I am not familiar with reading/analysing Crash
> > > Reports, it would be great if someone could give me some feedback on
> > > it.
> > >  
> > > If you have any question, please do not hesitate to contact me.
> > 
> > From your crash reports I see that git is apparently crashing in a
> > strchr() call from within ident_default_email() which is a function that
> > tries to assemble a name and email to put into your commits.
> 
> BTW, here is the callstack inlined from the crashreport:
> 
> bsystem_platform.dylib      	0x00007fff840db41c _platform_strchr$VARIANT$Haswell + 28
> 1   git                           	0x000000010ba1d3f4 ident_default_email + 801
> 2   git                           	0x000000010ba1d68f fmt_ident + 66
> 3   git                           	0x000000010ba4b495 files_log_ref_write + 175
> 4   git                           	0x000000010ba4b0a6 commit_ref_update + 106
> 5   git                           	0x000000010ba4c3a8 ref_transaction_commit + 468
> 6   git                           	0x000000010b994dd8 s_update_ref + 271
> 7   git                           	0x000000010b994556 fetch_refs + 1969
> 8   git                           	0x000000010b9935f2 fetch_one + 1913
> 9   git                           	0x000000010b992bc4 cmd_fetch + 549
> 10  git                           	0x000000010b9666c4 handle_builtin + 478
> 11  git                           	0x000000010b96602f main + 376
> 12  libdyld.dylib                 	0x00007fff834ef5ad start + 1
> 
> Maybe someone else has an idea what might be causing this...

The only strchr I can see that could be called here is in
canonical_name(), where it's called with addrinfo::ai_canonname.

Searching for OS X and ai_canonname, leads me straight back to this
list, although 7 years ago!  I think ident.c needs a fix similar to
commit 3e8a00a (daemon.c: fix segfault on OS X, 2009-04-27); from the
commit message there:

	On OS X (and maybe other unices), getaddrinfo(3) returns NULL
	in the ai_canonname field if it's called with an IP address for
	the hostname.

Re: Re: Homebrew and Git

[Jeff King]

On Tue, Sep 20, 2016 at 08:15:55PM +0100, John Keeping wrote:

> > BTW, here is the callstack inlined from the crashreport:
> > 
> > bsystem_platform.dylib      	0x00007fff840db41c _platform_strchr$VARIANT$Haswell + 28
> > 1   git                           	0x000000010ba1d3f4 ident_default_email + 801
> > 2   git                           	0x000000010ba1d68f fmt_ident + 66
> > 3   git                           	0x000000010ba4b495 files_log_ref_write + 175
> > 4   git                           	0x000000010ba4b0a6 commit_ref_update + 106
> > 5   git                           	0x000000010ba4c3a8 ref_transaction_commit + 468
> > 6   git                           	0x000000010b994dd8 s_update_ref + 271
> > 7   git                           	0x000000010b994556 fetch_refs + 1969
> > 8   git                           	0x000000010b9935f2 fetch_one + 1913
> > 9   git                           	0x000000010b992bc4 cmd_fetch + 549
> > 10  git                           	0x000000010b9666c4 handle_builtin + 478
> > 11  git                           	0x000000010b96602f main + 376
> > 12  libdyld.dylib                 	0x00007fff834ef5ad start + 1
> > 
> > Maybe someone else has an idea what might be causing this...
> 
> The only strchr I can see that could be called here is in
> canonical_name(), where it's called with addrinfo::ai_canonname.

There's one in add_domainname(), too, but it can never be NULL (we could
walk off the end of the buffer, but only if gethostname() lies to us
about its result code, which seems unlikely). So I agree it's probably
the call in canonical_name().

> Searching for OS X and ai_canonname, leads me straight back to this
> list, although 7 years ago!  I think ident.c needs a fix similar to
> commit 3e8a00a (daemon.c: fix segfault on OS X, 2009-04-27); from the
> commit message there:
> 
> 	On OS X (and maybe other unices), getaddrinfo(3) returns NULL
> 	in the ai_canonname field if it's called with an IP address for
> 	the hostname.

Interesting. We are already prepared for failure from getaddrinfo()
here, so probably:

diff --git a/ident.c b/ident.c
index e20a772..d17b5bd 100644
--- a/ident.c
+++ b/ident.c
@@ -101,7 +101,7 @@ static int canonical_name(const char *host, struct strbuf *out)
 	memset (&hints, '\0', sizeof (hints));
 	hints.ai_flags = AI_CANONNAME;
 	if (!getaddrinfo(host, NULL, &hints, &ai)) {
-		if (ai && strchr(ai->ai_canonname, '.')) {
+		if (ai && ai->ai_canonname && strchr(ai->ai_canonname, '.')) {
 			strbuf_addstr(out, ai->ai_canonname);
 			status = 0;
 		}

would be sufficient. Jonas, can you see if that patch helps?

-Peff

Aw: Re: Re: Homebrew and Git

[Jonas Thiel]

Sorry for my late reply. Thanks for your support -- I really appreciate that. 

@Jeff: Unfortunately, I do not know how to implement the patch you provided. Can you explain how to do that? 

Thanks and best regards,
Jonas

> Gesendet: Mittwoch, 21. September 2016 um 10:48 Uhr
> Von: "Jeff King" <peff@peff.net>
> An: "John Keeping" <john@keeping.me.uk>
> Cc: "Heiko Voigt" <hvoigt@hvoigt.net>, "Jonas Thiel" <jonas.lierschied@gmx.de>, git@vger.kernel.org
> Betreff: Re: Re: Homebrew and Git
>
> On Tue, Sep 20, 2016 at 08:15:55PM +0100, John Keeping wrote:
> 
> > > BTW, here is the callstack inlined from the crashreport:
> > > 
> > > bsystem_platform.dylib      	0x00007fff840db41c _platform_strchr$VARIANT$Haswell + 28
> > > 1   git                           	0x000000010ba1d3f4 ident_default_email + 801
> > > 2   git                           	0x000000010ba1d68f fmt_ident + 66
> > > 3   git                           	0x000000010ba4b495 files_log_ref_write + 175
> > > 4   git                           	0x000000010ba4b0a6 commit_ref_update + 106
> > > 5   git                           	0x000000010ba4c3a8 ref_transaction_commit + 468
> > > 6   git                           	0x000000010b994dd8 s_update_ref + 271
> > > 7   git                           	0x000000010b994556 fetch_refs + 1969
> > > 8   git                           	0x000000010b9935f2 fetch_one + 1913
> > > 9   git                           	0x000000010b992bc4 cmd_fetch + 549
> > > 10  git                           	0x000000010b9666c4 handle_builtin + 478
> > > 11  git                           	0x000000010b96602f main + 376
> > > 12  libdyld.dylib                 	0x00007fff834ef5ad start + 1
> > > 
> > > Maybe someone else has an idea what might be causing this...
> > 
> > The only strchr I can see that could be called here is in
> > canonical_name(), where it's called with addrinfo::ai_canonname.
> 
> There's one in add_domainname(), too, but it can never be NULL (we could
> walk off the end of the buffer, but only if gethostname() lies to us
> about its result code, which seems unlikely). So I agree it's probably
> the call in canonical_name().
> 
> > Searching for OS X and ai_canonname, leads me straight back to this
> > list, although 7 years ago!  I think ident.c needs a fix similar to
> > commit 3e8a00a (daemon.c: fix segfault on OS X, 2009-04-27); from the
> > commit message there:
> > 
> > 	On OS X (and maybe other unices), getaddrinfo(3) returns NULL
> > 	in the ai_canonname field if it's called with an IP address for
> > 	the hostname.
> 
> Interesting. We are already prepared for failure from getaddrinfo()
> here, so probably:
> 
> diff --git a/ident.c b/ident.c
> index e20a772..d17b5bd 100644
> --- a/ident.c
> +++ b/ident.c
> @@ -101,7 +101,7 @@ static int canonical_name(const char *host, struct strbuf *out)
>  	memset (&hints, '\0', sizeof (hints));
>  	hints.ai_flags = AI_CANONNAME;
>  	if (!getaddrinfo(host, NULL, &hints, &ai)) {
> -		if (ai && strchr(ai->ai_canonname, '.')) {
> +		if (ai && ai->ai_canonname && strchr(ai->ai_canonname, '.')) {
>  			strbuf_addstr(out, ai->ai_canonname);
>  			status = 0;
>  		}
> 
> would be sufficient. Jonas, can you see if that patch helps?
> 
> -Peff
> 

Re: Re: Re: Homebrew and Git

[Stefan Beller]

On Thu, Sep 22, 2016 at 2:23 AM, Jonas Thiel <jonas.lierschied@gmx.de> wrote:
> Sorry for my late reply. Thanks for your support -- I really appreciate that.
>
> @Jeff: Unfortunately, I do not know how to implement the patch you provided. Can you explain how to do that?

I think this should do:


    git clone https://github.com/git/git
    cd git
    # get the email
    wget http://public-inbox.org/git/20160921084841.phq7cfbagi5k7ku4@sigill.intra.peff.net/raw
    # apply patch:
    git am raw
    make
    make install

>
> Thanks and best regards,
> Jonas
>
>> Gesendet: Mittwoch, 21. September 2016 um 10:48 Uhr
>> Von: "Jeff King" <peff@peff.net>
>> An: "John Keeping" <john@keeping.me.uk>
>> Cc: "Heiko Voigt" <hvoigt@hvoigt.net>, "Jonas Thiel" <jonas.lierschied@gmx.de>, git@vger.kernel.org
>> Betreff: Re: Re: Homebrew and Git
>>
>> On Tue, Sep 20, 2016 at 08:15:55PM +0100, John Keeping wrote:
>>
>> > > BTW, here is the callstack inlined from the crashreport:
>> > >
>> > > bsystem_platform.dylib            0x00007fff840db41c _platform_strchr$VARIANT$Haswell + 28
>> > > 1   git                                   0x000000010ba1d3f4 ident_default_email + 801
>> > > 2   git                                   0x000000010ba1d68f fmt_ident + 66
>> > > 3   git                                   0x000000010ba4b495 files_log_ref_write + 175
>> > > 4   git                                   0x000000010ba4b0a6 commit_ref_update + 106
>> > > 5   git                                   0x000000010ba4c3a8 ref_transaction_commit + 468
>> > > 6   git                                   0x000000010b994dd8 s_update_ref + 271
>> > > 7   git                                   0x000000010b994556 fetch_refs + 1969
>> > > 8   git                                   0x000000010b9935f2 fetch_one + 1913
>> > > 9   git                                   0x000000010b992bc4 cmd_fetch + 549
>> > > 10  git                                   0x000000010b9666c4 handle_builtin + 478
>> > > 11  git                                   0x000000010b96602f main + 376
>> > > 12  libdyld.dylib                         0x00007fff834ef5ad start + 1
>> > >
>> > > Maybe someone else has an idea what might be causing this...
>> >
>> > The only strchr I can see that could be called here is in
>> > canonical_name(), where it's called with addrinfo::ai_canonname.
>>
>> There's one in add_domainname(), too, but it can never be NULL (we could
>> walk off the end of the buffer, but only if gethostname() lies to us
>> about its result code, which seems unlikely). So I agree it's probably
>> the call in canonical_name().
>>
>> > Searching for OS X and ai_canonname, leads me straight back to this
>> > list, although 7 years ago!  I think ident.c needs a fix similar to
>> > commit 3e8a00a (daemon.c: fix segfault on OS X, 2009-04-27); from the
>> > commit message there:
>> >
>> >     On OS X (and maybe other unices), getaddrinfo(3) returns NULL
>> >     in the ai_canonname field if it's called with an IP address for
>> >     the hostname.
>>
>> Interesting. We are already prepared for failure from getaddrinfo()
>> here, so probably:
>>
>> diff --git a/ident.c b/ident.c
>> index e20a772..d17b5bd 100644
>> --- a/ident.c
>> +++ b/ident.c
>> @@ -101,7 +101,7 @@ static int canonical_name(const char *host, struct strbuf *out)
>>       memset (&hints, '\0', sizeof (hints));
>>       hints.ai_flags = AI_CANONNAME;
>>       if (!getaddrinfo(host, NULL, &hints, &ai)) {
>> -             if (ai && strchr(ai->ai_canonname, '.')) {
>> +             if (ai && ai->ai_canonname && strchr(ai->ai_canonname, '.')) {
>>                       strbuf_addstr(out, ai->ai_canonname);
>>                       status = 0;
>>               }
>>
>> would be sufficient. Jonas, can you see if that patch helps?
>>
>> -Peff
>>

Re: Re: Re: Homebrew and Git

[Jeff King]

On Thu, Sep 22, 2016 at 08:57:45AM -0700, Stefan Beller wrote:

> On Thu, Sep 22, 2016 at 2:23 AM, Jonas Thiel <jonas.lierschied@gmx.de> wrote:
> > Sorry for my late reply. Thanks for your support -- I really appreciate that.
> >
> > @Jeff: Unfortunately, I do not know how to implement the patch you provided. Can you explain how to do that?
> 
> I think this should do:
> 
>     git clone https://github.com/git/git
>     cd git
>     # get the email
>     wget http://public-inbox.org/git/20160921084841.phq7cfbagi5k7ku4@sigill.intra.peff.net/raw
>     # apply patch:
>     git am raw

I publish my topic branches via git, too, so it may be easier to replace
these steps with:

  git clone -b jk/ident-canonname https://github.com/peff/git
  cd git

>     make
>     make install

After installing, you probably want to do:

  PATH=/path/to/git/bin-wrappers:$PATH

to run the new git (and make sure that any git sub-commands that are run
come from the built version, not the rest of the system).

I have access to an OS X system, but if I understand the bug correctly,
reproducing it may involve re-setting the system hostname, which is not
something I'll be able to do. But I'll give it a shot.

-Peff

[PATCH] ident: handle NULL ai_canonname

[Jeff King]

On Fri, Sep 23, 2016 at 12:07:30AM -0400, Jeff King wrote:

> I have access to an OS X system, but if I understand the bug correctly,
> reproducing it may involve re-setting the system hostname, which is not
> something I'll be able to do. But I'll give it a shot.

Actually, it turned out to be pretty simple to reproduce (after reading
3e8a00a that John found, anyway; hooray for detailed commit messages). 
We just have to fake the output of gethostname(), but that is easily
done since we can modify git's source. :)

So I was able to reproduce the bug, and indeed, the patch I posted fixes
it. Here it is with a commit message.

Jonas, I'd be curious to know what the output of "hostname" is on your
system.

-- >8 --
Subject: [PATCH] ident: handle NULL ai_canonname

We call getaddrinfo() to try to convert a short hostname
into a fully-qualified one (to use it as an email domain).
If there isn't a canonical name, getaddrinfo() will
generally return either a NULL addrinfo list, or one in
which ai->ai_canonname is a copy of the original name.

However, if the result of gethostname() looks like an IP
address, then getaddrinfo() behaves differently on some
systems. On OS X, it will return a "struct addrinfo" with a
NULL ai_canonname, and we segfault feeding it to strchr().

This is hard to test reliably because it involves not only a
system where we we have to fallback to gethostname() to come
up with an ident, but also where the hostname is a number
with no dots. But I was able to replicate the bug by faking
a hostname, like:

    diff --git a/ident.c b/ident.c
    index e20a772..b790d28 100644
    --- a/ident.c
    +++ b/ident.c
    @@ -128,6 +128,7 @@ static void add_domainname(struct strbuf *out, int *is_bogus)
                     *is_bogus = 1;
                     return;
             }
    +        xsnprintf(buf, sizeof(buf), "1");
             if (strchr(buf, '.'))
                     strbuf_addstr(out, buf);
             else if (canonical_name(buf, out) < 0) {

and running "git var GIT_AUTHOR_IDENT" on an OS X system.

Before this patch it segfaults, and after we correctly
complain of the bogus "user@1.(none)" address (though this
bogus address would be suitable for non-object uses like
writing reflogs).

Reported-by: Jonas Thiel <jonas.lierschied@gmx.de>
Diagnosed-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Jeff King <peff@peff.net>
---
 ident.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ident.c b/ident.c
index e20a772..d17b5bd 100644
--- a/ident.c
+++ b/ident.c
@@ -101,7 +101,7 @@ static int canonical_name(const char *host, struct strbuf *out)
 	memset (&hints, '\0', sizeof (hints));
 	hints.ai_flags = AI_CANONNAME;
 	if (!getaddrinfo(host, NULL, &hints, &ai)) {
-		if (ai && strchr(ai->ai_canonname, '.')) {
+		if (ai && ai->ai_canonname && strchr(ai->ai_canonname, '.')) {
 			strbuf_addstr(out, ai->ai_canonname);
 			status = 0;
 		}
-- 
2.10.0.482.gae5a597