[bug#77231,4/6] build-system/node: Add phase 'delete-unwanted-dev-dependencies.

  * guix/build/node-build-system.scm (npm-ignored-inputs): New variable.
(delete-dependencies): Extend procedure to accept a filtering
procedure.
(delete-unwanted-dev-dependencies): Define new phase to ignore all
npm-ignored-inputs systematically.
(%standard-phases): Add it here.
---
 guix/build/node-build-system.scm | 39 ++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)
  

Thanks for working all of this though, I really appreciate it all! I've
applied patches 1, 2, 3, and 5 locally, and after doing some sanity
checks over night, will be pushing it to the javascript-team

W.r.t patches 4 and 6;
A hardcoded list of ignored inputs as part of the build system seems
like it will tightly couple package expressions to a particular version
of guix. Changing it in the future (by adding or removing things) will
also be annoying for packages in guix, as well as generated package
expressions 'in the wilds'.

A similar argument was better framed in the discussion around
issues.guix.gnu.org/51838 (warning: long read). 

The idea of having the importer be able to effectively ignore a large
part of the huge dependency graph if we know it won't be useful to us is
a cool feature to have. Ideally it'd somehow be part of the generated
output as well though, so the generated package expressions run on more
revisions of guix.

Tangentially, how did you run into issues with this particular list of
deps?  Are they not often encountered as devDependencies (and
subsequently explicitly removed in the package expressions generated by
the importer)? Or is the point to minimise (generated) code duplication
between packages to remove these ne'er-needed-deps?

Thanks again!
- Jelle


Nicolas Graves via Guix-patches via <guix-patches@gnu.org> writes:

> * guix/build/node-build-system.scm (npm-ignored-inputs): New variable.
> (delete-dependencies): Extend procedure to accept a filtering
> procedure.
> (delete-unwanted-dev-dependencies): Define new phase to ignore all
> npm-ignored-inputs systematically.
> (%standard-phases): Add it here.
> ---
>  guix/build/node-build-system.scm | 39 ++++++++++++++++++++++++++++----
>  1 file changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/guix/build/node-build-system.scm b/guix/build/node-build-system.scm
> index 05940bc997..50e023b3ca 100644
> --- a/guix/build/node-build-system.scm
> +++ b/guix/build/node-build-system.scm
> @@ -29,6 +29,7 @@ (define-module (guix build node-build-system)
>    #:use-module (ice-9 match)
>    #:use-module (json)
>    #:use-module (srfi srfi-1)
> +  #:use-module (srfi srfi-26)
>    #:use-module (srfi srfi-71)
>    #:export (%standard-phases
>              delete-dependencies
> @@ -37,9 +38,21 @@ (define-module (guix build node-build-system)
>              modify-json
>              modify-json-fields
>              node-build
> +            npm-ignored-inputs
>              replace-fields
>              with-atomic-json-file-replacement))
>  
> +(define npm-ignored-inputs
> +  (list
> +   (list "aud" "nsp" ; passed end-of-life
> +         "covert" ; code coverage
> +         "auto-changelog" "npmignore" "evalmd" ; development tools
> +         "eclint" "eslint" "prettier-standard" "standard" ; lint
> +         "in-publish" "np" "safe-publish-latest") ; upload integration tools
> +   ;; Second value is a list of prefixes to ignore
> +   ;; Handy for personal configs and extensions of ignored inputs
> +   (list "@ljharb/" "eslint-")))
> +
>  (define* (assoc-ref* alist key #:optional default)
>    "Like assoc-ref, but return DEFAULT instead of #f if no value exists."
>    (match (assoc key alist)
> @@ -100,7 +113,7 @@ (define* (modify-json #:key (file "package.json") #:rest all-arguments)
>            modifications))
>        file)))
>  
> -(define (delete-dependencies dependencies-to-remove)
> +(define (delete-dependencies predicate-or-dependencies)
>    "Rewrite 'package.json' to allow the build to proceed without packages
>  listed in 'dependencies-to-remove', a list of strings naming npm packages.
>  
> @@ -114,9 +127,13 @@ (define (delete-dependencies dependencies-to-remove)
>            dependency-key
>            (lambda (dependencies)
>              (remove
> -              (lambda (dependency)
> -                (member (car dependency) dependencies-to-remove))
> -              dependencies))))
> +             (match predicate-or-dependencies
> +               ((? procedure? predicate)
> +                predicate)
> +               ((? list? dependencies-to-remove)
> +                (lambda (dependency)
> +                  (member (car dependency) dependencies-to-remove))))
> +             dependencies))))
>        pkg-meta
>        (list
>          "devDependencies"
> @@ -399,11 +416,25 @@ (define scripts
>                                    "echo Guix: avoiding node-gyp rebuild"))
>            out)))))
>  
> +(define delete-unwanted-dev-dependencies
> +  (lambda* args
> +    (modify-json
> +     (delete-dependencies
> +      (lambda (input)
> +        (match npm-ignored-inputs
> +          (((ignored ...) (prefixes ...) . ())
> +           (or (member (car input) ignored)
> +               (any (cut string-prefix? <> (car input))
> +                    prefixes)))
> +          (_ #f)))))))
> +
>  (define %standard-phases
>    (modify-phases gnu:%standard-phases
>      (add-after 'unpack 'set-home set-home)
>      (add-before 'configure 'patch-dependencies patch-dependencies)
>      (add-after 'patch-dependencies 'delete-lockfiles delete-lockfiles)
> +    (add-after 'patch-dependencies 'delete-unwanted-dev-dependencies
> +      delete-unwanted-dev-dependencies)
>      (replace 'configure configure)
>      (replace 'build build)
>      (replace 'check check)
> -- 
> 2.48.1
  

On 2025-03-24 22:24, Jelle Licht wrote:

> Thanks for working all of this though, I really appreciate it all! I've
> applied patches 1, 2, 3, and 5 locally, and after doing some sanity
> checks over night, will be pushing it to the javascript-team
>
> W.r.t patches 4 and 6;
> A hardcoded list of ignored inputs as part of the build system seems
> like it will tightly couple package expressions to a particular version
> of guix. Changing it in the future (by adding or removing things) will
> also be annoying for packages in guix, as well as generated package
> expressions 'in the wilds'.
>
> A similar argument was better framed in the discussion around
> issues.guix.gnu.org/51838 (warning: long read).

I did the same for python in 68315.  Wasn't aware of that discussion
indeed.

A good general (not Node specific) altenative IMO would be to have
"development-inputs" that are NOT injected in the build/derivation
neither evaluated (when <package> is compiled) but whose names could be
made available as an argument to the build-system.  That makes it
uncessary to record them at the build-system level (although adds more
package boilerplate).

Python basically has the same issue with a lot of false/unrequired
native-inputs.  I think I heard Nix solves this with a concept similar
to what I describe.

> The idea of having the importer be able to effectively ignore a large
> part of the huge dependency graph if we know it won't be useful to us is
> a cool feature to have. Ideally it'd somehow be part of the generated
> output as well though, so the generated package expressions run on more
> revisions of guix.

Do you mean we should do that as a snippet or in an earlier phase?

> Tangentially, how did you run into issues with this particular list of
> deps?  Are they not often encountered as devDependencies (and
> subsequently explicitly removed in the package expressions generated by
> the importer)? Or is the point to minimise (generated) code duplication
> between packages to remove these ne'er-needed-deps?

The latter, but it felt needed for some reason, the sheer count of lines
and readability is consequential.

>
> Thanks again!
> - Jelle