diff mbox series

[bug#50261] gnu: node: Enable cross-compilation.

Message ID 87a6kzsu92.fsf@gmx.com
State Accepted
Headers show
Series [bug#50261] gnu: node: Enable cross-compilation. | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Pierre Langlois Aug. 29, 2021, 10:06 p.m. UTC 
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

Comments

Mathieu Othacehe Aug. 31, 2021, 1:41 p.m. UTC | #1 
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
Pierre Langlois Sept. 2, 2021, 7:51 p.m. UTC | #2 
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
diff mbox series

Patch

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