bug#43228: r-v8 doesn't build since node 10.22 update

[Pierre Langlois <pierre.langlois@gmx.com>]

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

Hi Mark,

Mark H Weaver writes:

> Hi Pierre,
>
> I'm quoting your message out of order to ease my reply.
>
> Pierre Langlois <pierre.langlois@gmx.com> writes:
>
>> I'm afraid I broke r-v8 and a few of its dependants by updating node,
>> sorry about that!
> [...]
>> I'm not sure how to fix this, I'm happy to revert the node update if
>> needed, let me know! Then we'd have to wait for the next core-updates
>> cycle so that we no longer graft nghttp2.
>
> We will actually need node-10.22 (or at least 10.21) in 'master' in just
> over 2 weeks, when we'll be compelled to update IceCat to version 78.
> On Sept 22, Mozilla is scheduled to release a new batch of security
> fixes in 78.3, and there will be no corresponding 68.x release.

Oooh cool! Looking forwards to icecat 78!

> (In fact, I had an *identical* commit on my private branch to update
> 'node' to 10.22, to allow testing IceCat 78 WIP.)
>
> However, if needed, I suppose it might be sufficient for my purposes to
> leave 'node' at 10.19.0, and to bind a separate 'node-10.22' variable to
> the new version.
>
>> AFAIK, the new node uses a function from nghttp2 1.41 that's not
>> present in 1.40, `nghttp2_option_set_max_settings'. However, since curl
>> depends on nghttp2 we've grafted 1.40 -> 1.41 to avoid a full rebuild.
>>
>> Looking at r-v8's log [0], it complains that the symbol is missing,
>> indicating it's trying to link with the old version 1.40. I /believe/
>> it's inherited it through r-curl.
>
> If grafting is working as it should, then nghttp2-1.40 should never be
> linked at runtime.  However, it is certainly the case that most things
> (except node-10.22) are *built* against nghttp2-1.40, where the
> aforementioned symbol is missing.
>
> One possible solution might be to update the replacement (graft) for
> _curl_ so that it's *built* against nghttp2-1.41.  Something like this
> (untested):
>
> --8<---------------cut here---------------start------------->8---
> diff --git a/gnu/packages/curl.scm b/gnu/packages/curl.scm
> index 55b7e4393b..bfcb52b678 100644
> --- a/gnu/packages/curl.scm
> +++ b/gnu/packages/curl.scm
> @@ -183,6 +183,9 @@ tunneling, and so on.")
>                (sha256
>                 (base32
>                  "0wlppmx9iry8slh4pqcxj7lwc6fqwnlhh9ri2pcym2rx76a8gwfd"))))
> +    (inputs
> +     `(("nghttp2" ,nghttp2-1.41 "lib")
> +       ,@(alist-delete "nghttp2" (package-inputs curl))))
>      (arguments
>       (substitute-keyword-arguments (package-arguments curl)
>         ((#:phases phases)
> --8<---------------cut here---------------end--------------->8---
>
> Would you like to try this and see if it solves the problem?

I'm afraid this still doesn't solve the problem. AFAIU, grafting the new
curl happens after building r-v8, so at link time it still sees the old
nghttp2 version.

Thinking about this, since the new node essentially uses a new ABI to
link with nghttp2 by requiring a new symbol, this isn't something we can
fix with grafting I believe.

It's possible to make r-curl specifically depend on the new curl like
so:

--8<---------------cut here---------------start------------->8---
diff --git a/gnu/packages/cran.scm b/gnu/packages/cran.scm
index 2c202e8508..16022c695d 100644
--- a/gnu/packages/cran.scm
+++ b/gnu/packages/cran.scm
@@ -1038,7 +1038,7 @@ if(_ca_bundle != NULL) { curl_easy_setopt(handle, CURLOPT_CAINFO, _ca_bundle); }
 " m)))
              #t)))))
     (inputs
-     `(("libcurl" ,curl)
+     `(("libcurl" ,curl-7.71.0)
        ("zlib" ,zlib)))
     (native-inputs
      `(("pkg-config" ,pkg-config)))
diff --git a/gnu/packages/curl.scm b/gnu/packages/curl.scm
index 55b7e4393b..aa103306a6 100644
--- a/gnu/packages/curl.scm
+++ b/gnu/packages/curl.scm
@@ -172,7 +172,7 @@ tunneling, and so on.")
     (inputs (alist-delete "openldap" (package-inputs curl))))))
 
 ;; Replacement package to fix CVE-2020-8169 and CVE-2020-8177.
-(define curl-7.71.0
+(define-public curl-7.71.0
   (package
     (inherit curl)
     (version "7.71.0")
@@ -183,6 +183,9 @@ tunneling, and so on.")
               (sha256
                (base32
                 "0wlppmx9iry8slh4pqcxj7lwc6fqwnlhh9ri2pcym2rx76a8gwfd"))))
+    (inputs
+     `(("nghttp2" ,nghttp2-1.41 "lib")
+       ,@(alist-delete "nghttp2" (package-inputs curl))))
     (arguments
      (substitute-keyword-arguments (package-arguments curl)
        ((#:phases phases)
--8<---------------cut here---------------end--------------->8---

But I'm not sure I like this very much, this is getting a bit messy.

Instead, I'm thinking your suggestion of leaving 'node' at 10.19 for now
(or 10.20, I can try that) and then introduce a 'node-10.22' package
that can be used for Icecat is better. I can do that. How does that
sound?

Thanks,
Pierre

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 519 bytes --]