| Message ID | 20211217020325.520821-21-philip@philipmcgrath.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | guix: node-build-system: Support compiling add-ons with node-gyp. | expand |
| 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 |
Am Donnerstag, dem 16.12.2021 um 21:03 -0500 schrieb Philip McGrath: > * guix/build-system/node.scm (lower): 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 | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/guix/build-system/node.scm b/guix/build-system/node.scm > index 330d10dca5..47af4bb9e2 100644 > --- a/guix/build-system/node.scm > +++ b/guix/build-system/node.scm > @@ -2,6 +2,8 @@ > ;;; Copyright © 2016 Jelle Licht <jlicht@fsfe.org> > ;;; Copyright © 2019 Timothy Sample <samplet@ngyro.com> > ;;; Copyright © 2021 Ludovic Courtès <ludo@gnu.org> > +;;; Copyright © 2021 Pierre Langlois <pierre.langlois@gmx.com> > +;;; Copyright © 2021 Philip McGrath <philip@philipmcgrath.com> > ;;; > ;;; This file is part of GNU Guix. > ;;; > @@ -62,10 +64,15 @@ (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. > + ("libuv" ,@(assoc-ref (package-inputs node) > "libuv")) > ,@native-inputs)) > (outputs outputs) > (build node-build) Do this and #21 have to be separated so far from the rest? If not, I'd do build system first, then new packages. Otherwise fair enough.
Hi, On 12/17/21 00:08, Liliana Marie Prikler wrote: > Am Donnerstag, dem 16.12.2021 um 21:03 -0500 schrieb Philip McGrath: >> * guix/build-system/node.scm (lower): 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 | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> > Do this and #21 have to be separated so far from the rest? If not, I'd > do build system first, then new packages. Otherwise fair enough. I tried to follow Tim's suggestion in <https://issues.guix.gnu.org/51838#59> to put the changes related to #:absent-dependencies before the changes to support native addons, so that the earlier changes could potentially be applied even if there was more discussion needed for the later ones (if #:absent-dependencies were less controversial. But note that the patches before this one aren't adding new packages; they are changing existing packages to use #:absent-dependencies rather than deleting the configure phase. So the series is ordered overall as: 1. Changes to the `node` package itself 2. Build system changes for #:absent-dependencies (including the delete-lockfiles phase, because un-deleting the configure phase exposes those problems) 3. Packages changes to use #:absent-dependencies 4. Build system changes to support native addons 5. New packages to exercise the support for native addons -Philip
Hi, Am Samstag, dem 18.12.2021 um 11:16 -0500 schrieb Philip McGrath: > > > > Do this and #21 have to be separated so far from the rest? If not, > > I'd do build system first, then new packages. Otherwise fair > > enough. > > I tried to follow Tim's suggestion in > <https://issues.guix.gnu.org/51838#59> to put the changes related to > #:absent-dependencies before the changes to support native addons, so > that the earlier changes could potentially be applied even if there > was more discussion needed for the later ones (if #:absent- > dependencies were less controversial. Fair enough, that does make sense. However, I do think that "add package X" is not too big of a review burden, so I personally think the fact we're deleting 'configure everywhere is holding back the change to support native addons rather than the other way around. > But note that the patches before this one aren't adding new packages; > they are changing existing packages to use #:absent-dependencies > rather than deleting the configure phase. So the series is ordered > overall as: > > 1. Changes to the `node` package itself > 2. Build system changes for #:absent-dependencies > (including the delete-lockfiles phase, because un-deleting the > configure phase exposes those problems) > 3. Packages changes to use #:absent-dependencies > 4. Build system changes to support native addons > 5. New packages to exercise the support for native addons There is an unspoken bit here in #5 in that those packages still need to get rid of unwanted dependencies, which makes this set still unsplittable in a sense. If everyone else here agrees, I think we could at least upstream the changes to node itself while we still discuss 2-5. Timothy, Pierre, Jelle, WDYT?
Hi again, On 12/17/21 00:08, Liliana Marie Prikler wrote: > Am Donnerstag, dem 16.12.2021 um 21:03 -0500 schrieb Philip McGrath: >> * guix/build-system/node.scm (lower): 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 | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> Tangentially, for the record, I'd considered as an alternative approach making libuv a propagated input of node. That might be worth considering one day, when we unbundle npm, but for now I think libuv is only needed in a small minority of cases. -Philip
Liliana Marie Prikler <liliana.prikler@gmail.com> writes: > Hi, > > Am Samstag, dem 18.12.2021 um 11:16 -0500 schrieb Philip McGrath: >> > >> > Do this and #21 have to be separated so far from the rest? If not, >> > I'd do build system first, then new packages. Otherwise fair >> > enough. >> >> I tried to follow Tim's suggestion in >> <https://issues.guix.gnu.org/51838#59> to put the changes related to >> #:absent-dependencies before the changes to support native addons, so >> that the earlier changes could potentially be applied even if there >> was more discussion needed for the later ones (if #:absent- >> dependencies were less controversial. > Fair enough, that does make sense. However, I do think that "add > package X" is not too big of a review burden, so I personally think the > fact we're deleting 'configure everywhere is holding back the change to > support native addons rather than the other way around. > >> But note that the patches before this one aren't adding new packages; >> they are changing existing packages to use #:absent-dependencies >> rather than deleting the configure phase. So the series is ordered >> overall as: >> >> 1. Changes to the `node` package itself >> 2. Build system changes for #:absent-dependencies >> (including the delete-lockfiles phase, because un-deleting the >> configure phase exposes those problems) >> 3. Packages changes to use #:absent-dependencies >> 4. Build system changes to support native addons >> 5. New packages to exercise the support for native addons > There is an unspoken bit here in #5 in that those packages still need > to get rid of unwanted dependencies, which makes this set still > unsplittable in a sense. > > If everyone else here agrees, I think we could at least upstream the > changes to node itself while we still discuss 2-5. Timothy, Pierre, > Jelle, WDYT? Agreed, thanks for asking.
Philip McGrath <philip@philipmcgrath.com> writes: > Hi again, > > On 12/17/21 00:08, Liliana Marie Prikler wrote: >> Am Donnerstag, dem 16.12.2021 um 21:03 -0500 schrieb Philip McGrath: >>> * guix/build-system/node.scm (lower): 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 | 9 ++++++++- >>> 1 file changed, 8 insertions(+), 1 deletion(-) >>> > > Tangentially, for the record, I'd considered as an alternative approach > making libuv a propagated input of node. That might be worth considering > one day, when we unbundle npm, but for now I think libuv is only needed > in a small minority of cases. Not trying to dismiss this suggestion without a constructive alternative, but I would prefer not having node's libuv be a propagated input. What we need is some kind of 'propagated-for-builds-only-input'. FWIW, I really like the current approach which works as long as we have npm bundled, which realistically will still be the case for some {weeks,months,years}. - Jelle
Am Sonntag, dem 19.12.2021 um 21:41 +0100 schrieb Jelle Licht: > Not trying to dismiss this suggestion without a constructive > alternative, but I would prefer not having node's libuv be a propagated > input. What we need is some kind of 'propagated-for-builds-only-input'. Note that I already made a similar suggestion w.r.t. pkg-config-based propagated inputs over at guix-devel[1, 2] (They're the same thread, use whichever layout you prefer). IMHO, adding it as implicit input to node-build-system as is done here works for me. Cheers [1] https://lists.gnu.org/archive/html/guix-devel/2021-09/msg00051.html [2] https://yhetil.org/guix-devel/045891c151c74e0d66d91973c9e55e0194272df5.camel@gmail.com/
diff --git a/guix/build-system/node.scm b/guix/build-system/node.scm index 330d10dca5..47af4bb9e2 100644 --- a/guix/build-system/node.scm +++ b/guix/build-system/node.scm @@ -2,6 +2,8 @@ ;;; Copyright © 2016 Jelle Licht <jlicht@fsfe.org> ;;; Copyright © 2019 Timothy Sample <samplet@ngyro.com> ;;; Copyright © 2021 Ludovic Courtès <ludo@gnu.org> +;;; Copyright © 2021 Pierre Langlois <pierre.langlois@gmx.com> +;;; Copyright © 2021 Philip McGrath <philip@philipmcgrath.com> ;;; ;;; This file is part of GNU Guix. ;;; @@ -62,10 +64,15 @@ (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. + ("libuv" ,@(assoc-ref (package-inputs node) "libuv")) ,@native-inputs)) (outputs outputs) (build node-build)