* Re: 01/01: guix: node-build-system: Use guile-json instead of a custom parser. [not found] ` <20190714125821.1CA192088F@vcs0.savannah.gnu.org> @ 2019-07-14 13:40 ` Ludovic Courtès 2019-07-14 18:19 ` Julien Lepiller 0 siblings, 1 reply; 5+ messages in thread From: Ludovic Courtès @ 2019-07-14 13:40 UTC (permalink / raw) To: guix-devel, Julien Lepiller Hi Julien, guix-commits@gnu.org skribis: > commit 8eb0ba532ebbebef23180e666e0607ea735f9c1a > Author: Julien Lepiller <julien@lepiller.eu> > 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’. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: 01/01: guix: node-build-system: Use guile-json instead of a custom parser. 2019-07-14 13:40 ` 01/01: guix: node-build-system: Use guile-json instead of a custom parser Ludovic Courtès @ 2019-07-14 18:19 ` Julien Lepiller 2019-07-14 18:27 ` Robert Vollmert 0 siblings, 1 reply; 5+ messages in thread From: Julien Lepiller @ 2019-07-14 18:19 UTC (permalink / raw) To: Ludovic Courtès; +Cc: guix-devel Le Sun, 14 Jul 2019 15:40:43 +0200, Ludovic Courtès <ludo@gnu.org> a écrit : > Hi Julien, > > guix-commits@gnu.org skribis: > > > commit 8eb0ba532ebbebef23180e666e0607ea735f9c1a > > Author: Julien Lepiller <julien@lepiller.eu> > > 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’. OK, sorry for pushing this patch then... I've reverted it in a4bb18921099b2ec8c1699e08a73ca0fa78d0486. Thanks for the explanation! ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: 01/01: guix: node-build-system: Use guile-json instead of a custom parser. 2019-07-14 18:19 ` Julien Lepiller @ 2019-07-14 18:27 ` Robert Vollmert 2019-07-15 12:25 ` Ludovic Courtès 0 siblings, 1 reply; 5+ messages in thread From: Robert Vollmert @ 2019-07-14 18:27 UTC (permalink / raw) To: guix-devel On 14. Jul 2019, at 20:19, Julien Lepiller <julien@lepiller.eu> wrote: > > Le Sun, 14 Jul 2019 15:40:43 +0200, > Ludovic Courtès <ludo@gnu.org> a écrit : >> 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’. > > OK, sorry for pushing this patch then... I've reverted it in > a4bb18921099b2ec8c1699e08a73ca0fa78d0486. Thanks for the explanation! It’s a bit my fault, for remarking on the fact that the cargo build system does use guile-json. I suppose that’s not worth fixing either at this point? Cheers Robert ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: 01/01: guix: node-build-system: Use guile-json instead of a custom parser. 2019-07-14 18:27 ` Robert Vollmert @ 2019-07-15 12:25 ` Ludovic Courtès 2019-07-15 19:46 ` Robert Vollmert 0 siblings, 1 reply; 5+ messages in thread From: Ludovic Courtès @ 2019-07-15 12:25 UTC (permalink / raw) To: Robert Vollmert; +Cc: guix-devel Hi, Robert Vollmert <rob@vllmrt.net> skribis: > It’s a bit my fault, for remarking on the fact that the cargo build > system does use guile-json. I suppose that’s not worth fixing either > at this point? Indeed, ‘cargo-build-system’ has the same problem: --8<---------------cut here---------------start------------->8--- (define %cargo-build-system-modules ;; Build-side modules imported by default. `((guix build cargo-build-system) (json parser) ,@%cargo-utils-modules)) --8<---------------cut here---------------end--------------->8--- We should definitely fix it, or we’ll run into discrepancies like I described. For now, the easiest option would be to use (guix build json) as well. Could you look into it, Robert? Thanks, Ludo’. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: 01/01: guix: node-build-system: Use guile-json instead of a custom parser. 2019-07-15 12:25 ` Ludovic Courtès @ 2019-07-15 19:46 ` Robert Vollmert 0 siblings, 0 replies; 5+ messages in thread From: Robert Vollmert @ 2019-07-15 19:46 UTC (permalink / raw) To: Ludovic Courtès; +Cc: guix-devel > On 15. Jul 2019, at 14:25, Ludovic Courtès <ludo@gnu.org> wrote: > > Hi, > > Robert Vollmert <rob@vllmrt.net> skribis: > >> It’s a bit my fault, for remarking on the fact that the cargo build >> system does use guile-json. I suppose that’s not worth fixing either >> at this point? > > Indeed, ‘cargo-build-system’ has the same problem: > > --8<---------------cut here---------------start------------->8--- > (define %cargo-build-system-modules > ;; Build-side modules imported by default. > `((guix build cargo-build-system) > (json parser) > ,@%cargo-utils-modules)) > --8<---------------cut here---------------end--------------->8--- > > We should definitely fix it, or we’ll run into discrepancies like I > described. > > For now, the easiest option would be to use (guix build json) as well. > > Could you look into it, Robert? Done: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=36676 (Was the cargo-build-system not even working a known issue? I don’t see a bug report…) ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-15 19:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20190714125820.15568.58684@vcs0.savannah.gnu.org>
[not found] ` <20190714125821.1CA192088F@vcs0.savannah.gnu.org>
2019-07-14 13:40 ` 01/01: guix: node-build-system: Use guile-json instead of a custom parser Ludovic Courtès
2019-07-14 18:19 ` Julien Lepiller
2019-07-14 18:27 ` Robert Vollmert
2019-07-15 12:25 ` Ludovic Courtès
2019-07-15 19:46 ` Robert Vollmert
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/guix.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).