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

Ludovic Courtès wrote on 30 May 2017 23:58

Recipients:(address . guix-patches@gnu.org)

Message-ID: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:

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.

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

Ludovic Courtès wrote on 31 May 2017 00:05

[PATCH 1/2] DRAFT services: Extensions can specify a "finalization" procedure.

Recipients:(address . 27155@debbugs.gnu.org)

Message-ID: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

Ludovic Courtès wrote on 31 May 2017 00:05

[PATCH 2/2] system: pam: Remove custom API to transform PAM services.

Recipients:(address . 27155@debbugs.gnu.org)

Message-ID: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

Ludovic Courtès wrote on 31 May 2017 15:36

control message for bug #27155

Recipients:(address . control@debbugs.gnu.org)

Message-ID:87o9u9du70.fsf@gnu.org

severity 27155 important

Alex Kost wrote on 1 Jun 2017 11:57

Re: bug#27155: [PATCH 0/2] Support service extensions on the "final" service values

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 27155@debbugs.gnu.org)

Message-ID: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

Ludovic Courtès wrote on 1 Jun 2017 13:24

Recipients:(name . Alex Kost)(address . alezost@gmail.com)(address . 27155@debbugs.gnu.org)

Message-ID: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’.

Ludovic Courtès wrote on 3 Jun 2017 23:21

Recipients:(address . 27155@debbugs.gnu.org)(name . Alex Kost)(address . alezost@gmail.com)

Message-ID: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

https://lists.gnu.org/archive/html/guix-devel/2016-09/msg01505.html

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’.

Alex Kost wrote on 4 Jun 2017 16:26

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 27155@debbugs.gnu.org)

Message-ID: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

Ludovic Courtès wrote on 5 Jun 2017 12:06

Recipients:(name . Alex Kost)(address . alezost@gmail.com)(address . 27155@debbugs.gnu.org)

Message-ID: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.

Ricardo Wurmus wrote on 5 Jun 2017 14:52

Recipients:(address . 27155@debbugs.gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID: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

https://elephly.net

Ludovic Courtès wrote on 7 Jun 2017 01:07

Recipients:(name . Ricardo Wurmus)(address . rekado@elephly.net)(address . 27155@debbugs.gnu.org)

Message-ID: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

Recipients:(name . Ludovic Courtès)(address . ludo@gnu.org)

Message-ID: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!

Ludovic Courtès wrote on 21 Jun 2017 15:06

Recipients:(name . 宋文武)(address . iyzsong@member.fsf.org)

Message-ID: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 email to 27155@debbugs.gnu.org

is:open	open issues
is:done	closed issues
submitter:<who>	search issue submitter
author:<who>	search by message author
date:yesterday..now	search by issue date
mdate:3m..2d	search by message date