Package: guix-patches;
Reported by: Philip McGrath <philip <at> philipmcgrath.com>
Date: Sun, 14 Nov 2021 12:43:01 UTC
Severity: normal
Tags: patch
Done: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Bug is archived. No further changes may be made.
Message #41 received at 51838 <at> debbugs.gnu.org (full text, mbox):
From: Liliana Marie Prikler <liliana.prikler <at> gmail.com> To: Philip McGrath <philip <at> philipmcgrath.com>, 51838 <at> debbugs.gnu.org Cc: Pierre Langlois <pierre.langlois <at> gmx.com> Subject: Re: [PATCH 03/11] guix: node-build-system: Support compiling add-ons with node-gyp. Date: Sun, 14 Nov 2021 21:44:11 +0100
Hi Philip
Am Sonntag, den 14.11.2021, 08:04 -0500 schrieb Philip McGrath:
> * 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.
> (libnode)[arguments]: Adjust to delete 'patch-nested-shebangs rather
> than 'patch-npm-shebang and 'patch-node-shebang.
> * guix/build-system/node.scm (lower): Add optional #:python argument
> and corresponding implicit input. Add the version of libuv used
> as an input to the #:node package as a new implicit input.
> * guix/build/node-build-system.scm (set-node-gyp-paths): New
> function. Sets the "npm_config_nodedir" and "npm_config_python"
> environment variables. Adds the "node-gyp-bin" directory to "PATH".
> (configure-gyp): New function. Run `node-gyp configure` if we see
> a `binding.gyp` file.
> (%standard-phases): Add 'set-node-gyp-paths after 'set-paths.
> Add 'configure-gyp after 'configure.
>
> Co-authored-by: Pierre Langlois <pierre.langlois <at> gmx.com>
Looking at this patch, it does *a lot* at once and could probably be
separated into more than one. Particularly, I'd suggest providing
capabilities in node-build-system first, then switching over to the new
thing in node.
> [...]
> --- 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 <at> fsfe.org>
> ;;; Copyright © 2019 Timothy Sample <samplet <at> ngyro.com>
> +;;; Copyright © 2021 Pierre Langlois <pierre.langlois <at> gmx.com>
> +;;; Copyright © 2021 Philip McGrath <philip <at> philipmcgrath.com>
> ;;;
> ;;; This file is part of GNU Guix.
> ;;;
> @@ -24,6 +26,7 @@ (define-module (guix build-system node)
> #: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 (default-node)
> (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
> @@ -58,10 +62,13 @@ (define private-keywords
> `(("source" ,source))
> '())
> ,@inputs
> -
> ;; Keep the standard inputs of 'gnu-build-
> system'.
> ,@(standard-packages)))
> (build-inputs `(("node" ,node)
> + ("python" ,python)
> + ;; We don't always need libuv, but the libuv
> and
> + ;; node versions need to match:
> + ("libuv" ,@(assoc-ref (package-inputs node)
> "libuv"))
> ,@native-inputs))
> (outputs outputs)
> (build node-build)
Will this python input always or often enough be needed? What's the
build system ratio on node like, gyp vs. anything else, particularly
with packages close to the node core?
> diff --git a/guix/build/node-build-system.scm b/guix/build/node-
> build-system.scm
> index 70a367618e..6aeb0149dd 100644
> --- a/guix/build/node-build-system.scm
> +++ b/guix/build/node-build-system.scm
> @@ -2,6 +2,8 @@
> ;;; Copyright © 2015 David Thompson <davet <at> gnu.org>
> ;;; Copyright © 2016, 2020 Jelle Licht <jlicht <at> fsfe.org>
> ;;; Copyright © 2019, 2021 Timothy Sample <samplet <at> ngyro.com>
> +;;; Copyright © 2021 Pierre Langlois <pierre.langlois <at> gmx.com>
> +;;; Copyright © 2021 Philip McGrath <philip <at> philipmcgrath.com>
> ;;;
> ;;; This file is part of GNU Guix.
> ;;;
> @@ -46,6 +48,19 @@ (define (set-home . _)
> (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"))
> + (setenv "PATH"
> + (string-append (getenv "PATH")
> + ":"
> + ;; Put this at the end to make it easier to
> override,
> + ;; just in case that should ever be
> necessary:
> + (assoc-ref inputs "node")
> + "/lib/node_modules/npm/bin/node-gyp-bin"))
> + #t)
> +
Is this a necessary step to build node modules? If not can we skip
this step when packages don't need gyp?
> (define (module-name module)
> (let* ((package.json (string-append module "/package.json"))
> (package-meta (call-with-input-file package.json read-
> json)))
> @@ -101,6 +116,12 @@ (define* (configure #:key outputs inputs
> #:allow-other-keys)
> (invoke npm "--offline" "--ignore-scripts" "install")
> #t))
>
> +(define* (configure-gyp #:key inputs #:allow-other-keys)
> + "Run 'node-gyp configure' if we see a 'binding.gyp' file."
> + (if (file-exists? "binding.gyp")
> + (invoke (which "node-gyp") "configure")
> + #t))
> +
You might want to make this part of configure itself, though I'm not
sure what the consensus is there when mixing different build system
styles. (invoke (which ...) ) is also a pretty rare pattern, used in
only four locations so far.
Also, while better than the previous thing in that it actually checks
whether we have something gyp-esque at hand, I'd still prefer users
being able to not run this portion through some flag. See e.g. #:use-
setuptools? in python or #:glib-or-gtk? in meson.
Cheers
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.