unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [PATCH] gnu: r-curl: Respect CURL_CA_BUNDLE variable.
@ 2016-09-07 14:56 Ricardo Wurmus
  2016-09-13  5:45 ` Leo Famulari
  2016-09-13 21:53 ` Roel Janssen
  0 siblings, 2 replies; 8+ messages in thread
From: Ricardo Wurmus @ 2016-09-07 14:56 UTC (permalink / raw)
  To: guix-devel

* gnu/packages/web.scm (r-curl)[arguments]: Add phase
"allow-CURL_CA_BUNDLE".
---
 gnu/packages/web.scm | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
index 87bc3e2..321a250 100644
--- a/gnu/packages/web.scm
+++ b/gnu/packages/web.scm
@@ -3168,6 +3168,19 @@ applications.")
                (base32
                 "1p24bcaf1wbfdi1r9ibyyp0l0zp4kzs4g3srv8vikz93hycm1qa6"))))
     (build-system r-build-system)
+    (arguments
+     `(#:phases
+       (modify-phases %standard-phases
+         ;; The environment variable CURL_CA_BUNDLE is only respected when
+         ;; running Windows, so we disable the platform checks.
+         (add-after 'unpack 'allow-CURL_CA_BUNDLE
+           (lambda _
+             (substitute* "R/onload.R"
+               (("if \\(!grepl\\(\"mingw\".*")
+                "if (FALSE)\n"))
+             (substitute* "src/handle.c"
+               (("#ifdef _WIN32") "#if 1"))
+             #t)))))
     (inputs
      `(("libcurl" ,curl)))
     (home-page "https://github.com/jeroenooms/curl")
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] gnu: r-curl: Respect CURL_CA_BUNDLE variable.
  2016-09-07 14:56 [PATCH] gnu: r-curl: Respect CURL_CA_BUNDLE variable Ricardo Wurmus
@ 2016-09-13  5:45 ` Leo Famulari
  2016-09-13 21:53 ` Roel Janssen
  1 sibling, 0 replies; 8+ messages in thread
From: Leo Famulari @ 2016-09-13  5:45 UTC (permalink / raw)
  To: Ricardo Wurmus; +Cc: guix-devel

On Wed, Sep 07, 2016 at 04:56:59PM +0200, Ricardo Wurmus wrote:
> * gnu/packages/web.scm (r-curl)[arguments]: Add phase
> "allow-CURL_CA_BUNDLE".
> ---
>  gnu/packages/web.scm | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
> index 87bc3e2..321a250 100644
> --- a/gnu/packages/web.scm
> +++ b/gnu/packages/web.scm
> @@ -3168,6 +3168,19 @@ applications.")
>                 (base32
>                  "1p24bcaf1wbfdi1r9ibyyp0l0zp4kzs4g3srv8vikz93hycm1qa6"))))
>      (build-system r-build-system)
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         ;; The environment variable CURL_CA_BUNDLE is only respected when
> +         ;; running Windows, so we disable the platform checks.

It seems like a weird restriction. Could you see what the upstream
maintainers think about making this change?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] gnu: r-curl: Respect CURL_CA_BUNDLE variable.
  2016-09-07 14:56 [PATCH] gnu: r-curl: Respect CURL_CA_BUNDLE variable Ricardo Wurmus
  2016-09-13  5:45 ` Leo Famulari
@ 2016-09-13 21:53 ` Roel Janssen
  2016-09-14  1:17   ` Leo Famulari
  1 sibling, 1 reply; 8+ messages in thread
From: Roel Janssen @ 2016-09-13 21:53 UTC (permalink / raw)
  To: Ricardo Wurmus; +Cc: guix-devel


Ricardo Wurmus writes:

> * gnu/packages/web.scm (r-curl)[arguments]: Add phase
> "allow-CURL_CA_BUNDLE".
> ---
>  gnu/packages/web.scm | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
> index 87bc3e2..321a250 100644
> --- a/gnu/packages/web.scm
> +++ b/gnu/packages/web.scm
> @@ -3168,6 +3168,19 @@ applications.")
>                 (base32
>                  "1p24bcaf1wbfdi1r9ibyyp0l0zp4kzs4g3srv8vikz93hycm1qa6"))))
>      (build-system r-build-system)
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         ;; The environment variable CURL_CA_BUNDLE is only respected when
> +         ;; running Windows, so we disable the platform checks.
> +         (add-after 'unpack 'allow-CURL_CA_BUNDLE
> +           (lambda _
> +             (substitute* "R/onload.R"
> +               (("if \\(!grepl\\(\"mingw\".*")
> +                "if (FALSE)\n"))
> +             (substitute* "src/handle.c"
> +               (("#ifdef _WIN32") "#if 1"))
> +             #t)))))
>      (inputs
>       `(("libcurl" ,curl)))
>      (home-page "https://github.com/jeroenooms/curl")

