[PATCH 0/2] Support service extensions on the "final" service values

  • Open
  • quality assurance status badge
Details
4 participants
  • Alex Kost
  • ???
  • Ludovic Courtès
  • Ricardo Wurmus
Owner
unassigned
Submitted by
Ludovic Courtès
Severity
important
L
L
Ludovic Courtès wrote on 30 May 2017 23:58
(address . guix-patches@gnu.org)
20170530215850.7522-1-ludo@gnu.org
Hello!

This patch adds support for service extensions that modify the
"final" values of a service. This is meant to implement cross-cutting
concerns as well as system-wide customization as discussed with Alex
long ago:


To summarize, a "finalization extension" (for lack of a better name)
gets the final value of a service and returns a new value for that
service. This is in contrast with a "normal" extension which can only
contribute to the value of a target service, and not inspect the value
of that target service.

For example, for the /etc service, a "normal" extension can only add
entries for /etc. A "finalization" extension can instead inspect and
change all the /etc entries. IOW, it is a sort of a "sudo" for service
extensions; it's also quite inelegant compared to the "normal" extension
mechanism, but it's certainly useful.

A use case is given in the second patch: we change all the PAM services
to use pam_elogind.so or pam_limits.so. Likewise, the 'rename-etc-files'
service below shows how to rename all the files in /etc (for illustration
purposes only :-)):

