all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#50261] [PATCH] gnu: node: Enable cross-compilation.
@ 2021-08-29 22:06 Pierre Langlois
  2021-08-31 13:41 ` Mathieu Othacehe
  0 siblings, 1 reply; 3+ messages in thread
From: Pierre Langlois @ 2021-08-29 22:06 UTC (permalink / raw)
  To: 50261


[-- Attachment #1.1: Type: text/plain, Size: 1318 bytes --]

Hi Guix!

While working on tree-sitter I realized our node packages didn't
cross-compile [0] so I thought I'd give it a go, here's a patch!

Note that fundamentally, because of how the V8 JavaScript engine
bootstraps itself, cross-compiling only works for platforms with the
same bitness. So a 32-bit binary cannot be produced on a 64-bit
system, for example on x86_64:

--8<---------------cut here---------------start------------->8---
# Build OK.
$ guix build --target=aarch64-linux-gnu node

# Will fail...
$ guix build --target=arm-linux-gnueabihf node

# ... unless the host is 32-bit.
$ guix build --system=i686-linux --target=arm-linux-gnueabihf node
--8<---------------cut here---------------end--------------->8---

I'm not sure there's any way to express this restriction in the package
definition, it would be good to fail in a useful way, any thoughts? I
suspect a mechanism for this isn't really worth the effort, I don't know
of any other packages like this.

As a drive-by, the esbuild package didn't build on 32-bit systems so I
attached another patch for that. It was due to the build system using
the "-race" option to detect race conditions in the tests, and this
mechanism only works on 64-bit platforms.

Thanks,
Pierre

[0]: https://lists.gnu.org/archive/html/guix-patches/2021-08/msg00646.html


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

[-- Attachment #2: 0001-gnu-node-Enable-cross-compilation.patch --]
[-- Type: text/x-patch, Size: 17619 bytes --]

From 8b4cc4573d438eb108b8d1a9bcdf99f3af98b514 Mon Sep 17 00:00:00 2001
From: Pierre Langlois <pierre.langlois@gmx.com>
Date: Wed, 11 Aug 2021 16:43:45 +0100
Subject: [PATCH 1/2] gnu: node: Enable cross-compilation.

Node runs parts of itself on the host for bootstraping therefore for
cross-compiling support we need to fidle with the rpath in the build system,
as well as duplicating some of the dependencies as native-inputs and inputs.

* gnu/pakcages/node.scm (node)[arguments]: Refer to /bin/sh and /bin/env
directly instead of using (which).  Add new 'set-bootstrap-host-rpath phase
to correctly set the rpath for binaries that are meant to run on the host.
Pass --cross-compiling and --dest-cpu to configure script if needed.  Set the
CC_host, CXX_host, CC, CCX and PKG_CONFIG variable for cross-compilation.
Refer to the host python.  Do not return #t from any phases.
[native-inputs]: Add c-ares, http-parser, icu4c, libuv, nghttp2, openssl and
zlib.  Remove which.
[inputs]: Add bash and coreutils.
(llhttp-bootstrap)[arguments]: Refer to esbuild via (or native-inputs inputs).
(node-lts)[arguments]: Add new 'set-bootstrap-host-rpath phase to correctly
set the rpath for host binaries.  Pass --cross-compiling and --dest-cpu to
configure script if needed.  Set the CC_host, CXX_host, CCX and PKG_CONFIG
variable for cross-compilation.  Refer to the host python.  Do not return #t
from any phases.  Refer to /bin/sh and /bin/env directly instead of
using (which).  Do not return #t from any phases.
[native-inputs]: Hardcode native inputs instead of inheriting them from node.
[inputs]: Add bash and coreutils.
---
 gnu/packages/node.scm | 188 ++++++++++++++++++++++++++++++++++--------
 1 file changed, 153 insertions(+), 35 deletions(-)

diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
index 36c45e9c7a..2dc23b881c 100644
--- a/gnu/packages/node.scm
+++ b/gnu/packages/node.scm
@@ -6,7 +6,7 @@
 ;;; Copyright © 2017 Mike Gerwitz <mtg@gnu.org>
 ;;; Copyright © 2018 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;; Copyright © 2018, 2019, 2020, 2021 Marius Bakke <marius@gnu.org>
-;;; Copyright © 2020 Pierre Langlois <pierre.langlois@gmx.com>
+;;; Copyright © 2020, 2021 Pierre Langlois <pierre.langlois@gmx.com>
 ;;; Copyright © 2020 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2021 Simon Tournier <zimon.toutoune@gmail.com>
 ;;;
@@ -38,6 +38,7 @@
   #:use-module (gnu packages)
   #:use-module (gnu packages adns)
   #:use-module (gnu packages base)
+  #:use-module (gnu packages bash)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages gcc)
   #:use-module (gnu packages icu4c)
@@ -48,7 +49,9 @@
   #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages python)
   #:use-module (gnu packages tls)
-  #:use-module (gnu packages web))
+  #:use-module (gnu packages web)
+  #:use-module (ice-9 match)
+  #:use-module (srfi srfi-26))
 
 (define-public node
   (package
@@ -111,14 +114,14 @@
                             "test/parallel/test-stdio-closed.js"
                             "test/sequential/test-child-process-emfile.js")
                (("'/bin/sh'")
-                (string-append "'" (which "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 "'" (which "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.
@@ -155,23 +158,65 @@
              ;; TODO: Regenerate certs instead.
              (for-each delete-file
                        '("test/parallel/test-tls-passphrase.js"
-                         "test/parallel/test-tls-server-verify.js"))
-             #t))
+                         "test/parallel/test-tls-server-verify.js"))))
+         (add-before 'configure 'set-bootstrap-host-rpath
+           (lambda* (#:key native-inputs inputs #:allow-other-keys)
+             (let* ((inputs      (or native-inputs inputs))
+                    (c-ares      (assoc-ref inputs "c-ares"))
+                    (http-parser (assoc-ref inputs "http-parser"))
+                    (icu4c       (assoc-ref inputs "icu4c"))
+                    (nghttp2     (assoc-ref inputs "nghttp2"))
+                    (openssl     (assoc-ref inputs "openssl"))
+                    (libuv       (assoc-ref inputs "libuv"))
+                    (zlib        (assoc-ref inputs "zlib")))
+               (substitute* "deps/v8/gypfiles/v8.gyp"
+                 (("'target_name': 'torque'," target)
+                  (string-append target
+                                 "'ldflags': ['-Wl,-rpath="
+                                 c-ares "/lib:"
+                                 http-parser "/lib:"
+                                 icu4c "/lib:"
+                                 nghttp2 "/lib:"
+                                 openssl "/lib:"
+                                 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 '()) inputs
+           (lambda* (#:key outputs (configure-flags '()) native-inputs inputs
                      #:allow-other-keys)
              (let* ((prefix (assoc-ref outputs "out"))
-                    (flags (cons (string-append "--prefix=" prefix)
-                                 configure-flags)))
+                    (flags (cons*
+                             (string-append "--prefix=" prefix)
+                             ,@(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"))))
+                                 '())
+                             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" (string-append (assoc-ref inputs "gcc") "/bin/gcc"))
+               (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 inputs "python")
+                      (string-append (assoc-ref (or native-inputs inputs) "python")
                                      "/bin/python")
                       "configure" flags))))
          (add-after 'patch-shebangs 'patch-npm-shebang
@@ -181,29 +226,37 @@
                     (npm    (string-append bindir "/npm"))
                     (target (readlink npm)))
                (with-directory-excursion bindir
-                 (patch-shebang target (list bindir))
-                 #t))))
+                 (patch-shebang target (list bindir))))))
          (add-after 'install 'patch-node-shebang
            (lambda* (#:key outputs #:allow-other-keys)
              (let* ((bindir (string-append (assoc-ref outputs "out")
                                            "/bin"))
                     (npx    (readlink (string-append bindir "/npx"))))
                (with-directory-excursion bindir
-                 (patch-shebang npx (list bindir))
-                 #t)))))))
+                 (patch-shebang npx (list bindir)))))))))
     (native-inputs
-     `(("python" ,python-2)
+     `(;; Runtime dependencies for binaries used as a bootstrap.
+       ("c-ares" ,c-ares)
+       ("http-parser" ,http-parser)
+       ("icu4c" ,icu4c)
+       ("libuv" ,libuv)
+       ("nghttp2" ,nghttp2 "lib")
+       ("openssl" ,openssl)
+       ("zlib" ,zlib)
+       ;; Regular build-time dependencies.
        ("perl" ,perl)
        ("pkg-config" ,pkg-config)
        ("procps" ,procps)
-       ("util-linux" ,util-linux)
-       ("which" ,which)))
+       ("python" ,python-2)
+       ("util-linux" ,util-linux)))
     (native-search-paths
      (list (search-path-specification
             (variable "NODE_PATH")
             (files '("lib/node_modules")))))
     (inputs
-     `(("c-ares" ,c-ares)
+     `(("bash" ,bash)
+       ("coreutils" ,coreutils)
+       ("c-ares" ,c-ares)
        ("http-parser" ,http-parser)
        ("icu4c" ,icu4c)
        ("libuv" ,libuv)
@@ -551,9 +604,10 @@ parser definition into a C output.")
        #:phases
        (modify-phases %standard-phases
          (replace 'configure
-           (lambda* (#:key inputs #:allow-other-keys)
-             (let ((esbuild (string-append (assoc-ref inputs "esbuild")
-                                           "/bin/esbuild")))
+           (lambda* (#:key native-inputs inputs #:allow-other-keys)
+             (let ((esbuild (string-append
+                              (assoc-ref (or native-inputs inputs) "esbuild")
+                              "/bin/esbuild")))
                (invoke esbuild
                        "--platform=node"
                        "--outfile=bin/generate.js"
@@ -625,21 +679,72 @@ source files.")
            "--with-intl=system-icu"))
        ((#:phases phases)
         `(modify-phases ,phases
+           (replace 'set-bootstrap-host-rpath
+             (lambda* (#:key native-inputs inputs #:allow-other-keys)
+               (let* ((inputs        (or native-inputs inputs))
+                      (c-ares        (assoc-ref inputs "c-ares"))
+                      (google-brotli (assoc-ref inputs "google-brotli"))
+                      (icu4c         (assoc-ref inputs "icu4c"))
+                      (nghttp2       (assoc-ref inputs "nghttp2"))
+                      (openssl       (assoc-ref inputs "openssl"))
+                      (libuv         (assoc-ref inputs "libuv"))
+                      (zlib          (assoc-ref inputs "zlib"))
+                      (host-binaries '("torque"
+                                       "bytecode_builtins_list_generator"
+                                       "gen-regexp-special-case"
+                                       "node_mksnapshot"
+                                       "mksnapshot")))
+                 (substitute* '("node.gyp" "tools/v8_gypfiles/v8.gyp")
+                   (((string-append "'target_name': '("
+                                    (string-join host-binaries "|")
+                                    ")',")
+                      target)
+                    (string-append target
+                                   "'ldflags': ['-Wl,-rpath="
+                                   c-ares "/lib:"
+                                   google-brotli "/lib:"
+                                   icu4c "/lib:"
+                                   nghttp2 "/lib:"
+                                   openssl "/lib:"
+                                   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 '()) inputs
+             (lambda* (#:key outputs (configure-flags '()) native-inputs inputs
                        #:allow-other-keys)
                (let* ((prefix (assoc-ref outputs "out"))
-                      (flags (cons (string-append "--prefix=" prefix)
-                                   configure-flags)))
+                      (flags (cons*
+                               (string-append "--prefix=" prefix)
+                               ,@(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"))))
+                                   '())
+                               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 inputs "python")
+                        (string-append (assoc-ref (or native-inputs inputs) "python")
                                        "/bin/python3")
                         "configure" flags))))
            (replace 'patch-files
@@ -652,14 +757,14 @@ source files.")
                               "test/parallel/test-stdio-closed.js"
                               "test/sequential/test-child-process-emfile.js")
                  (("'/bin/sh'")
-                  (string-append "'" (which "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 "'" (which "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.
@@ -707,20 +812,33 @@ source files.")
                  (copy-file (string-append llhttp "/src/http.c")
                             "deps/llhttp/src/http.c")
                  (copy-file (string-append llhttp "/include/llhttp.h")
-                            "deps/llhttp/include/llhttp.h"))
-               #t))))))
+                            "deps/llhttp/include/llhttp.h"))))))))
+    (native-inputs
+     `(;; Runtime dependencies for binaries used as a bootstrap.
+       ("c-ares" ,c-ares)
+       ("google-brotli" ,google-brotli)
+       ("icu4c" ,icu4c-67)
+       ("libuv" ,libuv-for-node)
+       ("nghttp2" ,nghttp2 "lib")
+       ("openssl" ,openssl)
+       ("zlib" ,zlib)
+       ;; Regular build-time dependencies.
+       ("perl" ,perl)
+       ("pkg-config" ,pkg-config)
+       ("procps" ,procps)
+       ("python" ,python)
+       ("util-linux" ,util-linux)))
     (inputs
-     `(("c-ares" ,c-ares)
+     `(("bash" ,bash)
+       ("coreutils" ,coreutils)
+       ("c-ares" ,c-ares)
        ("icu4c" ,icu4c-67)
        ("libuv" ,libuv-for-node)
        ("llhttp" ,llhttp-bootstrap)
        ("google-brotli" ,google-brotli)
        ("nghttp2" ,nghttp2 "lib")
        ("openssl" ,openssl)
-       ("zlib" ,zlib)))
-    (native-inputs
-     (alist-replace "python" (list python-3)
-                    (package-native-inputs node)))))
+       ("zlib" ,zlib)))))
 
 (define-public libnode
   (package/inherit node
-- 
2.33.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-gnu-esbuild-Disable-race-detector-on-32-bit-targets.patch --]
[-- Type: text/x-patch, Size: 1477 bytes --]

From 17cd8ccb2255d4bb392a94468abeb785e134ff1e Mon Sep 17 00:00:00 2001
From: Pierre Langlois <pierre.langlois@gmx.com>
Date: Mon, 30 Aug 2021 00:25:58 +0100
Subject: [PATCH 2/2] gnu: esbuild: Disable race detector on 32-bit targets.

* gnu/packages/web.scm (esbuild)[arguments]: Set the ESBUILD_RACE
variable to an empty string to remove the -race option.
---
 gnu/packages/web.scm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/web.scm b/gnu/packages/web.scm
index f0ac9ccee2..5817d2dd95 100644
--- a/gnu/packages/web.scm
+++ b/gnu/packages/web.scm
@@ -1672,13 +1672,17 @@ used to validate and fix HTML data.")
            #t))))
     (build-system go-build-system)
     (arguments
-     '(#:import-path "github.com/evanw/esbuild/cmd/esbuild"
+     `(#:import-path "github.com/evanw/esbuild/cmd/esbuild"
        #:unpack-path "github.com/evanw/esbuild"
        #:phases
        (modify-phases %standard-phases
          (replace 'check
            (lambda* (#:key tests? unpack-path #:allow-other-keys)
              (when tests?
+               ;; The "Go Race Detector" is only supported on 64-bit
+               ;; platforms, this variable disables it.
+               (unless ,(target-64bit?)
+                 (setenv "ESBUILD_RACE" ""))
                (with-directory-excursion (string-append "src/" unpack-path)
                  (invoke "make" "test-go")))
              #t)))))
--
2.33.0


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

* [bug#50261] [PATCH] gnu: node: Enable cross-compilation.
  2021-08-29 22:06 [bug#50261] [PATCH] gnu: node: Enable cross-compilation Pierre Langlois
@ 2021-08-31 13:41 ` Mathieu Othacehe
  2021-09-02 19:51   ` bug#50261: " Pierre Langlois
  0 siblings, 1 reply; 3+ messages in thread
From: Mathieu Othacehe @ 2021-08-31 13:41 UTC (permalink / raw)
  To: Pierre Langlois; +Cc: 50261


Hello Pierre,

> While working on tree-sitter I realized our node packages didn't
> cross-compile [0] so I thought I'd give it a go, here's a patch!

Nice!

> I'm not sure there's any way to express this restriction in the package
> definition, it would be good to fail in a useful way, any thoughts? I
> suspect a mechanism for this isn't really worth the effort, I don't know
> of any other packages like this.

Yeah that's a strange limitation, I don't think we have something
similar elsewhere. A restriction that compares the host and target
bitness is maybe not worth it.

> -                      (string-append (assoc-ref inputs "python")
> +                      (string-append (assoc-ref (or native-inputs inputs) "python")

You have several lines such as this one that are longer than the 78
columns limit.

Other than that, it looks cool, and you can go ahead.

Thanks,

Mathieu




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

* bug#50261: [PATCH] gnu: node: Enable cross-compilation.
  2021-08-31 13:41 ` Mathieu Othacehe
@ 2021-09-02 19:51   ` Pierre Langlois
  0 siblings, 0 replies; 3+ messages in thread
From: Pierre Langlois @ 2021-09-02 19:51 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 50261-done, Pierre Langlois

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

Hi Mathieu,

Mathieu Othacehe <othacehe@gnu.org> writes:

> Hello Pierre,
>
>> While working on tree-sitter I realized our node packages didn't
>> cross-compile [0] so I thought I'd give it a go, here's a patch!
>
> Nice!
>
>> I'm not sure there's any way to express this restriction in the package
>> definition, it would be good to fail in a useful way, any thoughts? I
>> suspect a mechanism for this isn't really worth the effort, I don't know
>> of any other packages like this.
>
> Yeah that's a strange limitation, I don't think we have something
> similar elsewhere. A restriction that compares the host and target
> bitness is maybe not worth it.
>
>> -                      (string-append (assoc-ref inputs "python")
>> +                      (string-append (assoc-ref (or native-inputs inputs) "python")
>
> You have several lines such as this one that are longer than the 78
> columns limit.

I did my best to fix those by reworking the configure flags as such:

--8<---------------cut here---------------start------------->8---
             (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))))
--8<---------------cut here---------------end--------------->8---

It's still one column over, but I figured it was OK, I couldn't find
another way to write it without sacrificing readability, hope that's OK
:-).

> Other than that, it looks cool, and you can go ahead.

Thanks for the review! Pushed with
9f7c4f380fdd86d81c805b72e4d05e9e658d3dc2 and
5ee38c467298091e98fa12be45facdcc63a59a87.

Pierre

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

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

end of thread, other threads:[~2021-09-02 20:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-29 22:06 [bug#50261] [PATCH] gnu: node: Enable cross-compilation Pierre Langlois
2021-08-31 13:41 ` Mathieu Othacehe
2021-09-02 19:51   ` bug#50261: " Pierre Langlois

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.