diff mbox series

[bug#49946,3/3] guix: node-build-system: Support compiling addons with node-gyp.

Message ID 20210930225611.2143194-4-philip@philipmcgrath.com
State New
Headers show
Series guix: node-build-system: Support compiling addons with node-gyp. | expand

Checks

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

Commit Message

Philip McGrath Sept. 30, 2021, 10:56 p.m. UTC
* gnu/packages/node.scm (node)[arguments]: Replace 'patch-npm-shebang
and 'patch-node-shebang with a new 'patch-nested-shebangs that also
handles node-gyp and other shebangs under "/lib/node_modules".
[inputs]: Add Python for node-gyp as "python-for-target".
(node-lts)[inputs]: Likewise.
* guix/build-system/node.scm (lower): Add optional #:python argument.
* guix/build/node-build-system.scm (set-node-gyp-paths): New
function. Sets the "npm_config_nodedir" and "npm_config_python"
environment variables.
(%standard-phases): Add 'set-node-gyp-paths after 'set-paths.

Co-authored-by: Pierre Langlois <pierre.langlois@gmx.com>
---
 gnu/packages/node.scm            | 33 +++++++++++++++++---------------
 guix/build-system/node.scm       |  7 ++++++-
 guix/build/node-build-system.scm |  9 +++++++++
 3 files changed, 33 insertions(+), 16 deletions(-)

Comments

Pierre Langlois Oct. 2, 2021, 11:49 a.m. UTC | #1
Philip McGrath <philip@philipmcgrath.com> writes:

> * gnu/packages/node.scm (node)[arguments]: Replace 'patch-npm-shebang
> and 'patch-node-shebang with a new 'patch-nested-shebangs that also
> handles node-gyp and other shebangs under "/lib/node_modules".
> [inputs]: Add Python for node-gyp as "python-for-target".
> (node-lts)[inputs]: Likewise.
> * guix/build-system/node.scm (lower): Add optional #:python argument.
> * guix/build/node-build-system.scm (set-node-gyp-paths): New
> function. Sets the "npm_config_nodedir" and "npm_config_python"
> environment variables.
> (%standard-phases): Add 'set-node-gyp-paths after 'set-paths.

Nice! I'll test this with the tree-sitter series. I just had one comment
inline, otherwise it looks good to me.

Do you want me to integrate it into the tree-sitter series or submit it
separately? It might make its way upstream quicker separately, in which
case I'd suggest to send it again in a new bug for more visibility.

>
> Co-authored-by: Pierre Langlois <pierre.langlois@gmx.com>
> ---
>  gnu/packages/node.scm            | 33 +++++++++++++++++---------------
>  guix/build-system/node.scm       |  7 ++++++-
>  guix/build/node-build-system.scm |  9 +++++++++
>  3 files changed, 33 insertions(+), 16 deletions(-)
>
> diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
> index 6d9c3ccc71..805a4f18fc 100644
> --- a/gnu/packages/node.scm
> +++ b/gnu/packages/node.scm
> @@ -244,21 +244,22 @@
>                              python
>                              (string-append python "3")))
>                        "configure" flags))))
> -         (add-after 'patch-shebangs 'patch-npm-shebang
> -           (lambda* (#:key outputs #:allow-other-keys)
> -             (let* ((bindir (string-append (assoc-ref outputs "out")
> -                                           "/bin"))
> -                    (npm    (string-append bindir "/npm"))
> -                    (target (readlink npm)))
> -               (with-directory-excursion bindir
> -                 (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)))))))))
> +         (add-after 'patch-shebangs 'patch-nested-shebangs
> +           (lambda* (#:key inputs outputs #:allow-other-keys)
> +             (let* ((prefix (assoc-ref outputs "out"))
> +                    (path (map (lambda (dir)
> +                                 (string-append dir "/bin"))
> +                               (list prefix
> +                                     (assoc-ref inputs "python-for-target")))))
> +               (with-directory-excursion (string-append prefix "/lib/node_modules")
> +                 (for-each
> +                  (lambda (file)
> +                    (patch-shebang file path))
> +                  (find-files "."
> +                              (lambda (file stat)
> +                                (and (eq? 'regular (stat:type stat))
> +                                     (not (zero? (logand (stat:mode stat) #o100)))))
> +                              #:stat lstat)))))))))

Here you don't necessarily need with-directory-excursion. I see we also
have a executable-file? predicate function in (guix build utils), could
we use that? i.e:

  (for-each
    (lambda (file)
      (patch-shebang file path))
    (find-files (string-append prefix "/lib/node_modules") executable-file?))

>      (native-inputs
>       `(;; Runtime dependencies for binaries used as a bootstrap.
>         ("c-ares" ,c-ares)
> @@ -281,6 +282,7 @@
>      (inputs
>       `(("bash" ,bash-minimal)
>         ("coreutils" ,coreutils)
> +       ("python-for-target" ,python-wrapper) ;; for node-gyp (supports python3)
>         ("c-ares" ,c-ares)
>         ("http-parser" ,http-parser)
>         ("icu4c" ,icu4c)
> @@ -764,6 +766,7 @@ source files.")
>      (inputs
>       `(("bash" ,bash-minimal)
>         ("coreutils" ,coreutils)
> +       ("python-for-target" ,python-wrapper) ;; for node-gyp (supports python3)
>         ("c-ares" ,c-ares)
>         ("icu4c" ,icu4c-67)
>         ("libuv" ,libuv-for-node)
> diff --git a/guix/build-system/node.scm b/guix/build-system/node.scm
> index 98f63f87ef..3e49e67ff6 100644
> --- a/guix/build-system/node.scm
> +++ b/guix/build-system/node.scm
> @@ -1,6 +1,8 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2016 Jelle Licht <jlicht@fsfe.org>
>  ;;; Copyright © 2019 Timothy Sample <samplet@ngyro.com>
> +;;; Copyright © 2021 Pierre Langlois <pierre.langlois@gmx.com>
> +;;; Copyright © 2021 Philip McGrath <philip@philipmcgrath.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -24,6 +26,7 @@
>    #:use-module (guix search-paths)
>    #:use-module (guix build-system)
>    #:use-module (guix build-system gnu)
> +  #:use-module (guix build-system python)
>    #:use-module (ice-9 match)
>    #:export (%node-build-system-modules
>              node-build
> @@ -44,11 +47,12 @@
>  (define* (lower name
>                  #:key source inputs native-inputs outputs system target
>                  (node (default-node))
> +                (python (default-python)) ;; for node-gyp
>                  #:allow-other-keys
>                  #:rest arguments)
>    "Return a bag for NAME."
>    (define private-keywords
> -    '(#:source #:target #:node #:inputs #:native-inputs))
> +    '(#:source #:target #:node #:python #:inputs #:native-inputs))
>  
>    (and (not target)                    ;XXX: no cross-compilation
>         (bag
> @@ -62,6 +66,7 @@
>                          ;; Keep the standard inputs of 'gnu-build-system'.
>                          ,@(standard-packages)))
>           (build-inputs `(("node" ,node)
> +                         ("python" ,python)
>                           ,@native-inputs))
>           (outputs outputs)
>           (build node-build)
> diff --git a/guix/build/node-build-system.scm b/guix/build/node-build-system.scm
> index 70a367618e..5e62eb4784 100644
> --- a/guix/build/node-build-system.scm
> +++ b/guix/build/node-build-system.scm
> @@ -2,6 +2,8 @@
>  ;;; Copyright © 2015 David Thompson <davet@gnu.org>
>  ;;; Copyright © 2016, 2020 Jelle Licht <jlicht@fsfe.org>
>  ;;; Copyright © 2019, 2021 Timothy Sample <samplet@ngyro.com>
> +;;; Copyright © 2021 Pierre Langlois <pierre.langlois@gmx.com>
> +;;; Copyright © 2021 Philip McGrath <philip@philipmcgrath.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -46,6 +48,12 @@
>                (format #t "set HOME to ~s~%" (getenv "HOME")))))))
>    #t)
>  
> +(define* (set-node-gyp-paths #:key inputs #:allow-other-keys)
> +  "Initialize environment variables needed for building native addons."
> +  (setenv "npm_config_nodedir" (assoc-ref inputs "node"))
> +  (setenv "npm_config_python" (assoc-ref inputs "python"))
> +  #t)
> +
>  (define (module-name module)
>    (let* ((package.json (string-append module "/package.json"))
>           (package-meta (call-with-input-file package.json read-json)))
> @@ -144,6 +152,7 @@
>  
>  (define %standard-phases
>    (modify-phases gnu:%standard-phases
> +    (add-after 'set-paths 'set-node-gyp-paths set-node-gyp-paths)
>      (add-after 'unpack 'set-home set-home)
>      (add-before 'configure 'patch-dependencies patch-dependencies)
>      (replace 'configure configure)


The build system changes are now so nice and simple :-). I guess we
don't really need to set npm_config_python, it should be able to find
python in the PATH, but it doesn't hurt.

Thanks,
Pierre
diff mbox series

Patch

diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
index 6d9c3ccc71..805a4f18fc 100644
--- a/gnu/packages/node.scm
+++ b/gnu/packages/node.scm
@@ -244,21 +244,22 @@ 
                             python
                             (string-append python "3")))
                       "configure" flags))))
-         (add-after 'patch-shebangs 'patch-npm-shebang
-           (lambda* (#:key outputs #:allow-other-keys)
-             (let* ((bindir (string-append (assoc-ref outputs "out")
-                                           "/bin"))
-                    (npm    (string-append bindir "/npm"))
-                    (target (readlink npm)))
-               (with-directory-excursion bindir
-                 (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)))))))))
+         (add-after 'patch-shebangs 'patch-nested-shebangs
+           (lambda* (#:key inputs outputs #:allow-other-keys)
+             (let* ((prefix (assoc-ref outputs "out"))
+                    (path (map (lambda (dir)
+                                 (string-append dir "/bin"))
+                               (list prefix
+                                     (assoc-ref inputs "python-for-target")))))
+               (with-directory-excursion (string-append prefix "/lib/node_modules")
+                 (for-each
+                  (lambda (file)
+                    (patch-shebang file path))
+                  (find-files "."
+                              (lambda (file stat)
+                                (and (eq? 'regular (stat:type stat))
+                                     (not (zero? (logand (stat:mode stat) #o100)))))
+                              #:stat lstat)))))))))
     (native-inputs
      `(;; Runtime dependencies for binaries used as a bootstrap.
        ("c-ares" ,c-ares)
@@ -281,6 +282,7 @@ 
     (inputs
      `(("bash" ,bash-minimal)
        ("coreutils" ,coreutils)
+       ("python-for-target" ,python-wrapper) ;; for node-gyp (supports python3)
        ("c-ares" ,c-ares)
        ("http-parser" ,http-parser)
        ("icu4c" ,icu4c)
@@ -764,6 +766,7 @@  source files.")
     (inputs
      `(("bash" ,bash-minimal)
        ("coreutils" ,coreutils)
+       ("python-for-target" ,python-wrapper) ;; for node-gyp (supports python3)
        ("c-ares" ,c-ares)
        ("icu4c" ,icu4c-67)
        ("libuv" ,libuv-for-node)
diff --git a/guix/build-system/node.scm b/guix/build-system/node.scm
index 98f63f87ef..3e49e67ff6 100644
--- a/guix/build-system/node.scm
+++ b/guix/build-system/node.scm
@@ -1,6 +1,8 @@ 
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2016 Jelle Licht <jlicht@fsfe.org>
 ;;; Copyright © 2019 Timothy Sample <samplet@ngyro.com>
+;;; Copyright © 2021 Pierre Langlois <pierre.langlois@gmx.com>
+;;; Copyright © 2021 Philip McGrath <philip@philipmcgrath.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -24,6 +26,7 @@ 
   #:use-module (guix search-paths)
   #:use-module (guix build-system)
   #:use-module (guix build-system gnu)
+  #:use-module (guix build-system python)
   #:use-module (ice-9 match)
   #:export (%node-build-system-modules
             node-build
@@ -44,11 +47,12 @@ 
 (define* (lower name
                 #:key source inputs native-inputs outputs system target
                 (node (default-node))
+                (python (default-python)) ;; for node-gyp
                 #:allow-other-keys
                 #:rest arguments)
   "Return a bag for NAME."
   (define private-keywords
-    '(#:source #:target #:node #:inputs #:native-inputs))
+    '(#:source #:target #:node #:python #:inputs #:native-inputs))
 
   (and (not target)                    ;XXX: no cross-compilation
        (bag
@@ -62,6 +66,7 @@ 
                         ;; Keep the standard inputs of 'gnu-build-system'.
                         ,@(standard-packages)))
          (build-inputs `(("node" ,node)
+                         ("python" ,python)
                          ,@native-inputs))
          (outputs outputs)
          (build node-build)
diff --git a/guix/build/node-build-system.scm b/guix/build/node-build-system.scm
index 70a367618e..5e62eb4784 100644
--- a/guix/build/node-build-system.scm
+++ b/guix/build/node-build-system.scm
@@ -2,6 +2,8 @@ 
 ;;; Copyright © 2015 David Thompson <davet@gnu.org>
 ;;; Copyright © 2016, 2020 Jelle Licht <jlicht@fsfe.org>
 ;;; Copyright © 2019, 2021 Timothy Sample <samplet@ngyro.com>
+;;; Copyright © 2021 Pierre Langlois <pierre.langlois@gmx.com>
+;;; Copyright © 2021 Philip McGrath <philip@philipmcgrath.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -46,6 +48,12 @@ 
               (format #t "set HOME to ~s~%" (getenv "HOME")))))))
   #t)
 
+(define* (set-node-gyp-paths #:key inputs #:allow-other-keys)
+  "Initialize environment variables needed for building native addons."
+  (setenv "npm_config_nodedir" (assoc-ref inputs "node"))
+  (setenv "npm_config_python" (assoc-ref inputs "python"))
+  #t)
+
 (define (module-name module)
   (let* ((package.json (string-append module "/package.json"))
          (package-meta (call-with-input-file package.json read-json)))
@@ -144,6 +152,7 @@ 
 
 (define %standard-phases
   (modify-phases gnu:%standard-phases
+    (add-after 'set-paths 'set-node-gyp-paths set-node-gyp-paths)
     (add-after 'unpack 'set-home set-home)
     (add-before 'configure 'patch-dependencies patch-dependencies)
     (replace 'configure configure)