bug#38628: Character literals unsupported in source 'snippets'

bug#38628: Character literals unsupported in source 'snippets'

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

Guix,

If you try to use a character literal in a source 'snippet', Guix will
complain about an "unsupported input".  I.e. adding the following to the
'hello' package definition:

              (snippet '(begin #\!))

Results in:

guix build: error: /home/marius/guix/gnu/packages/base.scm:73:2: package `hello@2.10' has an invalid input: #\!

This is a fairly recent regression.  I tracked it down to the following commit:

commit 24ab804ce11fe12ff49cd144a3d9c4bfcf55b41c
Author: Ludovic Courtès <ludo@gnu.org>
Date:   Mon Sep 23 22:17:39 2019 +0200

    gexp: Catch and report non-self-quoting gexp inputs.
    
    Previously we would, for example, generate build scripts in the store;
    when trying to run them, we'd get a 'read' error due to the presence
    of #<foo> syntax in there.
    
    * guix/gexp.scm (gexp->sexp)[self-quoting?]: New procedure.
    [reference->sexp]: Check whether the argument in a <gexp-input> box is
    self-quoting.  Raise a '&gexp-input-error' condition if it's not.
    * tests/gexp.scm ("lower-gexp, non-self-quoting input"): New test.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#38628: [PATCH] gexp: Allow character literals in GEXP->SEXP.

[Marius Bakke]

Fixes <https://bugs.gnu.org/38628>

* guix/gexp.scm (gexp->sexp)[self-quoting?]: Add CHAR? to the tested types.
---
 guix/gexp.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guix/gexp.scm b/guix/gexp.scm
index a96592ac76..411f0844ff 100644
--- a/guix/gexp.scm
+++ b/guix/gexp.scm
@@ -1028,7 +1028,7 @@ and in the current monad setting (system type, etc.)"
                                (or (pred x)
                                    (one-of rest ...))))))
       (one-of symbol? string? keyword? pair? null? array?
-              number? boolean?)))
+              number? boolean? char?)))
 
   (define* (reference->sexp ref #:optional native?)
     (with-monad %store-monad
-- 
2.24.0

bug#38628: [PATCH] gexp: Allow character literals in GEXP->SEXP.

[Ludovic Courtès]

Hi!

Marius Bakke <mbakke@fastmail.com> skribis:

> Fixes <https://bugs.gnu.org/38628>
>
> * guix/gexp.scm (gexp->sexp)[self-quoting?]: Add CHAR? to the tested types.

Oops, good catch, LGTM!

Could you also fix the occurrences of the ‘one-of’ macro in (gnu tests)
and (guix repl)?

(I know, this shouldn’t be duplicated in the first place but (guix repl)
has to be self-contained…)

Thanks,
Ludo’.

bug#38628: [PATCH] gexp: Allow character literals in GEXP->SEXP.

[Ludovic Courtès]

Also it would be nice to add a tiny test close to the one that
24ab804ce11fe12ff49cd144a3d9c4bfcf55b41c added.

Ludo’.

bug#38628: [PATCH] gexp: Allow character literals in GEXP->SEXP.

[Marius Bakke]

[-- Attachment #1.1: Type: text/plain, Size: 198 bytes --]

Ludovic Courtès <ludo@gnu.org> writes:

> Also it would be nice to add a tiny test close to the one that
> 24ab804ce11fe12ff49cd144a3d9c4bfcf55b41c added.

Good idea.  I came up with this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Type: text/x-patch, Size: 468 bytes --]

diff --git a/tests/gexp.scm b/tests/gexp.scm
index 84c16422c2..8b1596f66d 100644
--- a/tests/gexp.scm
+++ b/tests/gexp.scm
@@ -886,6 +886,12 @@
     (run-with-store %store
       (lower-gexp #~(foo #$+)))))
 
+(test-equal "lower-gexp, character literal"
+  '(#\+)
+  (lowered-gexp-sexp
+   (run-with-store %store
+     (lower-gexp #~(#\+)))))
+
 (test-assertm "gexp->derivation #:references-graphs"
   (mlet* %store-monad
       ((one (text-file "one" (random-text)))

[-- Attachment #1.3: Type: text/plain, Size: 7 bytes --]


WDYT?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#38628: [PATCH] gexp: Allow character literals in GEXP->SEXP.

[Marius Bakke]

[-- Attachment #1: Type: text/plain, Size: 998 bytes --]

Marius Bakke <mbakke@fastmail.com> writes:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Also it would be nice to add a tiny test close to the one that
>> 24ab804ce11fe12ff49cd144a3d9c4bfcf55b41c added.
>
> Good idea.  I came up with this:
>
> diff --git a/tests/gexp.scm b/tests/gexp.scm
> index 84c16422c2..8b1596f66d 100644
> --- a/tests/gexp.scm
> +++ b/tests/gexp.scm
> @@ -886,6 +886,12 @@
>      (run-with-store %store
>        (lower-gexp #~(foo #$+)))))
>  
> +(test-equal "lower-gexp, character literal"
> +  '(#\+)
> +  (lowered-gexp-sexp
> +   (run-with-store %store
> +     (lower-gexp #~(#\+)))))
> +
>  (test-assertm "gexp->derivation #:references-graphs"
>    (mlet* %store-monad
>        ((one (text-file "one" (random-text)))
>
> WDYT?

I ended up pushing this in ab7010af1f1077c056529769a53a380147c3933f,
even though I suspect there is an easier test that could reveal this
problem without involving the store, somewhere.  Improvements welcome.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

bug#38628: [PATCH] gexp: Allow character literals in GEXP->SEXP.

[Ludovic Courtès]

Hi Marius,

Marius Bakke <mbakke@fastmail.com> skribis:

> Marius Bakke <mbakke@fastmail.com> writes:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>>
>>> Also it would be nice to add a tiny test close to the one that
>>> 24ab804ce11fe12ff49cd144a3d9c4bfcf55b41c added.
>>
>> Good idea.  I came up with this:
>>
>> diff --git a/tests/gexp.scm b/tests/gexp.scm
>> index 84c16422c2..8b1596f66d 100644
>> --- a/tests/gexp.scm
>> +++ b/tests/gexp.scm
>> @@ -886,6 +886,12 @@
>>      (run-with-store %store
>>        (lower-gexp #~(foo #$+)))))
>>  
>> +(test-equal "lower-gexp, character literal"
>> +  '(#\+)
>> +  (lowered-gexp-sexp
>> +   (run-with-store %store
>> +     (lower-gexp #~(#\+)))))
>> +
>>  (test-assertm "gexp->derivation #:references-graphs"
>>    (mlet* %store-monad
>>        ((one (text-file "one" (random-text)))
>>
>> WDYT?
>
> I ended up pushing this in ab7010af1f1077c056529769a53a380147c3933f,
> even though I suspect there is an easier test that could reveal this
> problem without involving the store, somewhere.  Improvements welcome.

We could have called ‘gexp->sexp’ directly, but we’d still need the
store anyway, so IMO that’s good!

Thanks,
Ludo’.