Re: 01/01: guix: node-build-system: Use guile-json instead of a custom parser.

[Ludovic Courtès]

Hi Julien,

address@hidden skribis:

> commit 8eb0ba532ebbebef23180e666e0607ea735f9c1a
> Author: Julien Lepiller <address@hidden>
> Date:   Sun Jul 14 14:50:21 2019 +0200
>
>     guix: node-build-system: Use guile-json instead of a custom parser.
>     
>     * guix/build/json.scm: Remove file.
>     * Makefile.am: Remove it.
>     * guix/build/node-build-system.scm: Use (json parser) instead of (guix 
> build json).
>     * guix/build-system/node.scm: Idem.

This commit log doesn’t spell out all the variables and entities that
were modified.  Please consider doing it next time, for clarity.

> +++ b/guix/build-system/node.scm
> @@ -18,7 +18,6 @@
>  
>  (define-module (guix build-system node)
>    #:use-module (guix store)
> -  #:use-module (guix build json)
>    #:use-module (guix build union)
>    #:use-module (guix utils)
>    #:use-module (guix packages)
> @@ -27,6 +26,7 @@
>    #:use-module (guix build-system)
>    #:use-module (guix build-system gnu)
>    #:use-module (ice-9 match)
> +  #:use-module (json parser)
>    #:export (npm-meta-uri
>              %node-build-system-modules
>              node-build
> @@ -40,8 +40,8 @@ registry."
>  (define %node-build-system-modules
>    ;; Build-side modules imported by default.
>    `((guix build node-build-system)
> -    (guix build json)
>      (guix build union)
> +    (json parser)

The effect of this change is to import the (json parser) from the host
side into the build side.

As a result, if I have installed Guile-JSON 1.2 and you have Guile-JSON
3.1, we’ll end up building different derivations (and one of them won’t
build :-)).

The solution here would be to do the equivalent of ‘with-extensions’ for
gexps.

However, given that that’s annoying to do without gexps, and given that
the plan is to move build systems to gexps Real Soon, I’d be in favor of
simply reverting this commit and using the custom JSON parser.  We can
add a TODO/FIXME explaining that we’ll replace it with Guile-JSON as
soon as build systems are rewritten to use gexps.

How does that sound?

Apologies if I overlooked it in your initial patch submission!

Thank you,
Ludo’.