Re: [msysgit] curl: update to use the curl-7.30.0 release. (1a84e2f)

[Pat Thoyts]

Sebastian Schuberth <notifi...@github.com> writes:

>@patthoyts, please note that upstream curl 7.30 has a bug that causes cloning
>via https to crash on Windows. We've fixed this in mingwGitDevEnv by manually
>applying this patch until there's a new stable curl release with the fix
>included.

That bug occurs when curl is built with winssl. However in commit
3a9151c curl: switch back to openssl and avoid dynamic link to zlib
I reverted back to using openssl as in testing I found that the winssl
version had trouble when trying to clone from an internal network
machine when the proxy access was unavailable due to checking for
certificate revocation. As this kind of corporate setup is common I
decided to go back to openssl which doesn't suffer this issue.
By happy chance that means we avoid this particular bug.

--
Pat Thoyts http://www.patthoyts.tk/
PGP fingerprint 2C 6E 98 07 2C 59 C8 97 10 CE 11 E6 04 E0 B9 DD

[Sebastian Schuberth]

On Wed, May 22, 2013 at 12:06 AM, Pat Thoyts
<patt...@users.sourceforge.net> wrote:

>>@patthoyts, please note that upstream curl 7.30 has a bug that causes cloning
>>via https to crash on Windows. We've fixed this in mingwGitDevEnv by manually
>>applying this patch until there's a new stable curl release with the fix
>>included.
>
> That bug occurs when curl is built with winssl. However in commit
> 3a9151c curl: switch back to openssl and avoid dynamic link to zlib
> I reverted back to using openssl as in testing I found that the winssl
> version had trouble when trying to clone from an internal network
> machine when the proxy access was unavailable due to checking for
> certificate revocation. As this kind of corporate setup is common I
> decided to go back to openssl which doesn't suffer this issue.
> By happy chance that means we avoid this particular bug.

Thanks for the clarification. @Thomas, any objections that we also
change our flags according to

https://github.com/msysgit/msysgit/commit/3a9151cddd239aab12436ec42b042720c1340f33

In mingwGitDevEnv, we should also get rid of the "export" and just add
CFG on the make command line, and also replace "make" with "mgwmake".

@Pat: In that diff it looks like there are no more spaces between the
CFG options. Is that correct / on purpose?

--
Sebastian Schuberth

[Pat Thoyts]

Yes - if you look at the examples in the curl lib/Makefile.m32 they
have no spaces. I don't think it matters much as the options are
detected using $(findstring -optname,$(CFG),-optname) so its a string
match on the name with the hyphen. The spaces are just ignored I
think.

> --
> --
> *** Please reply-to-all at all times ***
> *** (do not pretend to know who is subscribed and who is not) ***
> *** Please avoid top-posting. ***
> The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
>
> You received this message because you are subscribed to the Google
> Groups "msysGit" group.
> To post to this group, send email to msy...@googlegroups.com
> To unsubscribe from this group, send email to
> msysgit+u...@googlegroups.com
> For more options, and view previous threads, visit this group at
> http://groups.google.com/group/msysgit?hl=en_US?hl=en
>
> ---
> You received this message because you are subscribed to the Google Groups "msysGit" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+u...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[Thomas Braun]

Am 22.05.2013 09:48, schrieb Sebastian Schuberth:
> On Wed, May 22, 2013 at 12:06 AM, Pat Thoyts
> <patt...@users.sourceforge.net> wrote:
>
>>> @patthoyts, please note that upstream curl 7.30 has a bug that causes cloning
>>> via https to crash on Windows. We've fixed this in mingwGitDevEnv by manually
>>> applying this patch until there's a new stable curl release with the fix
>>> included.
>>
>> That bug occurs when curl is built with winssl. However in commit
>> 3a9151c curl: switch back to openssl and avoid dynamic link to zlib
>> I reverted back to using openssl as in testing I found that the winssl
>> version had trouble when trying to clone from an internal network
>> machine when the proxy access was unavailable due to checking for
>> certificate revocation. As this kind of corporate setup is common I
>> decided to go back to openssl which doesn't suffer this issue.
>> By happy chance that means we avoid this particular bug.
>
> Thanks for the clarification. @Thomas, any objections that we also
> change our flags according to
>
> https://github.com/msysgit/msysgit/commit/3a9151cddd239aab12436ec42b042720c1340f33

Well actually I would like to see the windows schannel code being used,
as then we don't have to provide (and update) the cur-ca-bundle.crt file.

But if winssl does not work properly, that's of course a reason to
switch to openssl. Does curl upstream know about the problem? If not,
how can I reproduce it?

> In mingwGitDevEnv, we should also get rid of the "export" and just add
> CFG on the make command line, and also replace "make" with "mgwmake".

See
https://github.com/t-b/mingwGitDevEnv/commit/c6d1ca412ca51ad1e7da5b785cadb2e7b51376ab

I added the ca-cert bundle to libcurl-*dll.tar.lzma. Or should I create
a separate package which curl-*bin.tar.lzma and libcurl-*dll.tar.lzma
depend on?

[Sebastian Schuberth]

On Tue, Jun 11, 2013 at 12:51 AM, Thomas Braun
<thomas...@virtuell-zuhause.de> wrote:

> Well actually I would like to see the windows schannel code being used, as
> then we don't have to provide (and update) the cur-ca-bundle.crt file.

Yeah, that would have been indeed nice.

> But if winssl does not work properly, that's of course a reason to switch to
> openssl. Does curl upstream know about the problem? If not, how can I
> reproduce it?

@Pat, can you answer this?

