[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#51838] [PATCH 03/11] guix: node-build-system: Support compiling add

From: Philip McGrath
Subject: [bug#51838] [PATCH 03/11] guix: node-build-system: Support compiling add-ons with node-gyp.
Date: Fri, 19 Nov 2021 23:26:05 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.1


On 11/14/21 15:44, Liliana Marie Prikler wrote:
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.

Thanks for these comments. Some of the things to which you drew my attention seem to have been workarounds for problems that had since been solved at a deeper level. Then, in particular, this comment:

>> +(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.

prompted me to look more closely at why so much manual work was needed in the first place.

It turns out that the `npm install` in the `'configure` phase should have handled most of it automatically, but the Guix packages were deleting the configure phase to avoid checking for devDependencies that aren't in Guix (or that we just don't want, e.g. some dependencies of node-sqlite3).

In v2 of this series (which will follow this email), I've removed all of the `node-gyp`-specific build-side code and tried a more general solution, adding an `#:absent-packages` argument to instruct the `'patch-dependencies` phase to remove the specified packages from the "package.json" file. This means that the Guix package can still run `'configure`/`npm install`, checking properly for the packages that we *do* have/want and doing all of the other automatic work `npm install` does. I also like that listing the missing packages in the Guix package definition provides a sort of documentation of what is missing: for example, it is clear which packages could have their tests enabled with the addition of a `node-tap` package, without having to inspect all of the individual "package.json" files.

I've updated all of the existing Node.js packages that deleted their `'configure` phase to use `#:absent-dependencies` instead.

@@ -58,10 +62,13 @@ (define private-keywords
                                `(("source" ,source))
                          ;; Keep the standard inputs of 'gnu-build-
           (build-inputs `(("node" ,node)
+                         ("python" ,python)
+                        ;; We don't always need libuv, but the libuv
+                        ;; node versions need to match:
+                        ("libuv" ,@(assoc-ref (package-inputs node)
           (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?

GYP is a Python program, and it (or at least node's fork of it) expects to have a python executable available to invoke with sub-programs. Since npm depends on node-gyp and GYP, it is pretty close to the core.

In v2, I've just had packages that use node-gyp add Python to their inputs. IIRC, that used to not be enough, but it seems underlying problems were fixed in the mean time.

An alternative approach would be to configure it using the npmrc file, as I do for `nodedir` in v2. I'm not sure that makes much difference either way, but in v2 I've tried to minimize the amount of `node-gyp`-specific handling.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]