[bug#49946] [PATCH 1/3] gnu: node: Avoid duplicating build phases.

[Philip McGrath <philip@philipmcgrath.com>]

* gnu/packages/node.scm (node)[arguments]: Split 'patch-files phase
into 'patch-hardcoded-program-references and
'delete-problematic-tests. Adapt those phases and 'configure to work
unmodified on node-lts.
(node, node-lts)[inputs]: Use bash-minimal rather than bash.
(node-lts)[arguments]: Inherit 'patch-hardcoded-program-references,
'delete-problemating-tests, and 'configure phases from the bootstrap
node. Remove the 'patch-files phase, keeping its remaining
non-inherited work in a new 'replace-llhttp-sources phase.
---
 gnu/packages/node.scm | 150 +++++++++++-------------------------------
 1 file changed, 39 insertions(+), 111 deletions(-)

diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
index f8ac95884c..34c2bfa9d4 100644
--- a/gnu/packages/node.scm
+++ b/gnu/packages/node.scm
@@ -9,6 +9,7 @@
 ;;; Copyright © 2020, 2021 Pierre Langlois <pierre.langlois@gmx.com>
 ;;; Copyright © 2020 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
+;;; Copyright © 2021 Philip McGrath <philip@philipmcgrath.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -105,14 +106,22 @@
        #:test-target "test-ci-js"
        #:phases
        (modify-phases %standard-phases
-         (add-before 'configure 'patch-files
+         (add-before 'configure 'patch-hardcoded-program-references
            (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-stdio-closed.js"
-                            "test/sequential/test-child-process-emfile.js")
+             (substitute*
+                 (let ((common
+                        '("lib/child_process.js"
+                          "lib/internal/v8_prof_polyfill.js"
+                          "test/parallel/test-child-process-spawnsync-shell.js"
+                          "test/parallel/test-stdio-closed.js"
+                          "test/sequential/test-child-process-emfile.js"))
+                       ;; not in bootstap node:
+                       (sigxfsz "test/parallel/test-fs-write-sigxfsz.js"))
+                   (if (file-exists? sigxfsz)
+                       (cons sigxfsz common)
+                       common))
                (("'/bin/sh'")
                 (string-append "'" (assoc-ref inputs "bash") "/bin/sh'")))
 
@@ -122,18 +131,28 @@
                             "test/parallel/test-child-process-exec-env.js")
                (("'/usr/bin/env'")
                 (string-append "'" (assoc-ref inputs "coreutils")
-                               "/bin/env'")))
+                               "/bin/env'")))))
+         (add-after 'patch-hardcoded-program-references 'delete-problematic-tests
+           (lambda* (#:key inputs #:allow-other-keys)
+             (define (delete-file-if-exists pth)
+               (when (file-exists? pth)
+                 (delete-file pth)))
 
              ;; 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
+             (for-each delete-file-if-exists
                        '("test/parallel/test-cluster-master-error.js"
                          "test/parallel/test-cluster-master-kill.js"
                          ;; See also <https://github.com/nodejs/node/issues/25903>.
+                         ;; (no longer exists in node-lts)
                          "test/sequential/test-performance.js"))
 
-             ;; This requires a DNS resolver.
-             (delete-file "test/parallel/test-dns.js")
+             ;; These require a DNS resolver.
+             (for-each delete-file-if-exists
+                       '("test/parallel/test-dns.js"
+                         ;; not in the bootstrap node:
+                         "test/parallel/test-dns-lookupService-promises.js"))
+
 
              ;; FIXME: This test fails randomly:
              ;; https://github.com/nodejs/node/issues/31213
@@ -217,9 +236,13 @@
                (setenv "CXX" ,(cxx-for-target))
                (setenv "PKG_CONFIG" ,(pkg-config-for-target))
                (apply invoke
-                      (string-append (assoc-ref (or native-inputs inputs)
-                                                "python")
-                                     "/bin/python")
+                      (let ((python
+                             (string-append (assoc-ref (or native-inputs inputs)
+                                                       "python")
+                                            "/bin/python")))
+                        (if (file-exists? python)
+                            python
+                            (string-append python "3")))
                       "configure" flags))))
          (add-after 'patch-shebangs 'patch-npm-shebang
            (lambda* (#:key outputs #:allow-other-keys)
@@ -256,7 +279,7 @@
             (variable "NODE_PATH")
             (files '("lib/node_modules")))))
     (inputs
-     `(("bash" ,bash)
+     `(("bash" ,bash-minimal)
        ("coreutils" ,coreutils)
        ("c-ares" ,c-ares)
        ("http-parser" ,http-parser)
@@ -711,103 +734,8 @@ source files.")
                                    libuv "/lib:"
                                    zlib "/lib"
                                    "'],"))))))
-           (replace 'configure
-             ;; Node's configure script is actually a python script, so we can't
-             ;; run it with bash.
-             (lambda* (#:key outputs (configure-flags '()) native-inputs inputs
-                       #:allow-other-keys)
-               (let* ((prefix (assoc-ref outputs "out"))
-                      (xflags ,(if (%current-target-system)
-                                   `'("--cross-compiling"
-                                     ,(string-append
-                                       "--dest-cpu="
-                                       (match (%current-target-system)
-                                         ((? (cut string-prefix? "arm" <>))
-                                          "arm")
-                                         ((? (cut string-prefix? "aarch64" <>))
-                                          "arm64")
-                                         ((? (cut string-prefix? "i686" <>))
-                                          "ia32")
-                                         ((? (cut string-prefix? "x86_64" <>))
-                                          "x64")
-                                         ((? (cut string-prefix? "powerpc64" <>))
-                                          "ppc64")
-                                         (_ "unsupported"))))
-                                   ''()))
-                      (flags (cons
-                               (string-append "--prefix=" prefix)
-                               (append xflags configure-flags))))
-                 (format #t "build directory: ~s~%" (getcwd))
-                 (format #t "configure flags: ~s~%" flags)
-                 ;; Node's configure script expects the CC environment variable to
-                 ;; be set.
-                 (setenv "CC_host" "gcc")
-                 (setenv "CXX_host" "g++")
-                 (setenv "CC" ,(cc-for-target))
-                 (setenv "CXX" ,(cxx-for-target))
-                 (setenv "PKG_CONFIG" ,(pkg-config-for-target))
-                 (apply invoke
-                        (string-append (assoc-ref (or native-inputs inputs)
-                                                  "python")
-                                       "/bin/python3")
-                        "configure" flags))))
-           (replace 'patch-files
+           (add-after 'delete-problematic-tests 'replace-llhttp-sources
              (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-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-master-error.js"
-                           "test/parallel/test-cluster-master-kill.js"))
-
-               ;; These require a DNS resolver.
-               (for-each delete-file
-                         '("test/parallel/test-dns.js"
-                           "test/parallel/test-dns-lookupService-promises.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"))
-
                ;; Replace pre-generated llhttp sources
                (let ((llhttp (assoc-ref inputs "llhttp")))
                  (copy-file (string-append llhttp "/src/llhttp.c")
@@ -834,7 +762,7 @@ source files.")
        ("python" ,python)
        ("util-linux" ,util-linux)))
     (inputs
-     `(("bash" ,bash)
+     `(("bash" ,bash-minimal)
        ("coreutils" ,coreutils)
        ("c-ares" ,c-ares)
        ("icu4c" ,icu4c-67)
-- 
2.30.2