diff mbox series

[bug#51838,v2,10/26] gnu: node-llparse-builder-bootstrap: Use #:absent-dependencies.

Message ID 20211120043406.952350-10-philip@philipmcgrath.com
State New
Headers show
Series [bug#51838,v2,01/26] gnu: node: Avoid duplicating build phases. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Philip McGrath Nov. 20, 2021, 4:33 a.m. UTC
gnu/packages/node.scm (node-llparse-builder-bootstrap)[arguments]: Add
`#:absent-dependencies`. Stop deleting the `'configure` phase.
Add a new phase `#:delete-package-lock` to remove the
problematic "package-lock.json".
---
 gnu/packages/node.scm | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Liliana Marie Prikler Nov. 20, 2021, 7:44 a.m. UTC | #1
Am Freitag, den 19.11.2021, 23:33 -0500 schrieb Philip McGrath:
> gnu/packages/node.scm (node-llparse-builder-bootstrap)[arguments]:
> Add `#:absent-dependencies`. Stop deleting the `'configure` phase.
> Add a new phase `#:delete-package-lock` to remove the
> problematic "package-lock.json".
> ---
>  gnu/packages/node.scm | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
> index 98a51276e7..9d4903a8ca 100644
> --- a/gnu/packages/node.scm
> +++ b/gnu/packages/node.scm
> @@ -479,9 +479,21 @@ (define-public node-llparse-builder-bootstrap
>      (arguments
>       `(#:node ,node-bootstrap
>         #:tests? #f
> +       #:absent-dependencies
> +       `("@types/mocha"
> +         "@types/node"
> +         "mocha"
> +         "ts-node"
> +         "tslint"
> +         "typescript")
>         #:phases
>         (modify-phases %standard-phases
> -         (delete 'configure)
> +         (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")))
I think a simple delete-file ought to work.  This should also be done
by configure or similar, compare cargo-build-system.
Philip McGrath Nov. 20, 2021, 5:09 p.m. UTC | #2
On 11/20/21 02:44, Liliana Marie Prikler wrote:
> Am Freitag, den 19.11.2021, 23:33 -0500 schrieb Philip McGrath:
>>          (modify-phases %standard-phases
>> -         (delete 'configure)
>> +         (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")))
> I think a simple delete-file ought to work.  This should also be done
> by configure or similar, compare cargo-build-system.

Yes, I thought about having the build system automatically delete 
"package-lock.json", but I'm not 100% sure it's always a problem to have 
it, or if there might even be some circumstance where we want to keep 
it. I'd prefer to wait until we see a significant number of node 
packages deleting their "package-lock.json" files before trying to 
abstract over the pattern.
Jelle Licht Nov. 23, 2021, 11:04 a.m. UTC | #3
Hey Philip,

Philip McGrath <philip@philipmcgrath.com> writes:

> gnu/packages/node.scm (node-llparse-builder-bootstrap)[arguments]: Add
> `#:absent-dependencies`. Stop deleting the `'configure` phase.
> Add a new phase `#:delete-package-lock` to remove the
> problematic "package-lock.json".
> ---
>  gnu/packages/node.scm | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
> index 98a51276e7..9d4903a8ca 100644
> --- a/gnu/packages/node.scm
> +++ b/gnu/packages/node.scm
> @@ -479,9 +479,21 @@ (define-public node-llparse-builder-bootstrap
>      (arguments
>       `(#:node ,node-bootstrap
>         #:tests? #f
> +       #:absent-dependencies
> +       `("@types/mocha"
> +         "@types/node"
> +         "mocha"
> +         "ts-node"
> +         "tslint"
> +         "typescript")
>         #:phases
>         (modify-phases %standard-phases
> -         (delete 'configure)
> +         (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.

package-lock.json lists exact versions _and integrity hashes_; since it
seems unlikely that after node-build-system's finaggling we end up with
an identical hash, we will always have a mismatch and fetch 'proper'
sources online accordingly. As far as npm + package-lock.json are
concerned, we don't have them properly installed.

From what I have seen package-lock.json offers us no benefits (because
we track exact dependency information via the guix store) and can (as
you have seen) prevent builds from working. My 2c: always remove it in a
phase in the build system. It's simply a preference though, so your
please go with what you think is the right choice. You might want to
update the comment nonetheless so it's clear we know what's going on.

> +           (lambda args
> +             (delete-file-recursively "package-lock.json")))
>           (replace 'build
>             (lambda* (#:key inputs #:allow-other-keys)
>               (let ((esbuild (string-append (assoc-ref inputs "esbuild")
> -- 
> 2.32.0
diff mbox series

Patch

diff --git a/gnu/packages/node.scm b/gnu/packages/node.scm
index 98a51276e7..9d4903a8ca 100644
--- a/gnu/packages/node.scm
+++ b/gnu/packages/node.scm
@@ -479,9 +479,21 @@  (define-public node-llparse-builder-bootstrap
     (arguments
      `(#:node ,node-bootstrap
        #:tests? #f
+       #:absent-dependencies
+       `("@types/mocha"
+         "@types/node"
+         "mocha"
+         "ts-node"
+         "tslint"
+         "typescript")
        #:phases
        (modify-phases %standard-phases
-         (delete 'configure)
+         (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")