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

From: Jelle Licht <[email-masked]>
Date: Mon, 24 Mar 2025 22:24:11 +0100

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 <[email-masked]> 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