This patch was essential to me being able to interact with HTTPS urls in
R.  As far as I understand, by default, R only looks for CURL_CA_BUNDLE
on Windows, but with this patch it looks for CURL_CA_BUNDLE on GNU/Linux
as well.  Is this correct?

I can confirm it works for me, so I'd like to see this patch pushed.

Kind regards,
Roel Janssen

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] gnu: r-curl: Respect CURL_CA_BUNDLE variable.
  2016-09-13 21:53 ` Roel Janssen
@ 2016-09-14  1:17   ` Leo Famulari
  2016-09-21 19:24     ` Ricardo Wurmus
  0 siblings, 1 reply; 8+ messages in thread
From: Leo Famulari @ 2016-09-14  1:17 UTC (permalink / raw)
  To: Roel Janssen; +Cc: guix-devel

On Tue, Sep 13, 2016 at 11:53:33PM +0200, Roel Janssen wrote:
> This patch was essential to me being able to interact with HTTPS urls in
> R.  As far as I understand, by default, R only looks for CURL_CA_BUNDLE
> on Windows, but with this patch it looks for CURL_CA_BUNDLE on GNU/Linux
> as well.  Is this correct?
> 
> I can confirm it works for me, so I'd like to see this patch pushed.

It's good to hear that it works, but I still think we should run it by
the upstream maintainers. We are activating C code that they
specifically decided not to use on GNU / Linux. Why did they do that?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] gnu: r-curl: Respect CURL_CA_BUNDLE variable.
  2016-09-14  1:17   ` Leo Famulari
@ 2016-09-21 19:24     ` Ricardo Wurmus
  2016-09-23  5:26       ` Leo Famulari
  0 siblings, 1 reply; 8+ messages in thread
From: Ricardo Wurmus @ 2016-09-21 19:24 UTC (permalink / raw)
  To: Leo Famulari; +Cc: guix-devel


Leo Famulari <leo@famulari.name> writes:

> On Tue, Sep 13, 2016 at 11:53:33PM +0200, Roel Janssen wrote:
>> This patch was essential to me being able to interact with HTTPS urls in
>> R.  As far as I understand, by default, R only looks for CURL_CA_BUNDLE
>> on Windows, but with this patch it looks for CURL_CA_BUNDLE on GNU/Linux
>> as well.  Is this correct?
>> 
>> I can confirm it works for me, so I'd like to see this patch pushed.
>
> It's good to hear that it works, but I still think we should run it by
> the upstream maintainers. We are activating C code that they
> specifically decided not to use on GNU / Linux. Why did they do that?

The comments in the code indicate that on Windows they try to load the
certs bundle that comes with R for Windows, i.e. in the R HOME’s “etc”
directory.  There is no such file on GNU, so no special handling is
needed.

On GNU this is taken care of by libcurl.  It comes with a default path
to the certs bundle, which can be overridden with configure flags
(“--with-ca-bundle” or “--with-ca-path”).  In our Guix package we don’t
do this (yet?), so by default SSL cert validation is broken.

libcurl does not respect CURL_CA_BUNDLE; it assumes that the application
will override the CA bundle path if it needs a special path, otherwise
it assumes that the default path is fine (using Guix this is not the
case).

The maintainers of the R curl package made the special case for Windows
because it is not needed on GNU systems following the FHS.  The best fix
here would be to patch libcurl such that it checks the CURL_CA_BUNDLE
environment variable invariably, just like the curl command line tool
does.  Until this is done I think we should path packages such as
r-curl to make them usable.  Once we have agreed on a fix to libcurl we
can remove all patches to individual packages using libcurl.

