diff mbox series

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

Message ID 20211208202838.752542-21-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/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(-)

Comments

Pierre Langlois Dec. 12, 2021, 4:09 p.m. UTC | #1
Philip McGrath <philip@philipmcgrath.com> 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
Philip McGrath Dec. 12, 2021, 9:26 p.m. UTC | #2
On 12/12/21 11:09, Pierre Langlois wrote:
> 
> Philip McGrath <philip@philipmcgrath.com> 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.

Right! I'll do that.

-Philip
diff mbox series

Patch

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")))
          (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)