GNU bug report logs - #50914
[PATCH] records: Raise a &fix-hint if a field has multiple values.

Previous Next

Package: guix-patches;

Reported by: Maxime Devos <maximedevos <at> telenet.be>

Date: Thu, 30 Sep 2021 09:33:01 UTC

Severity: normal

Tags: patch

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#50914; Package guix-patches. (Thu, 30 Sep 2021 09:33:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Maxime Devos <maximedevos <at> telenet.be>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Thu, 30 Sep 2021 09:33:01 GMT) Full text and rfc822 format available.

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

From: Maxime Devos <maximedevos <at> telenet.be>
To: guix-patches <at> gnu.org
Cc: Maxime Devos <maximedevos <at> telenet.be>
Subject: [PATCH] records: Raise a &fix-hint if a field has multiple values.
Date: Thu, 30 Sep 2021 11:32:29 +0200
* guix/records.scm (report-invalid-field-specifier): If
  'weird' is something like (field (record ...) extra ...), hint that 'extra
  ...' should probably be moved inside (record ...).
---
 guix/records.scm | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/guix/records.scm b/guix/records.scm
index ed94c83dac..db0c0a7ca0 100644
--- a/guix/records.scm
+++ b/guix/records.scm
@@ -1,6 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2018 Mark H Weaver <mhw <at> netris.org>
+;;; Copyright © 2021 Maxime Devos <maximedevos <at> telenet.be>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -21,10 +22,13 @@
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-9)
   #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-35)
+  #:use-module (ice-9 exceptions)
   #:use-module (ice-9 match)
   #:use-module (ice-9 regex)
   #:use-module (ice-9 rdelim)
   #:autoload (system base target) (target-most-positive-fixnum)
+  #:autoload (guix diagnostics) (&fix-hint)
   #:export (define-record-type*
             this-record
 
@@ -83,10 +87,35 @@ error-reporting purposes."
          ;; WEIRD may be an identifier, thus lacking source location info, and
          ;; BINDINGS is a list, also lacking source location info.  Hopefully
          ;; PARENT-FORM provides source location info.
