diff mbox series

[bug#53414] update Node LTS to 16.13.2

Message ID 65d2c6fb-9882-5815-b6b3-5eb6f7011464@arctype.co
State New
Headers show
Series [bug#53414] update Node LTS to 16.13.2 | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Ryan Sundberg June 9, 2022, 6:39 a.m. UTC
Hi all, attached is a revised patch that keeps node-lts bound to node-14
and just adds the new node-16.

Thank you for the feedback Pierre, I agree we should not break any
packages by transitioning node-lts over without testing.

I also updated again to 16.15.1 from 16.15.0 here.

--Ryan Sundberg

Comments

M June 9, 2022, 8:49 p.m. UTC | #1
Ryan Sundberg via Guix-patches via schreef op wo 08-06-2022 om 23:39 [-
0700]:
> +                  #t))))

No need to return #t.
M June 9, 2022, 8:50 p.m. UTC | #2
Ryan Sundberg via Guix-patches via schreef op wo 08-06-2022 om 23:39 [-
0700]:
> +       ((#:phases phases)
> +        `(modify-phases ,phases

Do #~(modify-phases #$phases ...) instead of quasiquote/unquote,
otherwise this will break when node-14 switches to G-exps.

Greetings,
Maxime.
M June 9, 2022, 8:53 p.m. UTC | #3
Ryan Sundberg via Guix-patches via schreef op wo 08-06-2022 om 23:39 [-
0700]:
> +              (modules '((guix build utils)))
> +              (snippet
> +               `(begin
> +                  ;; Remove bundled software, where possible
> +                  (for-each delete-file-recursively
> +                            '("deps/cares"
> +                              "deps/icu-small"
> +                              "deps/nghttp2"
> +                              "deps/openssl"
> +                              "deps/zlib"))
> +                  (substitute* "Makefile"
> +                    ;; Remove references to bundled software.
> +                    (("deps/uv/uv.gyp") "")
> +                    (("deps/zlib/zlib.gyp") ""))
> +                  #t))))

This is identical to what node-14 has, so you can use inheritance to
simplify things:

  (origin
    (inherit (package-source node-14))
    (uri ...)
    (sha256 ...))

Also, could you verify that no new things have been bundled?

Greetings,
Maxime.
M June 9, 2022, 8:55 p.m. UTC | #4
Ryan Sundberg via Guix-patches via schreef op wo 08-06-2022 om 23:39 [-
0700]:
> +                  (string-append "'" (assoc-ref inputs "bash")
> "/bin/sh'")))

Can be de-label-ified and simplified to (string-append "'" (search-
input-file inputs "bin/bash")).
M June 9, 2022, 9:03 p.m. UTC | #5
Ryan Sundberg via Guix-patches via schreef op wo 08-06-2022 om 23:39 [-
0700]:
> Hi all, attached is a revised patch that keeps node-lts bound to node-14
> and just adds the new node-16.
> 
> Thank you for the feedback Pierre, I agree we should not break any
> packages by transitioning node-lts over without testing.
> 
> I also updated again to 16.15.1 from 16.15.0 here.
> 
> --Ryan Sundberg

Debian's node package does some changes:

  * benchmark_without_alice.patch: removes something non-free
  * privacy_breach.patch: patch a html file to not point to something
    external
  * localhost-no-adrconfig.patch: a bug fix
  * test_ci.patch: flaky tests
  * skip-buffer-nan-internal-check.patch: remove broken test
  * falky-cpu-prof-riscv64.patch: likewise
  * flag_atomic.patch: ‘avoid surprises on ... and ppc*el’

Maybe some of them are needed on Guix as well?
(See https://packages.debian.org/sid/nodejs for a tarball with changes)

Greetings,
Maxime
Mekeor Melire Nov. 16, 2022, 11:23 p.m. UTC | #6
2022-06-09 / 23:03 / maximedevos@telenet.be:

> Debian's node package does some changes:
>   * benchmark_without_alice.patch: removes something non-free

I'm not sure why Debian removes "Alice's Adventures in Wonderland"
because this work is in Public Domain. I think it's fine to keep it.

  https://github.com/nodejs/node/blob/main/benchmark/fixtures/alice.html

> Maybe some of them are needed on Guix as well?
> (See https://packages.debian.org/sid/nodejs for a tarball with changes)
diff mbox series

Patch

From 78f63a9440c03e54ed24a58f9495613691dee1c6 Mon Sep 17 00:00:00 2001
From: Ryan Sundberg <ryan@arctype.co>
Date: Sun, 1 May 2022 23:00:13 -0700
Subject: [PATCH] gnu: node-16: Package node.js version 16.15.1

* gnu/packages/node.scm (node-16): New variable.

Co-authored-by: zamfofex <zamfofex@twdb.moe>
---
 gnu/packages/node.scm | 180 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 178 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
index c8d81fbd22..2838a41aa9 100644
--- a/gnu/packages/node.scm
+++ b/gnu/packages/node.scm
@@ -11,6 +11,8 @@ 
 ;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;; Copyright © 2021 Guillaume Le Vaillant <glv@posteo.net>
 ;;; Copyright © 2021, 2022 Philip McGrath <philip@philipmcgrath.com>
+;;; Copyright © 2022 zamfofex <zamfofex@twdb.moe>
+;;; Copyright © 2022 Ryan Sundberg <ryan@arctype.co>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -663,7 +665,7 @@  (define-public node-llparse-bootstrap
 parser definition into a C output.")
     (license license:expat)))
 
-(define-public llhttp-bootstrap
+(define llhttp-2
   (package
     (name "llhttp")
     (version "2.1.4")
@@ -733,7 +735,49 @@  (define-public llhttp-bootstrap
 source files.")
     (license license:expat)))
 
-(define-public node-lts
+(define-public llhttp-bootstrap
+  (package
+    (inherit llhttp-2)
+    (name "llhttp")
+    (version "6.0.6")
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/nodejs/llhttp.git")
+                    (commit (string-append "v" version))))
+              (file-name (git-file-name name version))
+              (sha256
+               (base32
+                "1c1p39m46frpslm5yx13hj58r7s0cila03yvqp6caip5dbizpfmr"))
+              (modules '((guix build utils)))
+              (snippet
+               '(begin
+                  ;; Fix imports for esbuild.
+                  ;; https://github.com/evanw/esbuild/issues/477
+                  (substitute* "src/llhttp/http.ts"
+                    (("\\* as assert") "assert"))
+                  (substitute* "Makefile"
+                    (("npx ts-node bin/generate.ts")
+                     "node bin/generate.js"))
+                  #t))))
+    (build-system gnu-build-system)
+    (arguments
+     (substitute-keyword-arguments (package-arguments llhttp-2)
+       ((#:phases phases)
+        `(modify-phases ,phases
+           (replace 'install-src
+             (lambda* (#:key outputs #:allow-other-keys)
+               (let* ((out (assoc-ref outputs "out"))
+                      (src-dir (string-append out "/src"))
+                      (src-src-dir (string-append src-dir "/src")))
+                 (install-file "src/llhttp.gyp" src-dir)
+                 (install-file "src/common.gypi" src-dir)
+                 (install-file "build/c/llhttp.c" src-src-dir)
+                 (install-file "src/native/http.c" src-src-dir)
+                 (install-file "src/native/api.c" src-src-dir)
+                 #t)))))))))
+
+(define-public node-14
   (package
     (inherit node)
     (version "14.19.3")
@@ -881,6 +925,136 @@  (define-public node-lts
            c-ares-for-node
            icu4c-70
            libuv-for-node
+           llhttp-2
+           brotli
+           `(,nghttp2 "lib")
+           openssl
+           python-wrapper ;; for node-gyp (supports python3)
+           zlib))))
+
+(define-public node-16
+  (package
+    (inherit node-14)
+    (version "16.15.1")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "https://nodejs.org/dist/v" version
+                                  "/node-v" version ".tar.xz"))
+              (sha256
+               (base32
+                "0zcv2raa9d4g7dr7v3i2pkfrq076b085f9bmlq4i2wb93wy9vsfl"))
+              (modules '((guix build utils)))
+              (snippet
+               `(begin
+                  ;; Remove bundled software, where possible
+                  (for-each delete-file-recursively
+                            '("deps/cares"
+                              "deps/icu-small"
+                              "deps/nghttp2"
+                              "deps/openssl"
+                              "deps/zlib"))
+                  (substitute* "Makefile"
+                    ;; Remove references to bundled software.
+                    (("deps/uv/uv.gyp") "")
+                    (("deps/zlib/zlib.gyp") ""))
+                  #t))))
+    (arguments
+     (substitute-keyword-arguments (package-arguments node-14)
+       ((#:phases phases)
+        `(modify-phases ,phases
+           (replace 'delete-problematic-tests
+             (lambda* (#:key inputs #:allow-other-keys)
+               ;; Fix hardcoded /bin/sh references.
+               (substitute* '("lib/child_process.js"
+                              "lib/internal/v8_prof_polyfill.js"
+                              "test/parallel/test-child-process-spawnsync-shell.js"
+                              "test/parallel/test-fs-write-sigxfsz.js"
+                              "test/parallel/test-stdin-from-file-spawn.js"
+                              "test/parallel/test-stdio-closed.js"
+                              "test/sequential/test-child-process-emfile.js")
+                 (("'/bin/sh'")
+                  (string-append "'" (assoc-ref inputs "bash") "/bin/sh'")))
+
+               ;; Fix hardcoded /usr/bin/env references.
+               (substitute* '("test/parallel/test-child-process-default-options.js"
+                              "test/parallel/test-child-process-env.js"
+                              "test/parallel/test-child-process-exec-env.js")
+                 (("'/usr/bin/env'")
+                  (string-append "'" (assoc-ref inputs "coreutils")
+                                 "/bin/env'")))
+
+               ;; FIXME: These tests fail in the build container, but they don't
+               ;; seem to be indicative of real problems in practice.
+               (for-each delete-file
+                         '("test/parallel/test-cluster-primary-error.js"
+                           "test/parallel/test-cluster-primary-kill.js"
+                           "test/parallel/test-release-npm.js"))
+
+               ;; These require a DNS resolver.
+               (for-each delete-file
+                         '("test/parallel/test-dns.js"
+                           "test/parallel/test-dns-lookupService-promises.js"))
+
+               ;; These tests require networking.
+               (delete-file "test/parallel/test-https-agent-unref-socket.js")
+
+               ;; This test is timing-sensitive, and fails sporadically on
+               ;; slow, busy, or even very fast machines.
+               (delete-file "test/parallel/test-fs-utimes.js")
+
+               ;; FIXME: This test fails randomly:
+               ;; https://github.com/nodejs/node/issues/31213
+               (delete-file "test/parallel/test-net-listen-after-destroying-stdin.js")
+
+               ;; FIXME: These tests fail on armhf-linux:
+               ;; https://github.com/nodejs/node/issues/31970
+               ,@(if (target-arm32?)
+                     '((for-each delete-file
+                                 '("test/parallel/test-zlib.js"
+                                   "test/parallel/test-zlib-brotli.js"
+                                   "test/parallel/test-zlib-brotli-flush.js"
+                                   "test/parallel/test-zlib-brotli-from-brotli.js"
+                                   "test/parallel/test-zlib-brotli-from-string.js"
+                                   "test/parallel/test-zlib-convenience-methods.js"
+                                   "test/parallel/test-zlib-random-byte-pipes.js"
+                                   "test/parallel/test-zlib-write-after-flush.js")))
+                     '())
+
+               ;; These tests have an expiry date: they depend on the validity of
+               ;; TLS certificates that are bundled with the source.  We want this
+               ;; package to be reproducible forever, so remove those.
+               ;; TODO: Regenerate certs instead.
+               (for-each delete-file
+                         '("test/parallel/test-tls-passphrase.js"
+                           "test/parallel/test-tls-server-verify.js"
+                           "test/parallel/test-https-selfsigned-no-keycertsign-no-crash.js"))))
+           (replace 'replace-llhttp-sources
+             (lambda* (#:key inputs #:allow-other-keys)
+               ;; Replace pre-generated llhttp sources
+               (delete-file-recursively "deps/llhttp")
+               (copy-recursively (string-append (assoc-ref inputs "llhttp") "/src")
+                                 "deps/llhttp")))))))
+    (native-inputs
+     (list ;; Runtime dependencies for binaries used as a bootstrap.
+           c-ares-for-node
+           brotli
+           icu4c
+           libuv-for-node
+           `(,nghttp2 "lib")
+           openssl
+           zlib
+           ;; Regular build-time dependencies.
+           perl
+           pkg-config
+           procps
+           python
+           util-linux))
+    (inputs
+     (list bash-minimal
+           coreutils
+           c-ares-for-node
+           icu4c
+           libuv-for-node
            llhttp-bootstrap
            brotli
            `(,nghttp2 "lib")
@@ -888,6 +1062,8 @@  (define-public node-lts
            python-wrapper ;; for node-gyp (supports python3)
            zlib))))
 
+(define-public node-lts node-14)
+
 (define-public libnode
   (package/inherit node
     (name "libnode")
-- 
2.34.0