--- Begin Message ---
|
Subject: |
[PATCH 0/2] Improve the reported location of configuration warnings |
|
Date: |
Wed, 17 Nov 2021 14:40:28 +0000 |
Hello everyone,
While working on the swap-space patch, I noticed that currently,
warnings about deprecated fields in guix records use the location of
the record definition macro, rather than of the invalid value. For
some records such as 'operating-system', this makes it rather user
unfriendly and confusing.
This patchset first adds the syntax 'define-with-syntax-properties',
which helps avoid boilerplate code to define sanitizers with proper
location reporting. I put it in guix/diagnostics.scm as I thought
this was the place that was most likely to be use-module'd for warning
messages, as this is quite tied to that use. The second patch makes
use of this new helper to update two warnings: the one about 'target'
to 'targets' in bootloader.scm, and the one about setuid-programs. In
both cases, a `guix system reconfigure` now reports the exact location
of the incriminating values, rather than of the
'bootloader-configuration' or 'operating-system' lines respectively.
The approach I've taken for the helper was to make something simple
and general enough for most uses. It should correctly report syntax
errors with custom errors messages rather than the generic "source
expression failed to match any pattern ...". Note although that it
isn't possible to do any defines in the body of the macro, as this
doesn't use any lambda-like macros (see the second case for an
example).
I can see two drawbacks to this macro:
1) This macro will not help you write expand-time checkers. This
would introduce too much complexity, and I'm not sure the end-user
would notice a significant change.
2) It doesn't deconstruct values such as lists to get the individual
list values's properties. This would also introduce too much
complexity (eg. checking if the list is literal, deconstructing it,
and falling back to a generic source location if the list is only
available at eval-time).
Best,
Josselin Poiret
Josselin Poiret (2):
guix: Add syntax to capture arguments' syntax-properties.
gnu: system: Improve location of some configuration warnings.
gnu/bootloader.scm | 16 +++++++---------
gnu/system.scm | 11 ++++++-----
guix/diagnostics.scm | 38 +++++++++++++++++++++++++++++++++++++-
3 files changed, 50 insertions(+), 15 deletions(-)
--
2.33.1
--- End Message ---
--- Begin Message ---
|
Subject: |
Re: bug#51922: [PATCH 0/2] Improve the reported location of configuration warnings |
|
Date: |
Wed, 17 Nov 2021 17:58:14 +0100 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) |
Hi,
Josselin Poiret <dev@jpoiret.xyz> skribis:
> While working on the swap-space patch, I noticed that currently,
> warnings about deprecated fields in guix records use the location of
> the record definition macro, rather than of the invalid value. For
> some records such as 'operating-system', this makes it rather user
> unfriendly and confusing.
>
> This patchset first adds the syntax 'define-with-syntax-properties',
> which helps avoid boilerplate code to define sanitizers with proper
> location reporting. I put it in guix/diagnostics.scm as I thought
> this was the place that was most likely to be use-module'd for warning
> messages, as this is quite tied to that use. The second patch makes
> use of this new helper to update two warnings: the one about 'target'
> to 'targets' in bootloader.scm, and the one about setuid-programs. In
> both cases, a `guix system reconfigure` now reports the exact location
> of the incriminating values, rather than of the
> 'bootloader-configuration' or 'operating-system' lines respectively.
Neat! That’s a much welcome improvement.
> The approach I've taken for the helper was to make something simple
> and general enough for most uses. It should correctly report syntax
> errors with custom errors messages rather than the generic "source
> expression failed to match any pattern ...". Note although that it
> isn't possible to do any defines in the body of the macro, as this
> doesn't use any lambda-like macros (see the second case for an
> example).
I think that’s OK, that’s a reasonable approach.
> I can see two drawbacks to this macro:
> 1) This macro will not help you write expand-time checkers. This
> would introduce too much complexity, and I'm not sure the end-user
> would notice a significant change.
> 2) It doesn't deconstruct values such as lists to get the individual
> list values's properties. This would also introduce too much
> complexity (eg. checking if the list is literal, deconstructing it,
> and falling back to a generic source location if the list is only
> available at eval-time).
Again, I think that’s fine: this macro solves what it was designed to
address, and if we need something fancier, we can think about it later.
> guix: Add syntax to capture arguments' syntax-properties.
> gnu: system: Improve location of some configuration warnings.
I tweaked the first commit log and applied it.
Thank you!
Ludo’.
--- End Message ---