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

[Pierre Langlois <pierre.langlois@gmx.com>]

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


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

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