Subject: [bug#51838] [PATCH v3 20/43] guix: node-build-system: Add delete-lockfiles phase.

From: Pierre Langlois <[email-masked]>
Date: Sun, 12 Dec 2021 16:09:59 +0000

Philip McGrath <[email-masked]> writes:

> * guix/build/node-build-system.scm (delete-lockfiles): New function.
> Remove 'package-lock.json', 'yarn.lock', and 'npm-shrinkwrap.json',
> if they exist.  Because these files specify dependency both exact
> versions and integrity hashes, they only cause problems for Guix.
> (%standard-phases): Add 'delete-lockfiles' after 'patch-dependencies'.
> ---
>  gnu/packages/node.scm            | 12 ------------
>  guix/build/node-build-system.scm | 12 ++++++++++++
>  2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
> index 6d48816c77..5289e2fe4f 100644
> --- a/gnu/packages/node.scm
> +++ b/gnu/packages/node.scm
> @@ -488,12 +488,6 @@ (define-public node-llparse-builder-bootstrap
>           "typescript")
>         #:phases
>         (modify-phases %standard-phases
> -         (add-before 'configure 'remove-package-lock
> -           ;; Having package-lock.json seems to cause npm
> -           ;; to look for things on the internet in the configure phase,
> -           ;; even if we have them properly installed.
> -           (lambda args
> -             (delete-file-recursively "package-lock.json")))
>           (replace 'build
>             (lambda* (#:key inputs #:allow-other-keys)
>               (let ((esbuild (string-append (assoc-ref inputs "esbuild")
> @@ -621,12 +615,6 @@ (define-public node-llparse-bootstrap
>           "typescript")
>         #:phases
>         (modify-phases %standard-phases
> -         (add-before 'configure 'remove-package-lock
> -           ;; Having package-lock.json seems to cause npm
> -           ;; to look for things on the internet in the configure phase,
> -           ;; even if we have them properly installed.
> -           (lambda args
> -             (delete-file-recursively "package-lock.json")))

These changes were added in this series right? I'd suggest to re-order
commits to have the build-system changes first so that they don't need
to be added at all.

>           (replace 'build
>             (lambda* (#:key inputs #:allow-other-keys)
>               (let ((esbuild (string-append (assoc-ref inputs "esbuild")
> diff --git a/guix/build/node-build-system.scm b/guix/build/node-build-system.scm
> index 249b3deee6..892104b6d2 100644
> --- a/guix/build/node-build-system.scm
> +++ b/guix/build/node-build-system.scm
> @@ -118,6 +118,17 @@ (define (resolve-dependencies meta-alist meta-key)
>          (write-json package-meta out))))
>    #t)
>  
> +(define* (delete-lockfiles #:key inputs #:allow-other-keys)
> +  "Delete 'package-lock.json', 'yarn.lock', and 'npm-shrinkwrap.json', if they
> +exist."
> +  (for-each (lambda (pth)
> +              (when (file-exists? pth)
> +                (delete-file pth)))
> +            '("package-lock.json"
> +              "yarn.lock"
> +              "npm-shrinkwrap.json"))
> +  #t)
> +
>  (define* (configure #:key outputs inputs #:allow-other-keys)
>    (let ((npm (string-append (assoc-ref inputs "node") "/bin/npm")))
>      (invoke npm "--offline" "--ignore-scripts" "install")
> @@ -168,6 +179,7 @@ (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)
>      (replace 'configure configure)
>      (replace 'build build)
>      (replace 'check check)

Otherwise LGTM!

Thanks,
Pierre