-         (apply syntax-violation name "invalid field specifier"
+         (let ((forms
                 (if parent-form
                     (list parent-form #'weird)
-                    (list #'weird)))))))
+                    (list #'weird))))
+           (syntax-case #'weird ()
+             ;; common mistake
+             ((field (record-name fields ...) extra-value extra-value* ...)
+              (raise-exception
+               (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’?"
+                                   (syntax->datum #'field)
+                                   (syntax->datum #'extra-value)
+                                   (syntax->datum #'(record-name fields ...)))))
+                (&syntax (form (car forms))
+                         (subform (and (not (null? (cdr forms)))
+                                       (cadr forms))))
+                (&fix-hint (hint (object->string
+                                  (syntax->datum
+                                   #'(field
+                                      (record-name fields ... extra-value
+                                                   extra-value* ...)))))))))
+             (_
+              (apply syntax-violation name
+                     "invalid field specifier" forms))))))))
 
   (define (report-duplicate-field-specifier name ctor)
     "Report the first duplicate identifier among the bindings in CTOR."

base-commit: 808f9ffbd3106da4c92d2367b118b98196c9e81e
prerequisite-patch-id: 7fdac44e8681baaf419cbf8da78cdebb8b9f9757
prerequisite-patch-id: 1f7f1597b9c85b2b1f9db1044d193bcf6ec8650e
prerequisite-patch-id: 588ca94b9c4603424094a9cc2854c4f9bc83c7e4
prerequisite-patch-id: 82b4951463e8979d1c4cd15e1ca6a36308b21b51
prerequisite-patch-id: 75cdb9eb6b038adfb605253163b94efd51e0276c
prerequisite-patch-id: 35140f4f2873d0b9f4fc8caca6ec2e013ecb830a
prerequisite-patch-id: ed97d14afd166e7b6cac37e3aa87a85246f7e320
prerequisite-patch-id: 3f3d43f5583dce32af7d4e9925771e581c3cc5ee
prerequisite-patch-id: e8f735697c0535afe9335448b16e3e1f308de362
prerequisite-patch-id: eaf1f67c4c07482fb4da81525cbd5dcb1ea2194e
prerequisite-patch-id: 9a15aa08fbbbf110ba76409dcc2a3ab5e0764806
prerequisite-patch-id: 675a3c516f47dfcbaf61d5ad41ca7f3babdd3f20
prerequisite-patch-id: ac188cb61957c9639d0ac125c941950afbdba9c7
prerequisite-patch-id: f3f1f02944a4aa9635ce7094bfda254315d990b3
prerequisite-patch-id: 5eee450b2221d67fbda1e6581d16628394c912a7
prerequisite-patch-id: bad535152857928abf624dc49dc2b27718d3885e
prerequisite-patch-id: 623edc835c2c5dfd8c83dcf32e650cfebea42aa0
prerequisite-patch-id: c0f50259e7fc09455f77dca31113b0b55e955220
prerequisite-patch-id: 9c7c0929a48b103b6b69626dbc86d7744f2f40ad
prerequisite-patch-id: 8f8c2af0b856f56c7798a3b79ba90b073bbb382f
prerequisite-patch-id: 7d88402829a8967c23650dffe115a94ae26433da
prerequisite-patch-id: ffb3d6215a89195f6e0f274f2f119c1f7b65259a
prerequisite-patch-id: 54eec153e523b58c3670c48afda9ef50ec44eb8e
prerequisite-patch-id: bc5dfc06e9d67d10a37fbd7ba61939907d93ca7c
prerequisite-patch-id: f85cc1c9eac0d44f40afe2a547ac3d866769f685
prerequisite-patch-id: 0db9692e872bf73242cfec6f8aa390abe14d08f1
prerequisite-patch-id: 36431a656d29e90e8eb218730c64807e2477c9d7
prerequisite-patch-id: 6817d73f5e42ccd33b19642ea4ef62814ad2b10e
prerequisite-patch-id: 9146aec4a40f7da60a4c64643a9aa0e405567b04
prerequisite-patch-id: ff2daf978d58ec12c25dcbce4e7ee010d337cd54
prerequisite-patch-id: c88d74073fcd5d1ff19a4a9338b0df63b648d98a
prerequisite-patch-id: 3032f2193b95d5cb625e33daec015f3354f01903
-- 
2.33.0





Information forwarded to guix-patches <at> gnu.org:
bug#50914; Package guix-patches. (Mon, 04 Oct 2021 14:27:02 GMT) Full text and rfc822 format available.

Message #8 received at 50914 <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’.




Information forwarded to guix-patches <at> gnu.org:
bug#50914; Package guix-patches. (Tue, 05 Oct 2021 02:03:02 GMT) Full text and rfc822 format available.

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

From: Jack Hill <jackhill <at> jackhill.us>
To: Maxime Devos <maximedevos <at> telenet.be>
Cc: 50914 <at> debbugs.gnu.org
Subject: Re: [bug#50914] [PATCH] records: Raise a &fix-hint if a field has
 multiple values.
Date: Mon, 4 Oct 2021 22:02:18 -0400 (EDT)
[Message part 1 (text/plain, inline)]
Wow, thanks for picking up on an annoyance I mentioned on IRC an providing 
a patch!

On Thu, 30 Sep 2021, Maxime Devos wrote:

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

I don't have any comments on the code directly, but while the error 
message is much improved, and would have helped me find my mistake 
quicker, it's still a bit verbose and doesn't seem to be printing your 
nice error message. The error I now get is,

```
gnu/packages/mail.scm:2884:2: error: (package (name "msgconvert") (version "0.920") (source (origin (method git-fetch) (uri (git-reference (url "https://github.com/mvz/email-outlook-message-perl") (commit "dd382f47fd112032bf91cb673178a27142d23e38"))))) (sha256 (base32 "0idhpvafy0gy3bdvw98q974wdv2x6vld2sv1f95ssl4l226cdqs4")) (file-name (git-file-name name version)) (build-system perl-build-system) (native-inputs (quasiquote (("perl-module-build" (unquote perl-module-build))))) (inputs (quasiquote (("perl-email-address" (unquote perl-email-address)) ("perl-email-mime" (unquote perl-email-mime)) ("perl-email-mime-contenttype" (unquote perl-email-mime-contenttype)) ("perl-email-sender" (unquote perl-email-sender)) ("perl-email-simple" (unquote perl-email-simple)) ("perl-io-all" (unquote perl-io-all)) ("perl-io-string" (unquote perl-io-string)) ("perl-ole-storage-lite" (unquote perl-ole-storage-lite))))) (home-page "https://www.matijs.net/software/msgconv") (synopsis "Foo") (description "Foo.") (license license:gpl1+)): extraneous field initializers (sha256 file-name)
```

For reference, the faulty package definition I used to test:

```
(define-public msgconvert
  (package
    (name "msgconvert")
    (version "0.920")
    (source
     (origin
       (method git-fetch)
       (uri
        (git-reference
         (url "https://github.com/mvz/email-outlook-message-perl")
         (commit "dd382f47fd112032bf91cb673178a27142d23e38" ;; (string-append "v" version)
                 )))
       ))
    (sha256
        (base32 "0idhpvafy0gy3bdvw98q974wdv2x6vld2sv1f95ssl4l226cdqs4"))
       (file-name (git-file-name name version))
    (build-system perl-build-system)
    (native-inputs
     `(("perl-module-build" ,perl-module-build)))
    (inputs
     `(("perl-email-address" ,perl-email-address)
       ("perl-email-mime" ,perl-email-mime)
       ("perl-email-mime-contenttype" ,perl-email-mime-contenttype)
       ("perl-email-sender" ,perl-email-sender)
       ("perl-email-simple" ,perl-email-simple)
       ("perl-io-all" ,perl-io-all)
       ("perl-io-string" ,perl-io-string)
       ("perl-ole-storage-lite" ,perl-ole-storage-lite)))
    (home-page "https://www.matijs.net/software/msgconv")
    (synopsis "Foo")
    (description "Foo.")
    (license license:gpl1+)))
```

Best,
Jack

Information forwarded to guix-patches <at> gnu.org:
bug#50914; Package guix-patches. (Tue, 05 Oct 2021 04:08:01 GMT) Full text and rfc822 format available.

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

From: Jack Hill <jackhill <at> jackhill.us>
To: Maxime Devos <maximedevos <at> telenet.be>
Cc: 50914 <at> debbugs.gnu.org
Subject: Re: [bug#50914] [PATCH] records: Raise a &fix-hint if a field has
 multiple values.
Date: Tue, 5 Oct 2021 00:06:54 -0400 (EDT)
[Message part 1 (text/plain, inline)]
On Mon, 4 Oct 2021, Jack Hill wrote:

> Wow, thanks for picking up on an annoyance I mentioned on IRC an providing a 
> patch!
>
> On Thu, 30 Sep 2021, Maxime Devos wrote:
>
>> * guix/records.scm (report-invalid-field-specifier): If
>>  'weird' is something like (field (record ...) extra ...), hint that 'extra
>>  ...' should probably be moved inside (record ...).
>> ---
>
> I don't have any comments on the code directly, but while the error message 
> is much improved, and would have helped me find my mistake quicker, it's 
> still a bit verbose and doesn't seem to be printing your nice error message.

I did perturb the package definition in different ways and got your error 
message

```
error: field ‘uri’ should only have one value, but an extra value ‘(sha256 (base32 0idhpvafy0gy3bdvw98q974wdv2x6vld2sv1f95ssl4l226cdqs4))’ was passed as well.  Perhaps this extra value was supposed to be a field specifier, and needs to be moved inside the record ‘(git-reference (url https://github.com/mvz/email-outlook-message-perl) (commit dd382f47fd112032bf91cb673178a27142d23e38))’?
hint: (uri (git-reference (url "https://github.com/mvz/email-outlook-message-perl") (commit
"dd382f47fd112032bf91cb673178a27142d23e38") (sha256 (base32
"0idhpvafy0gy3bdvw98q974wdv2x6vld2sv1f95ssl4l226cdqs4")) (file-name (git-file-name
name version))))
```

with

```
(define-public msgconvert
  (package
    (name "msgconvert")
    (version "0.920")
    (source
     (origin
       (method git-fetch)
       (uri
        (git-reference
         (url "https://github.com/mvz/email-outlook-message-perl")
         (commit "dd382f47fd112032bf91cb673178a27142d23e38" ;; (string-append "v" version)
))
        (sha256
         (base32 "0idhpvafy0gy3bdvw98q974wdv2x6vld2sv1f95ssl4l226cdqs4"))
        (file-name (git-file-name name version)))))
    (build-system perl-build-system)
    (native-inputs
     `(("perl-module-build" ,perl-module-build)))
    (inputs
     `(("perl-email-address" ,perl-email-address)
       ("perl-email-mime" ,perl-email-mime)
       ("perl-email-mime-contenttype" ,perl-email-mime-contenttype)
       ("perl-email-sender" ,perl-email-sender)
       ("perl-email-simple" ,perl-email-simple)
       ("perl-io-all" ,perl-io-all)
       ("perl-io-string" ,perl-io-string)
       ("perl-ole-storage-lite" ,perl-ole-storage-lite)))
    (home-page "https://www.matijs.net/software/msgconv")
    (synopsis "Foo")
    (description "Foo.")
    (license license:gpl1+)))
```

Best,
Jack

This bug report was last modified 2 years and 204 days ago.

Previous Next


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