~~ Ricardo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] gnu: r-curl: Respect CURL_CA_BUNDLE variable.
  2016-09-21 19:24     ` Ricardo Wurmus
@ 2016-09-23  5:26       ` Leo Famulari
  2016-09-24  7:46         ` Ricardo Wurmus
  0 siblings, 1 reply; 8+ messages in thread
From: Leo Famulari @ 2016-09-23  5:26 UTC (permalink / raw)
  To: Ricardo Wurmus; +Cc: guix-devel

On Wed, Sep 21, 2016 at 09:24:10PM +0200, Ricardo Wurmus wrote:
> Leo Famulari <leo@famulari.name> writes:
> 
> > On Tue, Sep 13, 2016 at 11:53:33PM +0200, Roel Janssen wrote:
> >> This patch was essential to me being able to interact with HTTPS urls in
> >> R.  As far as I understand, by default, R only looks for CURL_CA_BUNDLE
> >> on Windows, but with this patch it looks for CURL_CA_BUNDLE on GNU/Linux
> >> as well.  Is this correct?
> >> 
> >> I can confirm it works for me, so I'd like to see this patch pushed.
> >
> > It's good to hear that it works, but I still think we should run it by
> > the upstream maintainers. We are activating C code that they
> > specifically decided not to use on GNU / Linux. Why did they do that?
> 
> The comments in the code indicate that on Windows they try to load the
> certs bundle that comes with R for Windows, i.e. in the R HOME’s “etc”
> directory.  There is no such file on GNU, so no special handling is
> needed.
> 
> On GNU this is taken care of by libcurl.  It comes with a default path
> to the certs bundle, which can be overridden with configure flags
> (“--with-ca-bundle” or “--with-ca-path”).  In our Guix package we don’t
> do this (yet?), so by default SSL cert validation is broken.
> 
> libcurl does not respect CURL_CA_BUNDLE; it assumes that the application
> will override the CA bundle path if it needs a special path, otherwise
> it assumes that the default path is fine (using Guix this is not the
> case).
> 
> The maintainers of the R curl package made the special case for Windows
> because it is not needed on GNU systems following the FHS.  The best fix
> here would be to patch libcurl such that it checks the CURL_CA_BUNDLE
> environment variable invariably, just like the curl command line tool
> does.  Until this is done I think we should path packages such as
> r-curl to make them usable.  Once we have agreed on a fix to libcurl we
> can remove all patches to individual packages using libcurl.

That makes sense. Thank you for taking the time to explain.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] gnu: r-curl: Respect CURL_CA_BUNDLE variable.
  2016-09-23  5:26       ` Leo Famulari
@ 2016-09-24  7:46         ` Ricardo Wurmus
  2016-09-24 21:39           ` Roel Janssen
  0 siblings, 1 reply; 8+ messages in thread
From: Ricardo Wurmus @ 2016-09-24  7:46 UTC (permalink / raw)
  To: Leo Famulari; +Cc: guix-devel


Leo Famulari <leo@famulari.name> writes:

> On Wed, Sep 21, 2016 at 09:24:10PM +0200, Ricardo Wurmus wrote:
>> Leo Famulari <leo@famulari.name> writes:
>> 
>> > On Tue, Sep 13, 2016 at 11:53:33PM +0200, Roel Janssen wrote:
>> >> This patch was essential to me being able to interact with HTTPS urls in
>> >> R.  As far as I understand, by default, R only looks for CURL_CA_BUNDLE
>> >> on Windows, but with this patch it looks for CURL_CA_BUNDLE on GNU/Linux
>> >> as well.  Is this correct?
>> >> 
>> >> I can confirm it works for me, so I'd like to see this patch pushed.
>> >
>> > It's good to hear that it works, but I still think we should run it by
>> > the upstream maintainers. We are activating C code that they
>> > specifically decided not to use on GNU / Linux. Why did they do that?
>> 
>> The comments in the code indicate that on Windows they try to load the
>> certs bundle that comes with R for Windows, i.e. in the R HOME’s “etc”
>> directory.  There is no such file on GNU, so no special handling is
>> needed.
>> 
>> On GNU this is taken care of by libcurl.  It comes with a default path
>> to the certs bundle, which can be overridden with configure flags
>> (“--with-ca-bundle” or “--with-ca-path”).  In our Guix package we don’t
>> do this (yet?), so by default SSL cert validation is broken.
>> 
>> libcurl does not respect CURL_CA_BUNDLE; it assumes that the application
>> will override the CA bundle path if it needs a special path, otherwise
>> it assumes that the default path is fine (using Guix this is not the
>> case).
>> 
>> The maintainers of the R curl package made the special case for Windows
>> because it is not needed on GNU systems following the FHS.  The best fix
>> here would be to patch libcurl such that it checks the CURL_CA_BUNDLE
>> environment variable invariably, just like the curl command line tool
>> does.  Until this is done I think we should path packages such as
>> r-curl to make them usable.  Once we have agreed on a fix to libcurl we
>> can remove all patches to individual packages using libcurl.
>
> That makes sense. Thank you for taking the time to explain.