(define rename-etc-files
(let ((rename (lambda (prefix entries)
(map (match-lambda
((name . rest)
(cons (string-append prefix name)
rest)))
entries))))
(service-type
(name 'rename-etc-files)
(extensions (list (service-extension etc-service-type
(const '())
rename))))))


(operating-system
;; ...
(services (cons* (service rename-etc-files "foo-")
...)))

I think this should fulfill the need that Alex had expressed, which is
to not only be able to add files to /etc, but also to have the ability
to inspect and modify what goes to /etc.

The first patch currently lacks doc. I'll work on it if there's consensus
on the approach.

Feedback welcome!

Ludo'.

Ludovic Courtès (2):
DRAFT services: Extensions can specify a "finalization" procedure.
system: pam: Remove custom API to transform PAM services.

gnu/services.scm | 52 ++++++++++++++++++++++++++++++++++++++----------
gnu/services/base.scm | 33 ++++++++++++++++--------------
gnu/services/desktop.scm | 23 +++++++++++----------
gnu/system/pam.scm | 44 ++++++++--------------------------------
tests/services.scm | 34 +++++++++++++++++++++++++++++++
5 files changed, 114 insertions(+), 72 deletions(-)

--
2.13.0
L
L
Ludovic Courtès wrote on 31 May 2017 00:05
[PATCH 1/2] DRAFT services: Extensions can specify a "finalization" procedure.
(address . 27155@debbugs.gnu.org)
20170530220509.8254-1-ludo@gnu.org
TODO: Add doc

* gnu/services.scm (<service-extension>)[finalize]: New field.
Rename 'service-extension' to '%service-extension'.
(right-identity): New procedure.
(service-extension): New macro.
(fold-services)[apply-finalization, compose*]: New procedures.
Honor finalizations.
* tests/services.scm ("fold-services with finalizations"): New test.
---
gnu/services.scm | 52 ++++++++++++++++++++++++++++++++++++++++++----------
tests/services.scm | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+), 10 deletions(-)

Toggle diff (127 lines)
diff --git a/gnu/services.scm b/gnu/services.scm
index 5c314748d..4ebce753b 100644
--- a/gnu/services.scm
+++ b/gnu/services.scm
@@ -119,10 +119,24 @@
;;; Code:
(define-record-type <service-extension>
- (service-extension target compute)
+ (%service-extension target compute finalize)
service-extension?
- (target service-extension-target) ;<service-type>
- (compute service-extension-compute)) ;params -> params
+ (target service-extension-target) ;<service-type>
+ (compute service-extension-compute) ;value -> extension value
+ (finalize service-extension-finalize)) ;self other -> other
+
+(define (right-identity a b) b)
+
+(define-syntax service-extension
+ (syntax-rules ()
+ "Instantiate an extension of services of type TARGET. COMPUTE takes the
+value of the source service and returns the extension value of the target.
+Optionally, FINALIZE takes the value of the source service and the final value
+of the target, and returns a new value for the target."
+ ((_ target compute)
+ (%service-extension target compute right-identity))
+ ((_ target compute finalize)
+ (%service-extension target compute finalize))))
(define &no-default-value
;; Value used to denote service types that have no associated default value.
@@ -664,6 +678,21 @@ TARGET-TYPE; return the root service adjusted accordingly."
(($ <service-extension> _ compute)
(compute (service-value service))))))
+ (define (apply-finalization target)
+ (lambda (service)
+ (match (find (matching-extension target)
+ (service-type-extensions (service-kind service)))
+ (($ <service-extension> _ _ finalize)
+ (lambda (final)
+ (finalize (service-value service) final))))))
+
+ (define (compose* procs)
+ (match procs
+ (()
+ identity)
+ (_
+ (apply compose procs))))
+
(match (filter (lambda (service)
(eq? (service-kind service) target-type))
services)
@@ -671,15 +700,18 @@ TARGET-TYPE; return the root service adjusted accordingly."
(let loop ((sink sink))
(let* ((dependents (map loop (dependents sink)))
(extensions (map (apply-extension sink) dependents))
+ ;; We distinguish COMPOSE and EXTEND because PARAMS typically
+ ;; has a different type than the elements of EXTENSIONS.
(extend (service-type-extend (service-kind sink)))
(compose (service-type-compose (service-kind sink)))
- (params (service-value sink)))
- ;; We distinguish COMPOSE and EXTEND because PARAMS typically has a
- ;; different type than the elements of EXTENSIONS.
- (if extend
- (service (service-kind sink)
- (extend params (compose extensions)))
- sink))))
+ (value (if extend
+ (extend (service-value sink)
+ (compose extensions))
+ (service-value sink)))
+ (kind (service-kind sink))
+ (finalizations (map (apply-finalization sink)
+ dependents)))
+ (service kind ((compose* finalizations) value)))))
(()
(raise
(condition (&missing-target-service-error
diff --git a/tests/services.scm b/tests/services.scm
index 8484ee982..bb42e352a 100644
--- a/tests/services.scm
+++ b/tests/services.scm
@@ -88,6 +88,40 @@
(and (eq? (service-kind r) t1)
(service-value r))))
+(test-equal "fold-services with finalizations"
+ '(final 600 (initial-value 5 4 3 2 1 xyz 600))
+
+ ;; Similar to the one above, but this time with "finalization" extensions
+ ;; that modify the final result of compose/extend.
+ (let* ((t1 (service-type (name 't1) (extensions '())
+ (compose concatenate)
+ (extend cons)))
+ (t2 (service-type (name 't2)
+ (extensions
+ (list (service-extension t1
+ (cut list 'xyz <>)
+ (lambda (t2 t1)
+ `(final ,t2 ,t1)))))
+ (compose (cut reduce + 0 <>))
+ (extend *)))
+ (t3 (service-type (name 't3)
+ (extensions
+ (list (service-extension t2 identity)
+ (service-extension t1 list)))))
+ (t4 (service-type (name 't4)
+ (extensions
+ (list (service-extension t2 (const 0)
+ *)))))
+ (r (fold-services (cons* (service t1 'initial-value)
+ (service t2 4)
+ (service t4 10)
+ (map (lambda (x)
+ (service t3 x))
+ (iota 5 1)))
+ #:target-type t1)))
+ (and (eq? (service-kind r) t1)
+ (service-value r))))
+
(test-assert "fold-services, ambiguity"
(let* ((t1 (service-type (name 't1) (extensions '())
(compose concatenate)
--
2.13.0
L
L
Ludovic Courtès wrote on 31 May 2017 00:05
[PATCH 2/2] system: pam: Remove custom API to transform PAM services.
(address . 27155@debbugs.gnu.org)
20170530220509.8254-2-ludo@gnu.org
This specific way to extend 'pam-root-service-type' has been subsumed by
the "finalization extensions" of services.

* gnu/system/pam.scm (<pam-configuration>): Remove.
(/etc-entry): Adjust accordingly.
(extend-configuration): Remove.
(pam-root-service-type)[extend]: Set to 'append'.
(pam-root-service): Remove #:transform parameter. Adjust 'service'
form.
* gnu/services/desktop.scm (pam-extension-procedure): Rename to...
(elogind-pam-extension): ... this. Expect the complete list of
services and map over it.
(elogind-service-type): Change PAM-ROOT-SERVICE-TYPE extension to refer
to 'elogind-pam-extension'.
* gnu/services/base.scm (limits-pam-extension): New procedure.
(pam-limits-service-type): Remove 'pam-extension' procedure. Adjust
PAM-ROOT-SERVICE-TYPE extension accordingly.
---
gnu/services/base.scm | 33 ++++++++++++++++++---------------
gnu/services/desktop.scm | 23 ++++++++++++-----------
gnu/system/pam.scm | 44 ++++++++------------------------------------
3 files changed, 38 insertions(+), 62 deletions(-)

Toggle diff (179 lines)
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index 7cd9a34ca..d36f5c410 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -1239,6 +1239,21 @@ information on the configuration file syntax."
(service syslog-service-type config))
+(define (limits-pam-extension limits-file pam-services)
+ "Modify some of PAM-SERVICES to use 'pam_limits.so'."
+ (map (lambda (pam)
+ (let ((pam-limits (pam-entry
+ (control "required")
+ (module "pam_limits.so")
+ (arguments '("conf=/etc/security/limits.conf")))))
+ (if (member (pam-service-name pam) '("login" "su" "slim"))
+ (pam-service
+ (inherit pam)
+ (session (cons pam-limits
+ (pam-service-session pam))))
+ pam)))
+ pam-services))
+
(define pam-limits-service-type
(let ((security-limits
;; Create /etc/security containing the provided "limits.conf" file.
@@ -1250,26 +1265,14 @@ information on the configuration file syntax."
(mkdir #$output)
(stat #$limits-file)
(symlink #$limits-file
- (string-append #$output "/limits.conf"))))))))
- (pam-extension
- (lambda (pam)
- (let ((pam-limits (pam-entry
- (control "required")
- (module "pam_limits.so")
- (arguments '("conf=/etc/security/limits.conf")))))
- (if (member (pam-service-name pam)
- '("login" "su" "slim"))
- (pam-service
- (inherit pam)
- (session (cons pam-limits
- (pam-service-session pam))))
- pam)))))
+ (string-append #$output "/limits.conf")))))))))
(service-type
(name 'limits)
(extensions
(list (service-extension etc-service-type security-limits)
(service-extension pam-root-service-type
- (lambda _ (list pam-extension))))))))
+ (const '())
+ limits-pam-extension))))))
(define* (pam-limits-service #:optional (limits '()))
"Return a service that makes selected programs respect the list of
diff --git a/gnu/services/desktop.scm b/gnu/services/desktop.scm
index 36049587d..6495bc94c 100644
--- a/gnu/services/desktop.scm
+++ b/gnu/services/desktop.scm
@@ -1,5 +1,5 @@
;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2015 Andy Wingo <wingo@igalia.com>
;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
;;; Copyright © 2016 Sou Bunnbu <iyzsong@gmail.com>
@@ -637,21 +637,21 @@ include the @command{udisksctl} command, part of UDisks, and GNOME Disks."
"ELOGIND_CONF_FILE"
(elogind-configuration-file config))))
-(define (pam-extension-procedure config)
- "Return an extension for PAM-ROOT-SERVICE-TYPE that ensures that all the PAM
-services use 'pam_elogind.so', a module that allows elogind to keep track of
-logged-in users (run 'loginctl' to see elogind's world view of users and
-seats.)"
+(define (elogind-pam-extension config pam-services)
+ "Change PAM-SERVICES so that each of them uses 'pam_elogind.so', a module
+that allows elogind to keep track of logged-in users (run 'loginctl' to see
+elogind's world view of users and seats), and return that."
(define pam-elogind
(pam-entry
(control "required")
(module (file-append (elogind-package config)
"/lib/security/pam_elogind.so"))))
- (list (lambda (pam)
- (pam-service
- (inherit pam)
- (session (cons pam-elogind (pam-service-session pam)))))))
+ (map (lambda (pam)
+ (pam-service
+ (inherit pam)
+ (session (cons pam-elogind (pam-service-session pam)))))
+ pam-services))
(define elogind-service-type
(service-type (name 'elogind)
@@ -669,7 +669,8 @@ seats.)"
;; Extend PAM with pam_elogind.so.
(service-extension pam-root-service-type
- pam-extension-procedure)
+ (const '())
+ elogind-pam-extension)
;; We need /run/user, /run/systemd, etc.
(service-extension file-system-service-type
diff --git a/gnu/system/pam.scm b/gnu/system/pam.scm
index eedf93394..b1bfab7ba 100644
--- a/gnu/system/pam.scm
+++ b/gnu/system/pam.scm
@@ -1,5 +1,5 @@
;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2013, 2014, 2015, 2016 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -281,50 +281,22 @@ authenticate to run COMMAND."
;;; PAM root service.
;;;
-;; Overall PAM configuration: a list of services, plus a procedure that takes
-;; one <pam-service> and returns a <pam-service>. The procedure is used to
-;; implement cross-cutting concerns such as the use of the 'elogind.so'
-;; session module that keeps track of logged-in users.
-(define-record-type* <pam-configuration>
- pam-configuration make-pam-configuration? pam-configuration?
- (services pam-configuration-services) ;list of <pam-service>
- (transform pam-configuration-transform)) ;procedure
-
-(define (/etc-entry config)
+(define (/etc-entry services)
"Return the /etc/pam.d entry corresponding to CONFIG."
- (match config
- (($ <pam-configuration> services transform)
- (let ((services (map transform services)))
- `(("pam.d" ,(pam-services->directory services)))))))
-
-(define (extend-configuration initial extensions)
- "Extend INITIAL with NEW."
- (let-values (((services procs)
- (partition pam-service? extensions)))
- (pam-configuration
- (services (append (pam-configuration-services initial)
- services))
- (transform (apply compose
- (pam-configuration-transform initial)
- procs)))))
+ `(("pam.d" ,(pam-services->directory services))))
(define pam-root-service-type
(service-type (name 'pam)
(extensions (list (service-extension etc-service-type
/etc-entry)))
- ;; Arguments include <pam-service> as well as procedures.
+ ;; Arguments are <pam-service> objects.
(compose concatenate)
- (extend extend-configuration)))
+ (extend append)))
-(define* (pam-root-service base #:key (transform identity))
+(define* (pam-root-service base)
"The \"root\" PAM service, which collects <pam-service> instance and turns
-them into a /etc/pam.d directory, including the <pam-service> listed in BASE.
-TRANSFORM is a procedure that takes a <pam-service> and returns a
-<pam-service>. It can be used to implement cross-cutting concerns that affect
-all the PAM services."
- (service pam-root-service-type
- (pam-configuration (services base)
- (transform transform))))
+them into a /etc/pam.d directory, including the <pam-service> listed in BASE."
+ (service pam-root-service-type base))
--
2.13.0
L
L
Ludovic Courtès wrote on 31 May 2017 15:36
control message for bug #27155
(address . control@debbugs.gnu.org)
87o9u9du70.fsf@gnu.org
severity 27155 important
A
A
Alex Kost wrote on 1 Jun 2017 11:57
Re: bug#27155: [PATCH 0/2] Support service extensions on the "final" service values
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 27155@debbugs.gnu.org)
8760ggrpxm.fsf@gmail.com
Ludovic Courtès (2017-05-30 23:58 +0200) wrote:

Toggle quote (22 lines)
> Hello!
>
> This patch adds support for service extensions that modify the
> "final" values of a service. This is meant to implement cross-cutting
> concerns as well as system-wide customization as discussed with Alex
> long ago:
>
> https://lists.gnu.org/archive/html/guix-devel/2015-11/msg00623.html
> https://lists.gnu.org/archive/html/guix-devel/2016-09/msg01505.html
>
> To summarize, a "finalization extension" (for lack of a better name)
> gets the final value of a service and returns a new value for that
> service. This is in contrast with a "normal" extension which can only
> contribute to the value of a target service, and not inspect the value
> of that target service.
>
> For example, for the /etc service, a "normal" extension can only add
> entries for /etc. A "finalization" extension can instead inspect and
> change all the /etc entries. IOW, it is a sort of a "sudo" for service
> extensions; it's also quite inelegant compared to the "normal" extension
> mechanism, but it's certainly useful.

Definitely!

Toggle quote (28 lines)
> A use case is given in the second patch: we change all the PAM services
> to use pam_elogind.so or pam_limits.so. Likewise, the 'rename-etc-files'
> service below shows how to rename all the files in /etc (for illustration
> purposes only :-)):
>
> (define rename-etc-files
> (let ((rename (lambda (prefix entries)
> (map (match-lambda
> ((name . rest)
> (cons (string-append prefix name)
> rest)))
> entries))))
> (service-type
> (name 'rename-etc-files)
> (extensions (list (service-extension etc-service-type
> (const '())
> rename))))))
>
>
> (operating-system
> ;; ...
> (services (cons* (service rename-etc-files "foo-")
> ...)))
>
> I think this should fulfill the need that Alex had expressed, which is
> to not only be able to add files to /etc, but also to have the ability
> to inspect and modify what goes to /etc.

This is great! Just what I wanted, and thanks for this example! Based
on it, I made the following service:

(define replace-etc/profile-type
(let ((replace
(lambda (file entries)
(cons `("profile" ,file)
(map (match-lambda
((name . rest)
(cons (if (string= name "profile")
(string-append "original-profile")
name)
rest)))
entries)))))
(service-type
(name 'replace-etc/profile)
(extensions (list (service-extension etc-service-type
(const '())
replace))))))

(service replace-etc/profile-type (local-file ".../my-system-profile"))

So now I can use my own "/etc/profile", moreover I can look at the
"/etc/original-profile" anytime. I already use a system with this
service and I enjoy it, thanks a lot!

Toggle quote (3 lines)
> The first patch currently lacks doc. I'll work on it if there's consensus
> on the approach.

I agree with this approach!

--
Alex
L
L
Ludovic Courtès wrote on 1 Jun 2017 13:24
(name . Alex Kost)(address . alezost@gmail.com)(address . 27155@debbugs.gnu.org)
871sr43q89.fsf@gnu.org
Hi Alex,

Alex Kost <alezost@gmail.com> skribis:

Toggle quote (26 lines)
> This is great! Just what I wanted, and thanks for this example! Based
> on it, I made the following service:
>
> (define replace-etc/profile-type
> (let ((replace
> (lambda (file entries)
> (cons `("profile" ,file)
> (map (match-lambda
> ((name . rest)
> (cons (if (string= name "profile")
> (string-append "original-profile")
> name)
> rest)))
> entries)))))
> (service-type
> (name 'replace-etc/profile)
> (extensions (list (service-extension etc-service-type
> (const '())
> replace))))))
>
> (service replace-etc/profile-type (local-file ".../my-system-profile"))
>
> So now I can use my own "/etc/profile", moreover I can look at the
> "/etc/original-profile" anytime. I already use a system with this
> service and I enjoy it, thanks a lot!

Awesome, I’m glad you like it! It was long overdue.

Thanks for taking the time to test!

Ludo’.
L
L
Ludovic Courtès wrote on 3 Jun 2017 23:21
(address . 27155@debbugs.gnu.org)(name . Alex Kost)(address . alezost@gmail.com)
8737bgkbsy.fsf@gnu.org
Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (12 lines)
> This patch adds support for service extensions that modify the
> "final" values of a service. This is meant to implement cross-cutting
> concerns as well as system-wide customization as discussed with Alex
> long ago:
>
> https://lists.gnu.org/archive/html/guix-devel/2015-11/msg00623.html
> https://lists.gnu.org/archive/html/guix-devel/2016-09/msg01505.html
>
> To summarize, a "finalization extension" (for lack of a better name)
> gets the final value of a service and returns a new value for that
> service.

I found a better name: “customizations”.

Toggle quote (6 lines)
> For example, for the /etc service, a "normal" extension can only add
> entries for /etc. A "finalization" extension can instead inspect and
> change all the /etc entries. IOW, it is a sort of a "sudo" for service
> extensions; it's also quite inelegant compared to the "normal" extension
> mechanism, but it's certainly useful.

Not liking the “sudo” aspect of this patch, I thought it would be
natural if service types could control how customizations apply. That
way, the PAM or /etc service could still guarantee, for instance, that
customization does not add or remove entries, and so on.

In the end, this control by the service type makes it easier to reason
about what extensions do, whereas the “sudo” style means that an
extension can alter the service’s value in any possible way.

So I started modifying this patch set to add a ‘customize’ field to
<service-type>, next to ‘extend’. For the PAM and /etc services,
‘customize’ would compose and apply procedures that modify an entry, for
instance.

Then I realized that the only difference between ‘customize’ and
‘extend’ would be the meaning attached to it. IOW, both are some kind
of an extension.

So at this point, I started wondering whether we should just allow
service types to declare several extension points. So for PAM, we’d do:

Toggle snippet (21 lines)
(define pam-service-addition
;; The extension point to add PAM services.
(service-extension-point
(compose concatenate)
(extend append)))

(define pam-service-cutomization
;; The extension point to customize PAM services.
(service-extension-point
(compose compose)
(extend append)))

(define pam-root-service-type
(service-type (name 'pam)
(extensions (list (service-extension etc-service-type
/etc-entry)))

(extension-points (list pam-service-addtion
pam-service-customization))))

But then ‘service-extension’ would need to specify not only the target
service type but also the target extension point, which means more
boilerplate, etc.

So after so much thought and hacking, I feel like the ad hoc solution at
was not that bad after all.

Sorry to bother you with philosophical design questions when we already
have two ways to solve the problem at hand, but I feel like there’s a
pattern worth looking for!

Ludo’.
A
A
Alex Kost wrote on 4 Jun 2017 16:26
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 27155@debbugs.gnu.org)
87o9u3q15q.fsf@gmail.com
Ludovic Courtès (2017-06-03 23:21 +0200) wrote:

Toggle quote (16 lines)
> Ludovic Courtès <ludo@gnu.org> skribis:
>
>> This patch adds support for service extensions that modify the
>> "final" values of a service. This is meant to implement cross-cutting
>> concerns as well as system-wide customization as discussed with Alex
>> long ago:
>>
>> https://lists.gnu.org/archive/html/guix-devel/2015-11/msg00623.html
>> https://lists.gnu.org/archive/html/guix-devel/2016-09/msg01505.html
>>
>> To summarize, a "finalization extension" (for lack of a better name)
>> gets the final value of a service and returns a new value for that
>> service.
>
> I found a better name: “customizations”.

I kinda like "finalization" more :-) But "customization" is fine with
me, not a big deal.

Toggle quote (11 lines)
>> For example, for the /etc service, a "normal" extension can only add
>> entries for /etc. A "finalization" extension can instead inspect and
>> change all the /etc entries. IOW, it is a sort of a "sudo" for service
>> extensions; it's also quite inelegant compared to the "normal" extension
>> mechanism, but it's certainly useful.
>
> Not liking the “sudo” aspect of this patch, I thought it would be
> natural if service types could control how customizations apply. That
> way, the PAM or /etc service could still guarantee, for instance, that
> customization does not add or remove entries, and so on.

Ouch, that's what I don't like. I think a full control is better.
You'll never know what a user might want to do, and giving a user a full
freedom (even to break a system!) would be a great feature. So I'm
against such guarantees that strict users in modifying their systems.

Toggle quote (4 lines)
> In the end, this control by the service type makes it easier to reason
> about what extensions do, whereas the “sudo” style means that an
> extension can alter the service’s value in any possible way.

Right, "any possible way" is exactly what I want!

Toggle quote (36 lines)
> So I started modifying this patch set to add a ‘customize’ field to
> <service-type>, next to ‘extend’. For the PAM and /etc services,
> ‘customize’ would compose and apply procedures that modify an entry, for
> instance.
>
> Then I realized that the only difference between ‘customize’ and
> ‘extend’ would be the meaning attached to it. IOW, both are some kind
> of an extension.
>
> So at this point, I started wondering whether we should just allow
> service types to declare several extension points. So for PAM, we’d do:
>
> (define pam-service-addition
> ;; The extension point to add PAM services.
> (service-extension-point
> (compose concatenate)
> (extend append)))
>
> (define pam-service-cutomization
> ;; The extension point to customize PAM services.
> (service-extension-point
> (compose compose)
> (extend append)))
>
> (define pam-root-service-type
> (service-type (name 'pam)
> (extensions (list (service-extension etc-service-type
> /etc-entry)))
>
> (extension-points (list pam-service-addtion
> pam-service-customization))))
>
> But then ‘service-extension’ would need to specify not only the target
> service type but also the target extension point, which means more
> boilerplate, etc.

I don't have a deep understanding of services, but your suggestion seems
(to me) to have the following downsides:

- More additional work – to determine (and implement) what aspects of
services should and what should not be modified by a user.

- Less freedom (comparing to your previous solution) for users in
modifying services.

Toggle quote (4 lines)
> So after so much thought and hacking, I feel like the ad hoc solution at
> <https://lists.gnu.org/archive/html/guix-devel/2016-09/msg01505.html>
> was not that bad after all.

He-he :-)

Toggle quote (4 lines)
> Sorry to bother you with philosophical design questions when we already
> have two ways to solve the problem at hand, but I feel like there’s a
> pattern worth looking for!

No problem, looking for patterns is always an interesting occupation!
As for me, I agree with any solution that allows me to replace
"/etc/profile". But in general, I vote for that solution that allows
users to customize as much things as possible.

--
Alex
L
L
Ludovic Courtès wrote on 5 Jun 2017 12:06
(name . Alex Kost)(address . alezost@gmail.com)(address . 27155@debbugs.gnu.org)
8760gag344.fsf@gnu.org
Alex Kost <alezost@gmail.com> skribis:

Toggle quote (2 lines)
> Ludovic Courtès (2017-06-03 23:21 +0200) wrote:

[...]

Toggle quote (10 lines)
>> Not liking the “sudo” aspect of this patch, I thought it would be
>> natural if service types could control how customizations apply. That
>> way, the PAM or /etc service could still guarantee, for instance, that
>> customization does not add or remove entries, and so on.
>
> Ouch, that's what I don't like. I think a full control is better.
> You'll never know what a user might want to do, and giving a user a full
> freedom (even to break a system!) would be a great feature. So I'm
> against such guarantees that strict users in modifying their systems.

Just to be clear: I do want users to be able to modify their system as
they see fit. The argument is about how we should structure these
modifications.

In the end, people can always define and use their own services, or even
‘set!’ things. But if we can provide users with control over their
system in a structured way, I think it’s beneficial: they can do complex
customizations of their system and still reason about them.

Toggle quote (36 lines)
>> So at this point, I started wondering whether we should just allow
>> service types to declare several extension points. So for PAM, we’d do:
>>
>> (define pam-service-addition
>> ;; The extension point to add PAM services.
>> (service-extension-point
>> (compose concatenate)
>> (extend append)))
>>
>> (define pam-service-cutomization
>> ;; The extension point to customize PAM services.
>> (service-extension-point
>> (compose compose)
>> (extend append)))
>>
>> (define pam-root-service-type
>> (service-type (name 'pam)
>> (extensions (list (service-extension etc-service-type
>> /etc-entry)))
>>
>> (extension-points (list pam-service-addtion
>> pam-service-customization))))
>>
>> But then ‘service-extension’ would need to specify not only the target
>> service type but also the target extension point, which means more
>> boilerplate, etc.
>
> I don't have a deep understanding of services, but your suggestion seems
> (to me) to have the following downsides:
>
> - More additional work – to determine (and implement) what aspects of
> services should and what should not be modified by a user.
>
> - Less freedom (comparing to your previous solution) for users in
> modifying services.

I see what you mean.

Ludo’, who thinks some more.
R
R
Ricardo Wurmus wrote on 5 Jun 2017 14:52
(address . 27155@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
87mv9m7g0t.fsf@elephly.net
I think it is useful to have the ability to add rewriters at the end of
service composition. In my opinion it is always good to have an escape
hatch, and this seems to fit the bill. But I agree that it is not
an elegant solution, and I wouldn’t want to advocate using it.

As to your second idea: it seems tedious for service writers to have to
anticipate the ways in which services could be extended (here given by
providing extension points).

Would it make more sense to allow *extensions* to specify how they
should be applied rather than letting services define extension points?
This would shift the burden away from services to service extensions.
Extensions would still need to provide a way of extending the parent
service, but this could be optional.

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6 2150 197A 5888 235F ACAC
L
L
Ludovic Courtès wrote on 7 Jun 2017 01:07
(name . Ricardo Wurmus)(address . rekado@elephly.net)(address . 27155@debbugs.gnu.org)
87bmq07m0y.fsf@gnu.org
Hi Ricardo,

Ricardo Wurmus <rekado@elephly.net> skribis:

Toggle quote (5 lines)
> I think it is useful to have the ability to add rewriters at the end of
> service composition. In my opinion it is always good to have an escape
> hatch, and this seems to fit the bill. But I agree that it is not
> an elegant solution, and I wouldn’t want to advocate using it.

Right. As discussed on IRC, one problem is ordering: if there are
several users of this features for a given service, you can’t really
tell what’s going to happen, unless the modifications happen to be
commutable.

Toggle quote (4 lines)
> As to your second idea: it seems tedious for service writers to have to
> anticipate the ways in which services could be extended (here given by
> providing extension points).

Boilerplate aside, I’m not sure it would be this tedious.

Toggle quote (6 lines)
> Would it make more sense to allow *extensions* to specify how they
> should be applied rather than letting services define extension points?
> This would shift the burden away from services to service extensions.
> Extensions would still need to provide a way of extending the parent
> service, but this could be optional.

What would it look like?

It seems to me there are two options: either service type specify how
they can be extended, or they expose their raw values letting any
extension alter it (the patch I sent).

Thanks for your feedback!

Ludo’.
?
?
??? wrote on 15 Jun 2017 19:12
(name . Ludovic Courtès)(address . ludo@gnu.org)
87mv99rx8w.fsf@member.fsf.org
ludo@gnu.org (Ludovic Courtès) writes:

Toggle quote (28 lines)
> Hi Ricardo,
>
> Ricardo Wurmus <rekado@elephly.net> skribis:
>
>> I think it is useful to have the ability to add rewriters at the end of
>> service composition. In my opinion it is always good to have an escape
>> hatch, and this seems to fit the bill. But I agree that it is not
>> an elegant solution, and I wouldn’t want to advocate using it.
>
> Right. As discussed on IRC, one problem is ordering: if there are
> several users of this features for a given service, you can’t really
> tell what’s going to happen, unless the modifications happen to be
> commutable.
>
>> As to your second idea: it seems tedious for service writers to have to
>> anticipate the ways in which services could be extended (here given by
>> providing extension points).
>
> Boilerplate aside, I’m not sure it would be this tedious.
>
>> Would it make more sense to allow *extensions* to specify how they
>> should be applied rather than letting services define extension points?
>> This would shift the burden away from services to service extensions.
>> Extensions would still need to provide a way of extending the parent
>> service, but this could be optional.
>
> What would it look like?

Maybe allow a service to override extensions specified by its
type?

It can be:
Toggle snippet (19 lines)
(define etc-service-type
(service-type
(name 'etc)
(default-extensions (list ...))
(extension-points (list ...))))

(define builtin-etc-service
(... %base-services))

(define my-etc-service
(service etc-service-type
(service-value builtin-etc-service)
#:extensions
(list (service-extension
activation-service-type
activate-my-etc-files-in-my-way)
...)))

So we can change what service actually do, this is really powerful!
L
L
Ludovic Courtès wrote on 21 Jun 2017 15:06
(name . ???)(address . iyzsong@member.fsf.org)
87h8z9ij6t.fsf@gnu.org
Hi!

iyzsong@member.fsf.org (???) skribis:

Toggle quote (25 lines)
> Maybe allow a service to override extensions specified by its
> type?
>
> It can be:
>
> (define etc-service-type
> (service-type
> (name 'etc)
> (default-extensions (list ...))
> (extension-points (list ...))))
>
> (define builtin-etc-service
> (... %base-services))
>
> (define my-etc-service
> (service etc-service-type
> (service-value builtin-etc-service)
> #:extensions
> (list (service-extension
> activation-service-type
> activate-my-etc-files-in-my-way)
> ...)))
>
> So we can change what service actually do, this is really powerful!

The problem as I see it is that this would be redundant with extensions
in service types.

Also, the “etc” service is one of the “special” services that are not in
‘%base-services’; instead they’re automatically added by
‘essential-services’ in (gnu system).

Thanks for your feedback,
Ludo’.
?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 27155@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 27155
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch