GNU bug report logs - #49671
[PATCH] guix: records: Improve error reporting.

Previous Next

To reply to this bug, email your comments to 49671 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Julien Lepiller <julien <at> lepiller.eu>
To: guix-patches <at> gnu.org
Subject: [PATCH] guix: records: Improve error reporting.
Date: Wed, 21 Jul 2021 01:40:47 +0200

[Message part 1 (text/plain, inline)]

Hi Guix!

This patch improves error reporting a bit when making mistakes in guix
records. This is motivated by a user getting "invalid field specifier"
for their whole services field in their os record. With this patches,
they would have seen:

multiple values in field specifier. Got 2 values associated with key
services. Values are:
(append (list (service ...) ...))
(modify-services %desktop-services ...)

Which would have hinted them at fixing the parenthesis. Or at least, it
would have saved us some time trying to count them :)

Here are the cases that are handled and the associated message:

(operating-system
  services)
guix system: error: services: invalid field specifier. The format of a
field is `(services value)'

(operating-system
  (services))
test.scm:2:2: error: (services): Value missing in field specifier. The
format of a field is `(services value)'.

(operating-system
  (services 1 2 3))
test.scm:2:2: error: (services 1 2 3): multiple values in field
specifier. Got 3 values associated with key services. Values are:
1
2
3

(operating-system
  ())
guix system: error: (): invalid field specifier. The format of a field
is `(field value)'

(operating-system
  ((services %desktop-services)))
test.scm:2:2: error: ((services %desktop-services)): invalid field
specifier. (services %desktop-services) is not a valid field name.


Of course, we can improve these error messages, and internationalize
them.

WDYT?

[0001-guix-records-Improve-error-reporting.patch (text/x-patch, attachment)]

Message #8 received at 49671 <at> debbugs.gnu.org (full text, mbox):

From: Sarah Morgensen <iskarian <at> mgsn.dev>
To: Julien Lepiller <julien <at> lepiller.eu>
Cc: 49671 <at> debbugs.gnu.org
Subject: Re: bug#49671: [PATCH] guix: records: Improve error reporting.
Date: Wed, 21 Jul 2021 12:21:23 -0700

Hi Julien,

Julien Lepiller <julien <at> lepiller.eu> writes:

> Hi Guix!
>
> This patch improves error reporting a bit when making mistakes in guix
> records. This is motivated by a user getting "invalid field specifier"
> for their whole services field in their os record. With this patches,
> they would have seen:

After applying your patch, I get:

--8<---------------cut here---------------start------------->8---
guix/records.scm:108:19: warning: "multiple values in field specifier. Got ~a values associated with key ~a. Values are:~%~{~a~%~}": unsupported format option ~{, use (ice-9 format) instead
--8<---------------cut here---------------end--------------->8---

After adding `(ice-9 format)` to imports it works as expected. I see
this also applies to package records! This will be great for those
starting to package in Guix.

>
> multiple values in field specifier. Got 2 values associated with key
> services. Values are:
> (append (list (service ...) ...))
> (modify-services %desktop-services ...)
>
> Which would have hinted them at fixing the parenthesis. Or at least, it
> would have saved us some time trying to count them :)
>
> Here are the cases that are handled and the associated message:
>
> (operating-system
>   services)
> guix system: error: services: invalid field specifier. The format of a
> field is `(services value)'
>
> (operating-system
>   (services))
> test.scm:2:2: error: (services): Value missing in field specifier. The
> format of a field is `(services value)'.
>
> (operating-system
>   (services 1 2 3))
> test.scm:2:2: error: (services 1 2 3): multiple values in field
> specifier. Got 3 values associated with key services. Values are:
                                              ^ Wrap in `'?
> 1
> 2
> 3
>
> (operating-system
>   ())
> guix system: error: (): invalid field specifier. The format of a field
> is `(field value)'
>
> (operating-system
>   ((services %desktop-services)))
> test.scm:2:2: error: ((services %desktop-services)): invalid field
> specifier. (services %desktop-services) is not a valid field name.
             ^ Should this also be wrapped in `'?

Why do some of these messages lose their context and come from `guix
system` instead?

