diff mbox series

[bug#51838,v3,06/43] guix: node-build-system: Add #:absent-dependencies argument.

Message ID 20211208202838.752542-7-philip@philipmcgrath.com
State Accepted
Headers show
Series guix: node-build-system: Support compiling add-ons with node-gyp. | expand

Commit Message

Philip McGrath Dec. 8, 2021, 8:28 p.m. UTC 
* guix/build-system/node.scm (lower, node-build): Add optional
argument #:absent-dependencies with default of ''(). Pass it on
to the build-side code.
* guix/build/node-build-system.scm (patch-dependencies): Respect
the #:absent-dependencies argument, removing specified npm
packages from the "dependencies" or "devDependencies" tables
in "package.json".
---
 guix/build-system/node.scm       | 19 ++++++++++++++++++-
 guix/build/node-build-system.scm |  7 +++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

Comments

Pierre Langlois Dec. 12, 2021, 3:31 p.m. UTC | #1 
Philip McGrath <philip@philipmcgrath.com> writes:

> * guix/build-system/node.scm (lower, node-build): Add optional
> argument #:absent-dependencies with default of ''(). Pass it on
> to the build-side code.
> * guix/build/node-build-system.scm (patch-dependencies): Respect
> the #:absent-dependencies argument, removing specified npm
> packages from the "dependencies" or "devDependencies" tables
> in "package.json".

Nice, I like this new option, I've needed this functionnality before as
well. For instance I've seen dependencies like "prebuild-install" [0]
for which the whole purepose is to download binaries instead of building
them... in Guix we always want to remove those dependencies.

I'd suggest to add documentation for this new option in the manual, but
that could be done as a follow-up.

[0]: https://www.npmjs.com/package/prebuild-install

Thanks,
Pierre
Philip McGrath Dec. 12, 2021, 8:22 p.m. UTC | #2 
On 12/12/21 10:31, Pierre Langlois wrote:
> 
> Philip McGrath <philip@philipmcgrath.com> writes:
> 
>> * guix/build-system/node.scm (lower, node-build): Add optional
>> argument #:absent-dependencies with default of ''(). Pass it on
>> to the build-side code.
>> * guix/build/node-build-system.scm (patch-dependencies): Respect
>> the #:absent-dependencies argument, removing specified npm
>> packages from the "dependencies" or "devDependencies" tables
>> in "package.json".
> 
> Nice, I like this new option, I've needed this functionnality before as
> well. For instance I've seen dependencies like "prebuild-install" [0]
> for which the whole purepose is to download binaries instead of building
> them... in Guix we always want to remove those dependencies.
> 
> I'd suggest to add documentation for this new option in the manual, but
> that could be done as a follow-up.

I agree documentation would be good: I don't think there's any for 
node-build-system at all, yet. I haven't written texinfo before, but I 
may give it a try as a follow-up.

-Philip
diff mbox series

Patch

diff --git a/guix/build-system/node.scm b/guix/build-system/node.scm
index 98f63f87ef..4f437f9c0d 100644
--- a/guix/build-system/node.scm
+++ b/guix/build-system/node.scm
@@ -44,6 +44,7 @@  (define (default-node)
 (define* (lower name
                 #:key source inputs native-inputs outputs system target
                 (node (default-node))
+                (absent-dependencies ''())
                 #:allow-other-keys
                 #:rest arguments)
   "Return a bag for NAME."
@@ -73,6 +74,7 @@  (define* (node-build store name inputs
                      (tests? #t)
                      (phases '(@ (guix build node-build-system)
                                  %standard-phases))
+                     (absent-dependencies ''())
                      (outputs '("out"))
                      (search-paths '())
                      (system (%current-system))
@@ -80,7 +82,21 @@  (define* (node-build store name inputs
                      (imported-modules %node-build-system-modules)
                      (modules '((guix build node-build-system)
                                 (guix build utils))))
-  "Build SOURCE using NODE and INPUTS."
+  "Build SOURCE using NODE and INPUTS.
+
+The builder will remove Node.js packages listed in ABSENT-DEPENCENCIES from
+the 'package.json' file's 'dependencies' and 'devDependencies' tables.  This
+mechanism can be used both avoid dependencies we don't want (e.g. optional
+features that would increase closure size) and to work around dependencies
+that haven't been packaged for Guix yet (e.g. test utilities)."
+  ;; Before #:absent-dependencies existed, this scenario was often handled by
+  ;; deleting the 'configure phase. Using #:absent-dependencies, instead,
+  ;; retains the check that no dependencies are silently missing and other
+  ;; actions performed by 'npm install', such as building native
+  ;; addons. Having an explicit list of absent dependencies in the package
+  ;; definition should also facilitate future maintenence: for example, if we
+  ;; add a package for a test framework, it should be easy to find all the
+  ;; other packages that use it and enable their tests.
   (define builder
     `(begin
        (use-modules ,@modules)
@@ -94,6 +110,7 @@  (define builder
                    #:test-target ,test-target
                    #:tests? ,tests?
                    #:phases ,phases
+                   #:absent-dependencies ,absent-dependencies
                    #:outputs %outputs
                    #:search-paths ',(map search-path-specification->sexp
                                          search-paths)
diff --git a/guix/build/node-build-system.scm b/guix/build/node-build-system.scm
index 6fade54670..249b3deee6 100644
--- a/guix/build/node-build-system.scm
+++ b/guix/build/node-build-system.scm
@@ -69,7 +69,8 @@  (define (list-modules directory)
               input-paths)
     index))
 
-(define* (patch-dependencies #:key inputs #:allow-other-keys)
+(define* (patch-dependencies #:key inputs absent-dependencies
+                             #:allow-other-keys)
 
   (define index (index-modules (map cdr inputs)))
 
@@ -86,7 +87,9 @@  (define (resolve-dependencies meta-alist meta-key)
       (('@ . orig-deps)
        (fold (match-lambda*
                (((key . value) acc)
-                (acons key (hash-ref index key value) acc)))
+                (if (member key absent-dependencies)
+                    acc
+                    (acons key (hash-ref index key value) acc))))
              '()
              orig-deps))))