diff mbox series

[bug#51838,v2,18/26] guix: node-build-system: Add optional #:libuv? argument.

Message ID 20211120043406.952350-18-philip@philipmcgrath.com
State Accepted
Headers show
Series [bug#51838,v2,01/26] gnu: node: Avoid duplicating build phases. | 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

Commit Message

Philip McGrath Nov. 20, 2021, 4:33 a.m. UTC
* guix/build-system/node.scm (lower): Add an optional #:libuv?
argument to tell the build system to add the version of libuv
used as an input to the #:node package as an additional implicit
input, so that packages needing libuv always get the correct version.
---
 guix/build-system/node.scm | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Liliana Marie Prikler Nov. 20, 2021, 7:46 a.m. UTC | #1
Am Freitag, den 19.11.2021, 23:33 -0500 schrieb Philip McGrath:
> * guix/build-system/node.scm (lower): Add an optional #:libuv?
> argument to tell the build system to add the version of libuv
> used as an input to the #:node package as an additional implicit
> input, so that packages needing libuv always get the correct version.
Would it change something if we always had libuv as implicit input?  I
don't think it'd do much for closure size, but it might impact implicit
package search as proposed by my solution to the #:absent-dependencies
thing.
Philip McGrath Nov. 20, 2021, 5:16 p.m. UTC | #2
On 11/20/21 02:46, Liliana Marie Prikler wrote:
> Am Freitag, den 19.11.2021, 23:33 -0500 schrieb Philip McGrath:
>> * guix/build-system/node.scm (lower): Add an optional #:libuv?
>> argument to tell the build system to add the version of libuv
>> used as an input to the #:node package as an additional implicit
>> input, so that packages needing libuv always get the correct version.
> Would it change something if we always had libuv as implicit input?  I
> don't think it'd do much for closure size, but it might impact implicit
> package search as proposed by my solution to the #:absent-dependencies
> thing.

 From a Guix perspective, I don't think it would make much difference, 
since libuv is always needed by node itself. Maybe it would require more 
grafting than would be needed otherwise?

But I think the recommendation for authors of node add-ons is to avoid 
libuv unless it's really needed, because it has fewer stability 
guarantees that other Node.js APIs.
Timothy Sample Nov. 20, 2021, 7:55 p.m. UTC | #3
Hello,

Philip McGrath <philip@philipmcgrath.com> writes:

> On 11/20/21 02:46, Liliana Marie Prikler wrote:
>> Am Freitag, den 19.11.2021, 23:33 -0500 schrieb Philip McGrath:
>>> * guix/build-system/node.scm (lower): Add an optional #:libuv?
>>> argument to tell the build system to add the version of libuv
>>> used as an input to the #:node package as an additional implicit
>>> input, so that packages needing libuv always get the correct version.
>> Would it change something if we always had libuv as implicit input?  I
>> don't think it'd do much for closure size, but it might impact implicit
>> package search as proposed by my solution to the #:absent-dependencies
>> thing.
>
> From a Guix perspective, I don't think it would make much difference,
> since libuv is always needed by node itself. Maybe it would require
> more grafting than would be needed otherwise?
>
> But I think the recommendation for authors of node add-ons is to avoid
> libuv unless it's really needed, because it has fewer stability 
> guarantees that other Node.js APIs.

My assumption is that most packages would ignore the libuv headers, and
they wouldn’t retain a reference to it (except via Node.js itself).
Hence, I don’t think it would make any difference either to grafting or
closure size to just always add it.


-- Tim
Philip McGrath Dec. 2, 2021, 9:52 p.m. UTC | #4
On 11/20/21 14:55, Timothy Sample wrote:
> My assumption is that most packages would ignore the libuv headers, and
> they wouldn’t retain a reference to it (except via Node.js itself).
> Hence, I don’t think it would make any difference either to grafting or
> closure size to just always add it.
That makes sense to me.

-Philip
diff mbox series

Patch

diff --git a/guix/build-system/node.scm b/guix/build-system/node.scm
index 75ae34508f..f83a7f64ce 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.
 ;;;
@@ -44,12 +46,13 @@  (define (default-node)
 (define* (lower name
                 #:key source inputs native-inputs outputs system target
                 (node (default-node))
+                (libuv? #f)
                 (absent-dependencies ''())
                 #:allow-other-keys
                 #:rest arguments)
   "Return a bag for NAME."
   (define private-keywords
-    '(#:source #:target #:node #:inputs #:native-inputs))
+    '(#:source #:target #:node #:libuv? #:inputs #:native-inputs))
 
   (and (not target)                    ;XXX: no cross-compilation
        (bag
@@ -59,10 +62,18 @@  (define private-keywords
                               `(("source" ,source))
                               '())
                         ,@inputs
-
                         ;; Keep the standard inputs of 'gnu-build-system'.
                         ,@(standard-packages)))
          (build-inputs `(("node" ,node)
+                         ;; Many packages with native addons need
+                         ;; libuv headers. The libuv version must
+                         ;; be exactly the same as for the node
+                         ;; package we are adding implicitly,
+                         ;; so we take care of adding libuv, too.
+                         ,@(if libuv?
+                               `(("libuv" ,@(assoc-ref (package-inputs node)
+                                                       "libuv")))
+                               '())
                          ,@native-inputs))
          (outputs outputs)
          (build node-build)