>
> Of course, we can improve these error messages, and internationalize
> them.
>
> WDYT?

[...]

> -         (apply syntax-violation name "invalid field specifier"
> -                (if parent-form
> +         (syntax-case #'weird ()
> +           (()                             ;the empty list
> +             (apply syntax-violation name
> +                    "invalid field specifier. The format of a field is `(field value)'"
>                      (list parent-form #'weird)
> -                    (list #'weird)))))))
> +                    (list #'weird)))

Why the extra `(list #'weird')`? AFAICT right now this is providing
`(list parent-form #:'weird)` as the parent form.  And since parent-form
is optional, shouldn't this be

--8<---------------cut here---------------start------------->8---
  (apply syntax-violation name
         "invalid field specifier. The format of a field is `(field value)'"
         (if parent-form (list parent-form #:'weird) (list weird)))
--8<---------------cut here---------------end--------------->8---

(and similar for the others)?

--
Sarah

Message #11 received at 49671 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Julien Lepiller <julien <at> lepiller.eu>
Cc: 49671 <at> debbugs.gnu.org
Subject: Re: bug#49671: [PATCH] guix: records: Improve error reporting.
Date: Wed, 04 Aug 2021 17:19:47 +0200

Hello!

Julien Lepiller <julien <at> lepiller.eu> skribis:

> Here are the cases that are handled and the associated message:
>
> (operating-system
>   services)
> guix system: error: services: invalid field specifier. The format of a
> field is `(services value)'
>
> (operating-system
>   (services))
> test.scm:2:2: error: (services): Value missing in field specifier. The
> format of a field is `(services value)'.
>
> (operating-system
>   (services 1 2 3))
> test.scm:2:2: error: (services 1 2 3): multiple values in field
> specifier. Got 3 values associated with key services. Values are:
> 1
> 2
> 3
>
> (operating-system
>   ())
> guix system: error: (): invalid field specifier. The format of a field
> is `(field value)'
>
> (operating-system
>   ((services %desktop-services)))
> test.scm:2:2: error: ((services %desktop-services)): invalid field
> specifier. (services %desktop-services) is not a valid field name.
>
>
> Of course, we can improve these error messages, and internationalize
> them.

I like the idea!

I would prefer for error messages to be plain errors, without hints, so:

  test.scm:2:2: error: (services 1 2 3): multiple values in field specifier

(Not a sentence, no period.)

But I’d also like to have hints, and ideally all hints should be
reported consistently, via ‘&fix-hint’ or similar.

Thankfully, we can do that via (guix diagnostics) + SRFI-35/34:

  (raise (condition
           (&syntax (form 'x) (subform 'y))
           (&fix-hint (hint "Consider fixing this."))))

‘call-with-error-handling’ in (guix ui) might need to be adjusted.

The only downside is that (guix records) would now depend on another
Guix modules, which I tried to avoid so far so it could be more easily
reused elsewhere.  But that’s the price to pay to get consistent error
reports, and I think it’s okay.

Note that ‘tests/guix-system.sh’ and a couple of other files test exact
error reports, so we’ll have to make sure it still works and possibly
augment the tests.

WDYT?

Thanks,
Ludo’.

Message #14 received at 49671 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxime Devos <maximedevos <at> telenet.be>
Cc: 49671 <at> debbugs.gnu.org, 50914 <at> debbugs.gnu.org
Subject: Re: bug#50914: [PATCH] records: Raise a &fix-hint if a field has
 multiple values.
Date: Mon, 04 Oct 2021 16:26:33 +0200

Hi,

Maxime Devos <maximedevos <at> telenet.be> skribis:

> * guix/records.scm (report-invalid-field-specifier): If
>   'weird' is something like (field (record ...) extra ...), hint that 'extra
>   ...' should probably be moved inside (record ...).

Please see also <https://issues.guix.gnu.org/49671>.

> +               (condition
> +                (&origin (origin name))
> +                (&message (message
> +                           (format #f "field ‘~a’ should only have one \
> +value, but an extra value ‘~a’ was passed as well.  Perhaps this extra \
> +value was supposed to be a field specifier, and needs to be moved inside \
> +the record ‘~a’?"

We’ll need i18n here and straight quotes.

> +                (&syntax (form (car forms))
> +                         (subform (and (not (null? (cdr forms)))
> +                                       (cadr forms))))

No cadrcdr please.  :-)

> +                (&fix-hint (hint (object->string
> +                                  (syntax->datum
> +                                   #'(field
> +                                      (record-name fields ... extra-value
> +                                                   extra-value* ...)))))))))

The ‘hint’ field must be a string, typically the message you had above;
see other examples in the code.

TIA!

Ludo’.

Message #17 received at 49671 <at> debbugs.gnu.org (full text, mbox):

From: Julien Lepiller <julien <at> lepiller.eu>
To: 49671 <at> debbugs.gnu.org
Subject: [PATCH] guix: records: Improve error reporting.
Date: Sun, 31 Oct 2021 03:06:35 +0100

[Message part 1 (text/plain, inline)]

Hi Guix!

Here is a new patch that attempts to better catch various causes of
syntax errors in records. I think I fixed all the concerns Ludo had,
and I draw some inspiration from https://issues.guix.gnu.org/50914 for
using conditions.

Here are examples of what you get:

(operating-system
  ())

test.scm:1:0: error: invalid field specifier: ()
hint: The format of a field is `(field value)'



(operating-system
  ((services %base-services)))

test.scm:1:0: error: invalid field specifier: (services %base-services)
is not a valid field name



(operating-system
  (services))

test.scm:1:0: error: missing value in field specifier
hint: The field is missing a value: `(services <value>)'.



(operating-system
  (services (service tor-service-type) %base-services))

test.scm:1:0: error: multiple values in field specifier
hint: 2 values were associated with field `services'.  We got the
following values:

     (service tor-service-type)
     %base-services
     

Perhaps the additional values were intended to be other field
specifiers.  This usually indicates missing or misplaced parenthesis.



(operating-system
  services %base-services)

test.scm:1:0: error: invalid field specifier: #<syntax services>
hint: The format of a field is `(field value)'



(not sure why the last one is wrapped in syntax...)

[0001-guix-records-Improve-error-reporting.patch (text/x-patch, attachment)]

Message #20 received at 49671 <at> debbugs.gnu.org (full text, mbox):

From: Julien Lepiller <julien <at> lepiller.eu>
To: 49671 <at> debbugs.gnu.org
Subject: Re: [bug#49671] [PATCH v3] guix: records: Improve error reporting.
Date: Mon, 22 Nov 2021 03:40:22 +0100

[Message part 1 (text/plain, inline)]

Here is another improvement compared to v2. This time there are two
patches: the first adds support for &syntax in (guix ui), and will
print something like

in form: <pretty-printed-form>

where "in form" is in green.

The second patch is very similar to v2, but will now also raise a
&syntax condition, so it can be pretty-printed. The previous issue
where I printed #<syntax ...> is fixed, I simply forgot a syntax->datum.

WDYT?

[0001-guix-ui-Print-syntax-errors.patch (text/x-patch, attachment)]

[0002-guix-records-Improve-error-reporting.patch (text/x-patch, attachment)]

Message #23 received at 49671 <at> debbugs.gnu.org (full text, mbox):

From: Julien Lepiller <julien <at> lepiller.eu>
To: 49671 <at> debbugs.gnu.org
Subject: Re: [bug#49671] [PATCH v3] guix: records: Improve error reporting.
Date: Sun, 17 Jul 2022 08:23:39 +0200

Hi Guix!

A few months have passed without comment. I don't feel confident enough
to change Guix internals without comments.

What are your thoughts on this?

Le Mon, 22 Nov 2021 03:40:22 +0100,
Julien Lepiller <julien <at> lepiller.eu> a écrit :

> Here is another improvement compared to v2. This time there are two
> patches: the first adds support for &syntax in (guix ui), and will
> print something like
> 
> in form: <pretty-printed-form>
> 
> where "in form" is in green.
> 
> The second patch is very similar to v2, but will now also raise a
> &syntax condition, so it can be pretty-printed. The previous issue
> where I printed #<syntax ...> is fixed, I simply forgot a
> syntax->datum.
> 
> WDYT?

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.