> See
> https://github.com/t-b/mingwGitDevEnv/commit/c6d1ca412ca51ad1e7da5b785cadb2e7b51376ab
>
> I added the ca-cert bundle to libcurl-*dll.tar.lzma. Or should I create a
> separate package which curl-*bin.tar.lzma and libcurl-*dll.tar.lzma depend
> on?

I think shipping *.crt as part of libcurl-*dll.tar.lzma is fine. I've
adopted your commit to:

https://github.com/sschuberth/mingwGitDevEnv-packages/commit/62242a932e1e146bcb66f9cff4358b5dacf27eb1

But with libcurl built like this I get with Git:

$ git clone https://github.com/msysgit/git.git test
Cloning into 'test'...
fatal: unable to access 'https://github.com/msysgit/git.git/': SSL
certificate problem: unable to get local issuer certificate

Maybe the local certificate is just installed into the wrong place. I
don't have the time to check this in more detail right now. @Thomas,
I'd be glad if you could help debugging this and sending a fixup for
62242a932e1e146bcb66f9cff4358b5dacf27eb1.

Thanks!

--
Sebastian Schuberth

[Thomas Braun]

Am 23.06.2013 10:04, schrieb Sebastian Schuberth:
>
> I think shipping *.crt as part of libcurl-*dll.tar.lzma is fine. I've
> adopted your commit to:
>
> https://github.com/sschuberth/mingwGitDevEnv-packages/commit/62242a932e1e146bcb66f9cff4358b5dacf27eb1
>
> But with libcurl built like this I get with Git:
>
> $ git clone https://github.com/msysgit/git.git test
> Cloning into 'test'...
> fatal: unable to access 'https://github.com/msysgit/git.git/': SSL
> certificate problem: unable to get local issuer certificate
>
> Maybe the local certificate is just installed into the wrong place. I
> don't have the time to check this in more detail right now. @Thomas,
> I'd be glad if you could help debugging this and sending a fixup for
> 62242a932e1e146bcb66f9cff4358b5dacf27eb1.

We have to tell git where the curl-ca-bundle.crt lies, as msysgit does
it. But that was not straightforward possible as sysconfdir where git
should look for this file, is difficult to set due to the way mgwport works.

All changes are in
https://github.com/t-b/mingwGitDevEnv-packages/commits/curl-to-openssl-v2 including
a curl version update. I have set /mingw/etc as the place for gitconfig,
and not /etc.

Thomas

PS: I had some problems using git from the mgwport package as it could
not find the git-merge command called by "git pull". Is the removal of
the builtins with git-do-not-install-built-ins.mingw32.patch correct?
Why are they then shipped with msysgit?

[Sebastian Schuberth]

On Tue, Jun 25, 2013 at 12:56 AM, Thomas Braun
<thomas...@virtuell-zuhause.de> wrote:

>> Maybe the local certificate is just installed into the wrong place. I
>> don't have the time to check this in more detail right now. @Thomas,
>> I'd be glad if you could help debugging this and sending a fixup for
>> 62242a932e1e146bcb66f9cff4358b5dacf27eb1.
>
> We have to tell git where the curl-ca-bundle.crt lies, as msysgit does
> it. But that was not straightforward possible as sysconfdir where git
> should look for this file, is difficult to set due to the way mgwport works.
>
> All changes are in
> https://github.com/t-b/mingwGitDevEnv-packages/commits/curl-to-openssl-v2 including
> a curl version update. I have set /mingw/etc as the place for gitconfig,
> and not /etc.

Uh, that looks indeed way more complicated than I anticipated. Maybe
we can discuss this when we meet.

> PS: I had some problems using git from the mgwport package as it could
> not find the git-merge command called by "git pull". Is the removal of
> the builtins with git-do-not-install-built-ins.mingw32.patch correct?

Well, define "correct". Removing the built-ins is not supported /
encouraged by upstream (which also is why it's not straight forward
how to do it). But given the issues we had with the built-ins on
Windows (mainly due to the way links work on NTFS) I decided to simply
not include them. Who types "git-add" instead of "git add" anyway?
It's just that some of the scripts that still call the "dashed
version" need to be adjusted.

> Why are they then shipped with msysgit?

Because no one adjusted the scripts that still use the "dashed version" yet.

--
Sebastian Schuberth

[Thomas Braun]

Ahh I see.

So if I start to fix the scripts to use "git $foo" instead of "git-$foo"
should I create a patch for mingwGitDevEnv's git package or do it
directly in msysgit's git?

[Sebastian Schuberth]

On Thu, Jun 27, 2013 at 12:51 PM, Thomas Braun
<thomas...@virtuell-zuhause.de> wrote:

>>> Why are they then shipped with msysgit?
>>
>> Because no one adjusted the scripts that still use the "dashed version" yet.
>
> Ahh I see.
>
> So if I start to fix the scripts to use "git $foo" instead of "git-$foo"
> should I create a patch for mingwGitDevEnv's git package or do it
> directly in msysgit's git?

In the best case, neither, but send a patch to upstream Git instead.
Although upstream Git does not encourage dropping the built-ins,
scripts like git-pull.sh already use a mixture of calling dashed and
non-dashed versions of built-ins. For example, it calls "git
diff-index" and "git config", but also "git-merge". All Git commands
that are available as built-in should be called in their non-dashed
form and commands that still are scripts should be called in their
dashed form. The latter is a performance improvement: From an existing
script, it's faster to call another script than calling e.g. "git
pull" which would call the git binary in order to spawn another shell
instance that calls "git-pull".

--
Sebastian Schuberth