Sure!  Pushed to master as 8f309571d3847d4bca331061e881fa01d9badb77.

~~ Ricardo

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] gnu: r-curl: Respect CURL_CA_BUNDLE variable.
  2016-09-24  7:46         ` Ricardo Wurmus
@ 2016-09-24 21:39           ` Roel Janssen
  0 siblings, 0 replies; 8+ messages in thread
From: Roel Janssen @ 2016-09-24 21:39 UTC (permalink / raw)
  To: Ricardo Wurmus; +Cc: guix-devel


Ricardo Wurmus writes:

> Leo Famulari <leo@famulari.name> writes:
>
>> On Wed, Sep 21, 2016 at 09:24:10PM +0200, Ricardo Wurmus wrote:
>>> Leo Famulari <leo@famulari.name> writes:
>>> 
>>> > On Tue, Sep 13, 2016 at 11:53:33PM +0200, Roel Janssen wrote:
>>> >> This patch was essential to me being able to interact with HTTPS urls in
>>> >> R.  As far as I understand, by default, R only looks for CURL_CA_BUNDLE
>>> >> on Windows, but with this patch it looks for CURL_CA_BUNDLE on GNU/Linux
>>> >> as well.  Is this correct?
>>> >> 
>>> >> I can confirm it works for me, so I'd like to see this patch pushed.
>>> >
>>> > It's good to hear that it works, but I still think we should run it by
>>> > the upstream maintainers. We are activating C code that they
>>> > specifically decided not to use on GNU / Linux. Why did they do that?
>>> 
>>> The comments in the code indicate that on Windows they try to load the
>>> certs bundle that comes with R for Windows, i.e. in the R HOME’s “etc”
>>> directory.  There is no such file on GNU, so no special handling is
>>> needed.
>>> 
>>> On GNU this is taken care of by libcurl.  It comes with a default path
>>> to the certs bundle, which can be overridden with configure flags
>>> (“--with-ca-bundle” or “--with-ca-path”).  In our Guix package we don’t
>>> do this (yet?), so by default SSL cert validation is broken.
>>> 
>>> libcurl does not respect CURL_CA_BUNDLE; it assumes that the application
>>> will override the CA bundle path if it needs a special path, otherwise
>>> it assumes that the default path is fine (using Guix this is not the
>>> case).
>>> 
>>> The maintainers of the R curl package made the special case for Windows
>>> because it is not needed on GNU systems following the FHS.  The best fix
>>> here would be to patch libcurl such that it checks the CURL_CA_BUNDLE
>>> environment variable invariably, just like the curl command line tool
>>> does.  Until this is done I think we should path packages such as
>>> r-curl to make them usable.  Once we have agreed on a fix to libcurl we
>>> can remove all patches to individual packages using libcurl.
>>
>> That makes sense. Thank you for taking the time to explain.
>
> Sure!  Pushed to master as 8f309571d3847d4bca331061e881fa01d9badb77.

Thanks!

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-09-24 21:38 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-07 14:56 [PATCH] gnu: r-curl: Respect CURL_CA_BUNDLE variable Ricardo Wurmus
2016-09-13  5:45 ` Leo Famulari
2016-09-13 21:53 ` Roel Janssen
2016-09-14  1:17   ` Leo Famulari
2016-09-21 19:24     ` Ricardo Wurmus
2016-09-23  5:26       ` Leo Famulari
2016-09-24  7:46         ` Ricardo Wurmus
2016-09-24 21:39           ` Roel Janssen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).