[bug#63985,v3,00/11] Service subsystem improvements

This patch-series is an agglomeration of smaller but weakly-related
patch-series, done so in order to build up the case for the changes.
I structured the series in order to make it suitable for cherry-picking.


Summary of changes:

* Plumbing changes to serialize-configuration

By orienting it around SRFI-171 transducers, it's now easier
to build custom configuration serializing procedures.

* New define-configuration syntax: serializer-options

Serializers may now accept more than two arguments.

* New module (gnu services configuration generic-ini)

* Deduplicate often used predicates.

* network-manager-service-type cleanup & new fields.



Notable changes since v2:

* Documentation changes

The documentation for define-configuration was reworded to make later
syntax extensions easier to document.
In addition, the new 'serializer-options' is now documented.


* (gnu services configuration) changes

** New predicates.

Reduce code duplication by migrating some commonly used predicates into
this module.


* generic-ini changes

** Initial field testing

Some deficiencies were found & corrected while doing a first field
testing when network-manager-service-type was refactored to make use
of this module.

** generic-ini- serializers

** Handling of multiple entries

Implemented as a transducer, this is useful to deal with escape-hatch
fields.


* network-manager-service-type changes

** Use define-configuration for <network-manager-configuration>

** Refactored serialization process to use the new generic-ini module

** New configuration fields: log-configuration and extra-options

Provides an escape hatch for options not yet implemented in the
record type.

** Renamed fields: 'network-manager' to 'package'

Naming the field 'package' is more informative & less confusing than a
reduplication of the package-name with the field-name itself.



Omissions in generic-ini:

For now, I've omitted:
* Custom leading (presumed to be whitespace) characters for entries
  à la gitconfig, mostly beautifying purposes
* Configurable delimiter (\n, \r\n, \0, ...)
* Configurable Key-value separator (this is usually =)

These can be implemented later if required.



Notes:

The interface in (gnu services configuration generic-ini) is still in its
infancy and might require further adjustments/additions and I'm still
thinking about its potential generalizations to TOML & co.
For the time being I'd prefer not to promise any interface stability.



Nice to haves:

I didn't have much luck in replacing the memq approach in the predicate
with define-enumeration:

--8<---------------cut here---------------start------------->8---
;; This works.
(define (network-manager-log-level? x)
  (memq x '(off err warn info debug trace)))

;; This does not.
(define-enumeration network-manager-log-level?
  (off err warn info debug trace)
  network-manager-log-level-set)

;; While executing meta-command:
;; ERROR:
;;   1. &origin: "network-manager-log-level?"
;;   2. &message: "not a member of the set"
;;   3. &syntax:
;;      form: #f
;;      subform: #f
--8<---------------cut here---------------end--------------->8---

Would be nice to know what went wrong and whether an enumeration could
be used here instead.


Bruno Victal (11):
  services: configuration: Simplify normalize-extra-args.
  services: configuration: Use transducers within
    serialize-configuration.
  services: fstrim-service-type: Serialize with SRFI-171 transducers.
  doc: Rewrite define-configuration.
  services: configuration: Add serializer-options field.
  services: configuration: New generic-ini module.
  services: configuration: Add some commonly used predicates.
  services: NetworkManager: Use define-configuration and generic-ini.
  services: NetworkManager: Prefer package over network-manager.
  services: NetworkManager: add log-configuration field.
  services: NetworkManager: Add extra-options field.

 Makefile.am                                  |   1 +
 doc/guix.texi                                | 161 +++++----
 gnu/local.mk                                 |   1 +
 gnu/services/audio.scm                       |   7 +-
 gnu/services/configuration.scm               | 108 ++++--
 gnu/services/configuration/generic-ini.scm   | 165 +++++++++
 gnu/services/linux.scm                       |  11 +-
 gnu/services/networking.scm                  | 352 ++++++++++++++-----
 gnu/services/telephony.scm                   |  49 ++-
 tests/services/configuration.scm             |  88 ++++-
 tests/services/configuration/generic-ini.scm | 129 +++++++
 11 files changed, 861 insertions(+), 211 deletions(-)
 create mode 100644 gnu/services/configuration/generic-ini.scm
 create mode 100644 tests/services/configuration/generic-ini.scm


base-commit: ac86174e22fcd762893bd4515786b1376af9397b
  

Am Montag, dem 26.06.2023 um 22:57 +0100 schrieb Bruno Victal:
> ** Renamed fields: 'network-manager' to 'package'
> 
> Naming the field 'package' is more informative & less confusing than
> a reduplication of the package-name with the field-name itself.
This goes against established practice in pretty much every other
service out there.  It'd also be way more painful to read if it was
applied broadly – so many fields simply named "package" in your
configuration might be easier to grep, but certainly not easier to
understand.  As such, I'd suggest to revert this particular change.

As for the other changes I'll have a detailed look later.

Cheers
  

Hi all,

I've been pondering about the changes here and would like to comment
on them:

On 2023-06-26 22:57, Bruno Victal wrote:
> Bruno Victal (11):
>   services: configuration: Simplify normalize-extra-args.
>   services: configuration: Use transducers within
>     serialize-configuration.
>   services: fstrim-service-type: Serialize with SRFI-171 transducers.
>   doc: Rewrite define-configuration.
>   services: configuration: Add serializer-options field.

I think these changes are OK on their own since they add some extra
flexibility to the serialize-configuration procedure and address a
TODO item.

>   services: configuration: New generic-ini module.
>   services: configuration: Add some commonly used predicates.

IMO I'm afraid this might be somewhat short-sighted and would be
better addressed directly in Guile by implementing SRFI-233, perhaps
by doing some adaptations to the approach taken here.

>   services: NetworkManager: Use define-configuration and generic-ini.>   services: NetworkManager: Prefer package over network-manager.
>   services: NetworkManager: add log-configuration field.
>   services: NetworkManager: Add extra-options field.

Naturally these are no longer relevant if this generic-ini module
approach is abandoned.

Comments?
  

Am Samstag, dem 16.09.2023 um 22:22 +0100 schrieb Bruno Victal:
> Hi all,
> 
> I've been pondering about the changes here and would like to comment
> on them:
> 
> On 2023-06-26 22:57, Bruno Victal wrote:
> > Bruno Victal (11):
> >   services: configuration: Simplify normalize-extra-args.
> >   services: configuration: Use transducers within
> >     serialize-configuration.
> >   services: fstrim-service-type: Serialize with SRFI-171
> > transducers.
> >   doc: Rewrite define-configuration.
> >   services: configuration: Add serializer-options field.
> 
> I think these changes are OK on their own since they add some extra
> flexibility to the serialize-configuration procedure and address a
> TODO item.
I'm not sure whether serializer options really add much value.  You can
use functional programming to define serializers for you and pass those
options in a cleaner way IMHO.  The documentation should be updated as
the changes are made.  As for the switch to SRFI 171, I'm not sure
whether backwards compatibility with Guile 2.2 is a requirement
somewhere; if it isn't, that change is probably fine.

> >   services: configuration: New generic-ini module.
> >   services: configuration: Add some commonly used predicates.
> 
> IMO I'm afraid this might be somewhat short-sighted and would be
> better addressed directly in Guile by implementing SRFI-233, perhaps
> by doing some adaptations to the approach taken here.
Even if Guile implemented SRFI 233 now, I'm not sure we could use it
tomorrow.  And even once we can use SRFI 233, we can keep backwards-
compatibility be re-exporting things.  The question is how necessary it
will be for us to maintain our own INI format writer.  NetworkManager
is one use case, but perhaps we have others (perhaps even in the gnome
world – gdm maybe?)

> >   services: NetworkManager: Use define-configuration and generic-
> > ini.>   services: NetworkManager: Prefer package over network-
> > manager.
> >   services: NetworkManager: add log-configuration field.
> >   services: NetworkManager: Add extra-options field.
> 
> Naturally these are no longer relevant if this generic-ini module
> approach is abandoned.
I think we can still upgrade this to define-configuration without a
generic-ini, but see above.  That being said, we can certainly split
this into two series at the point you currently feel comfortable with
and work from there.

WDYT?
  

On 2023-09-16 22:55, Liliana Marie Prikler wrote:
> I'm not sure whether serializer options really add much value.  You can
> use functional programming to define serializers for you and pass those
> options in a cleaner way IMHO.  The documentation should be updated as
> the changes are made.  As for the switch to SRFI 171, I'm not sure
> whether backwards compatibility with Guile 2.2 is a requirement
> somewhere; if it isn't, that change is probably fine.

Is SRFI-171 not available in Guile 2.2?
Your last remark surprised me though: is Guix not running with Guile 3.0?
I was the impression that this was the case since otherwise wouldn't this
imply that `spawn' & co. can't be used anywhere?

> Even if Guile implemented SRFI 233 now, I'm not sure we could use it
> tomorrow.  And even once we can use SRFI 233, we can keep backwards-
> compatibility be re-exporting things. (…)

In that aspect I'm willing to be patient vs. maintaining a temporary
(read as: permanent) code though I think that it should be possible to
make it backwards-compatible, even if we completely gut out its innards
later and replace them with SRFI-233.

> (…) The question is how necessary it
> will be for us to maintain our own INI format writer.  NetworkManager
> is one use case, but perhaps we have others (perhaps even in the gnome
> world – gdm maybe?)

Certainly there are many applications that make use of INI-like files
for configuration and for INI ones it would be convenient, though I
should caution that there are many things that can look like INI but
aren't:

* NetworkManager accepts some entries that have append behavior via
'KW += val' and have repetition. In some cases I think the ordering
matters too. (Since our define-configuration definition for it
doesn't attempt to fully cover every nook and cranny of it I think
using INI here doesn't hurt.)

* TOML

* Files that can have leading entries but without a section. These
can be thought to belong to some top level but invisible section yet
the generic-ini doesn't handle these. (yet)


There's some assumptions I made while writing generic-ini which make
it not as generic as imparted by its name and as such, it can only be
used in the following conditions:

* The ordering of the entries and sections doesn't matter.
* Every entry belongs to a section.
* (… perhaps more? …)

>> Naturally these are no longer relevant if this generic-ini module
>> approach is abandoned.
> I think we can still upgrade this to define-configuration without a
> generic-ini, but see above.  That being said, we can certainly split> this into two series (…)

Sure.

> (…) at the point you currently feel comfortable with
> and work from there.
> 
> WDYT?

I'm inclined to write-off the generic-ini though as discussed above,
there's some demand for some kind of INI format writer so personally
I'd be OK with temporarily maintaining this writer if we can really
make it an experiment/true to the word “temporary” thing. This would
mean that:

* It should be only used internally by services living in Guix
repository. I'm OK with going around and reworking/replacing usages
of it when the time comes to retire it/when guile gets this INI thing
natively. (i.e. #:export (…) doesn't mean that I'm intending it to
be used outside of the repo with stability promises.)


WDYT?
  

Am Samstag, dem 23.09.2023 um 14:35 +0100 schrieb Bruno Victal:
> On 2023-09-16 22:55, Liliana Marie Prikler wrote:
> > I'm not sure whether serializer options really add much value.  You
> > can use functional programming to define serializers for you and
> > pass those options in a cleaner way IMHO.  The documentation should
> > be updated as the changes are made.  As for the switch to SRFI 171,
> > I'm not sure whether backwards compatibility with Guile 2.2 is a
> > requirement somewhere; if it isn't, that change is probably fine.
> 
> Is SRFI-171 not available in Guile 2.2?
> Your last remark surprised me though: is Guix not running with Guile
> 3.0?  I was the impression that this was the case since otherwise
> wouldn't this imply that `spawn' & co. can't be used anywhere?
Nope, it was introduced with 3.0.  Again, "I'm not sure" meaning "I
don't know".  Would be nice to have another reviewer check this.  
(Looking at CC:) Maxim? Ludo?

> 
> > (…) The question is how necessary it
> > will be for us to maintain our own INI format writer. 
> > NetworkManager is one use case, but perhaps we have others (perhaps
> > even in the gnome world – gdm maybe?)
> 
> Certainly there are many applications that make use of INI-like files
> for configuration and for INI ones it would be convenient, though I
> should caution that there are many things that can look like INI but
> aren't:
> 
> * NetworkManager accepts some entries that have append behavior via
> 'KW += val' and have repetition. In some cases I think the ordering
> matters too. (Since our define-configuration definition for it
> doesn't attempt to fully cover every nook and cranny of it I think
> using INI here doesn't hurt.)
> 
> * TOML
> 
> * Files that can have leading entries but without a section. These
> can be thought to belong to some top level but invisible section yet
> the generic-ini doesn't handle these. (yet)
> 
> 
> There's some assumptions I made while writing generic-ini which make
> it not as generic as imparted by its name and as such, it can only be
> used in the following conditions:
> 
> * The ordering of the entries and sections doesn't matter.
> * Every entry belongs to a section.
> * (… perhaps more? …)
Some of these look like bugs, others are a result of trying to cover
too much.  Let's perhaps just cover the simple case of 

[maybe a section]
<var> <op> <val>

where <op> will almost always be "=" rather than worrying about the
specification of e.g. TOML.  If needed, a TOML-specific module can
hopefully reuse the procedures by which we produce INI files.

> > 
> > (…) at the point you currently feel comfortable with
> > and work from there.
> > 
> > WDYT?
> 
> I'm inclined to write-off the generic-ini though as discussed above,
> there's some demand for some kind of INI format writer so personally
> I'd be OK with temporarily maintaining this writer if we can really
> make it an experiment/true to the word “temporary” thing. This would
> mean that:
> 
> * It should be only used internally by services living in Guix
> repository. I'm OK with going around and reworking/replacing usages
> of it when the time comes to retire it/when guile gets this INI thing
> natively. (i.e. #:export (…) doesn't mean that I'm intending it to
> be used outside of the repo with stability promises.)
> 
> 
> WDYT?
Do you have commit access?  The only real place where you can
experiment in the (guix) namespace is on feature branches and if you
feel like you need to experiment further, I'd recommend doing so.  If
not, you could roll out a channel with an extension like the one that
was uses for (guix home)¹.  You might also want to reach out to guix-
devel to try and explain your approach to everyone in terms of how it
would simplify writing services.

Cheers

¹ I think the Guix Home thing itself shows that you can put
technological previews to Guix itself and have them tested (and
depended on!) by many.  This may or may not be what you want, there
sadly isn't a "clean" option.
  

Hello!

Liliana Marie Prikler <liliana.prikler@gmail.com> skribis:

> I'm not sure whether serializer options really add much value.  You can
> use functional programming to define serializers for you and pass those
> options in a cleaner way IMHO.  The documentation should be updated as
> the changes are made.  As for the switch to SRFI 171, I'm not sure
> whether backwards compatibility with Guile 2.2 is a requirement
> somewhere; if it isn't, that change is probably fine.

Backward compatibility with 2.2 is not required in the service code.

(The places where compatibility with 2.2 or even 2.0 may be required are
some of the (guix build …) modules and core (guix …) modules, the latter
so that a very old Guix can still pull the new one.)

Ludo’.