Philip McGrath 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 > --- > 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 > ;;; Copyright © 2019 Timothy Sample > +;;; Copyright © 2021 Pierre Langlois > +;;; Copyright © 2021 Philip McGrath > ;;; > ;;; 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 > ;;; Copyright © 2016, 2020 Jelle Licht > ;;; Copyright © 2019, 2021 Timothy Sample > +;;; Copyright © 2021 Pierre Langlois > +;;; Copyright © 2021 Philip McGrath > ;;; > ;;; 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