[PATCH] guix: records: Improve error reporting.

OpenSubmitted by Julien Lepiller.
Details
2 participants
  • Sarah Morgensen
  • Julien Lepiller
Owner
unassigned
Severity
normal
J
J
Julien Lepiller wrote on 21 Jul 01:40 +0200
Download
(address . guix-patches@gnu.org)
20210721014047.3878a0c7@tachikoma.lepiller.eu
Download
Hi Guix!
This patch improves error reporting a bit when making mistakes in guixrecords. 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 keyservices. Values are:(append (list (service ...) ...))(modify-services %desktop-services ...)
Which would have hinted them at fixing the parenthesis. Or at least, itwould 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 afield is `(services value)'
(operating-system (services))test.scm:2:2: error: (services): Value missing in field specifier. Theformat of a field is `(services value)'.
(operating-system (services 1 2 3))test.scm:2:2: error: (services 1 2 3): multiple values in fieldspecifier. Got 3 values associated with key services. Values are:123
(operating-system ())guix system: error: (): invalid field specifier. The format of a fieldis `(field value)'
(operating-system ((services %desktop-services)))test.scm:2:2: error: ((services %desktop-services)): invalid fieldspecifier. (services %desktop-services) is not a valid field name.

Of course, we can improve these error messages, and internationalizethem.
WDYT?
Download
From 4cde5555682ca6a3c78cb274cc308087e51ea0e9 Mon Sep 17 00:00:00 2001From: Julien Lepiller <julien@lepiller.eu>Date: Wed, 21 Jul 2021 01:30:04 +0200Subject: [PATCH] guix: records: Improve error reporting.
* guix/records.scm (report-invalid-field-specifier): Handle variousinvalidity causes separately.--- guix/records.scm | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-)
Toggle diff (58 lines)diff --git a/guix/records.scm b/guix/records.scmindex 3d54a51956..3c3dd478a5 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 Ludovic Courtès <ludo@gnu.org> ;;; Copyright © 2018 Mark H Weaver <mhw@netris.org>+;;; Copyright © 2021 Julien Lepiller <julien@lepiller.eu> ;;; ;;; This file is part of GNU Guix. ;;;@@ -83,10 +84,40 @@ 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"- (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)))+ (((field ...) _ ...) ;a list whose first element is a list+ (apply syntax-violation name+ (format #f "invalid field specifier. ~a is not a valid field name."+ (map syntax->datum #'(field ...)))+ (list parent-form #'weird)+ (list #'weird)))+ ((field) ;a list with one element+ (apply syntax-violation name+ (format #f "Value missing in field specifier. The format of \+a field is `(~a value)'."+ (syntax->datum #'field))+ (list parent-form #'weird)+ (list #'weird)))+ ((field value ...) ;any other list+ (apply syntax-violation name+ (format #f "multiple values in field specifier. Got ~a values \+associated with key ~a. Values are:~%~{~a~%~}"+ (length #'(value ...)) (syntax->datum #'field)+ (map syntax->datum #'(value ...)))+ (list parent-form #'weird)+ (list #'weird)))+ (field ;not a list+ (apply syntax-violation name+ (format #f "invalid field specifier. The format of a field \+is `(~a value)'"+ (syntax->datum #'field))+ (list parent-form #'weird)+ (list #'weird)))))))) (define (report-duplicate-field-specifier name ctor) "Report the first duplicate identifier among the bindings in CTOR."-- 2.32.0
S
S
Sarah Morgensen wrote on 21 Jul 21:21 +0200
Download
(name . Julien Lepiller)(address . julien@lepiller.eu)(address . 49671@debbugs.gnu.org)
86zgufscb0.fsf@mgsn.dev
Download
Hi Julien,
Julien Lepiller <julien@lepiller.eu> writes:
Toggle quote (7 lines)> 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:
Toggle snippet (3 lines)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
After adding `(ice-9 format)` to imports it works as expected. I seethis also applies to package records! This will be great for thosestarting to package in Guix.
Toggle quote (25 lines)>> 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 `'?
Toggle quote (13 lines)> 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 `guixsystem` instead?
Toggle quote (6 lines)>> Of course, we can improve these error messages, and internationalize> them.>> WDYT?
[...]
Toggle quote (10 lines)> - (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-formis optional, shouldn't this be
Toggle snippet (5 lines) (apply syntax-violation name "invalid field specifier. The format of a field is `(field value)'" (if parent-form (list parent-form #:'weird) (list weird)))
(and similar for the others)?
--Sarah
?
Your may also send email to 49671@debbugs.gnu.org to comment.