Message ID | d6b3eb6c374d02a3f6d415da4c9a87defdbdada0.1679583701.git.mirai@makinata.eu |
---|---|
State | New |
Headers | show |
Series | [bug#62298,v2,1/8] services: configuration: Add user-defined sanitizer support. | expand |
Am Donnerstag, dem 23.03.2023 um 15:02 +0000 schrieb Bruno Victal: > If a string value is encountered, it is ignored and a predefined > variable is used instead. This is essentially a rollback to how it > used to be before '5c5f0fc1135ff15f9c4adfc5f27eadd9a592b5d1'. As far as I can see, this is not actually what happens. Don't forget to update your commit messages :) > Fixes #61570 <https://issues.guix.gnu.org/61570>. You only need one newline after this one imho. > --- a/gnu/services/audio.scm > +++ b/gnu/services/audio.scm > @@ -140,6 +140,14 @@ (define (uglify-field-name field-name) > (define list-of-symbol? > (list-of symbol?)) > > +;; helpers for deprecated field types, to be removed later > +(define %lazy-group (make-symbol "%lazy-group")) > + > +(define (inject-group-into-user user group) > + (user-account > + (inherit user) > + (group (user-group-name group)))) > + > > ;;; > ;;; MPD > @@ -559,17 +620,11 @@ (define (mpd-shepherd-service config) > > (define (mpd-accounts config) > (match-record config <mpd-configuration> (user group) > - (list (user-group > - (name group) > - (system? #t)) > - (user-account > - (name user) > - (group group) > - (system? #t) > - (comment "Music Player Daemon (MPD) user") > - ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its > data > - (home-directory "/var/lib/mpd") > - (shell (file-append shadow "/sbin/nologin")))))) > + ;; TODO: deprecation code, to be removed > + (let ((user (if (eq? (user-account-group user) %lazy-group) > + (inject-group-into-user user group) > + user))) > + (list user group)))) A little over-engineered, but works for me :) Cheers
Hello! Bruno Victal <mirai@makinata.eu> writes: > services: mpd: Use user-account (resp. user-group) for user (resp. group) fields. Please keep your git summary lines under 80 chars. > Deprecate using strings for these fields and prefer user-account (resp. user-group) > instead to avoid duplication within account-service-type. > If a string value is encountered, it is ignored and a predefined variable > is used instead. This is essentially a rollback to how it used to be before > '5c5f0fc1135ff15f9c4adfc5f27eadd9a592b5d1'. > > Fixes #61570 <https://issues.guix.gnu.org/61570>. > > * gnu/services/audio.scm (mpd-serialize-user-account, mpd-serialize-user-group) > (mpd-user-sanitizer, mpd-group-sanitizer): New procedure. > (%mpd-user, %mpd-group): New variable. > (mpd-configuration)[user, group]: Set value type to user-account (resp. user-group). > (mpd-shepherd-service): Adapt for user-account values in user field. > (mpd-accounts): Adapt for user-account (resp. user-group) in user (resp. group) field. Watch the 80 characters mark :-). Probably a good idea to configure your editor to automatically wrap lines at that mark. > > * doc/guix.texi (Audio Services): Update documentation. > --- > doc/guix.texi | 4 +- > gnu/services/audio.scm | 89 ++++++++++++++++++++++++++++++++++-------- > 2 files changed, 74 insertions(+), 19 deletions(-) > > diff --git a/doc/guix.texi b/doc/guix.texi > index af9f7d78c0..520a65b0b1 100644 > --- a/doc/guix.texi > +++ b/doc/guix.texi > @@ -33491,10 +33491,10 @@ Audio Services > @item @code{package} (default: @code{mpd}) (type: file-like) > The MPD package. > > -@item @code{user} (default: @code{"mpd"}) (type: string) > +@item @code{user} (type: maybe-user-account) > The user to run mpd as. > > -@item @code{group} (default: @code{"mpd"}) (type: string) > +@item @code{group} (type: maybe-user-group) > The group to run mpd as. > > @item @code{shepherd-requirement} (default: @code{()}) (type: list-of-symbol) > diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm > index 198157a83b..c168d1481c 100644 > --- a/gnu/services/audio.scm > +++ b/gnu/services/audio.scm > @@ -140,6 +140,14 @@ (define (uglify-field-name field-name) > (define list-of-symbol? > (list-of symbol?)) > > +;; helpers for deprecated field types, to be removed later > +(define %lazy-group (make-symbol "%lazy-group")) > + > +(define (inject-group-into-user user group) > + (user-account > + (inherit user) > + (group (user-group-name group)))) nitpick: I'd prefer the plainer language 'set-user-group'. > > ;;; > ;;; MPD > @@ -164,9 +172,32 @@ (define mpd-serialize-boolean mpd-serialize-field) > (define (mpd-serialize-list-of-strings field-name value) > #~(string-append #$@(map (cut mpd-serialize-string field-name <>) value))) > > +(define (mpd-serialize-user-account field-name value) > + (mpd-serialize-string field-name (user-account-name value))) > + > +(define (mpd-serialize-user-group field-name value) > + (mpd-serialize-string field-name (user-group-name value))) > + > (define-maybe string (prefix mpd-)) > (define-maybe list-of-strings (prefix mpd-)) > (define-maybe boolean (prefix mpd-)) > +(define-maybe user-account (prefix mpd-)) > +(define-maybe user-group (prefix mpd-)) > + > +(define %mpd-user > + (user-account > + (name "mpd") > + (group "mpd") > + (system? #t) > + (comment "Music Player Daemon (MPD) user") > + ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data > + (home-directory "/var/lib/mpd") > + (shell (file-append shadow "/sbin/nologin")))) > + > +(define %mpd-group > + (user-group > + (name "mpd") > + (system? #t))) > > ;;; TODO: Procedures for deprecated fields, to be removed. > > @@ -197,6 +228,33 @@ (define (mpd-serialize-port field-name value) > > (define-maybe port (prefix mpd-)) > > +;;; procedures for unsupported value types, to be removed. Please make this a complete sentence, and clarify this is related to a deprecated usage? > +(define (mpd-user-sanitizer value) > + (cond ((user-account? value) value) > + ((string? value) > + (warning (G_ "string value for 'user' is deprecated, use \ > +user-account instead~%")) > + (user-account > + (inherit %mpd-user) > + (name value) > + ;; XXX: this is to be lazily substituted in (mpd-accounts) > + ;; with the value from 'group'. Nitpick: XXX: This (with capitalized first letter), and no hanging indent for continued line. > + (group %lazy-group))) > + (else > + (configuration-field-error #f 'user value)))) > + > +(define (mpd-group-sanitizer value) > + (cond ((user-group? value) value) > + ((string? value) > + (warning (G_ "string value for 'group' is deprecated, use \ > +user-group instead~%")) > + (user-group > + (inherit %mpd-group) > + (name value))) > + (else > + (configuration-field-error #f 'group value)))) > + > ;;; > > ;; Generic MPD plugin record, lists only the most prevalent fields. > @@ -347,12 +405,14 @@ (define-configuration mpd-configuration > empty-serializer) > > (user > - (string "mpd") > - "The user to run mpd as.") > + (maybe-user-account %mpd-user) > + "The user to run mpd as." > + (sanitizer mpd-user-sanitizer)) > > (group > - (string "mpd") > - "The group to run mpd as.") > + (maybe-user-group %mpd-group) > + "The group to run mpd as." > + (sanitizer mpd-group-sanitizer)) > > (shepherd-requirement > (list-of-symbol '()) > @@ -516,7 +576,8 @@ (define (mpd-shepherd-service config) > log-file playlist-directory > db-file state-file sticker-file > environment-variables) > - (let* ((config-file (mpd-serialize-configuration config))) > + (let ((config-file (mpd-serialize-configuration config)) > + (username (user-account-name user))) > (shepherd-service > (documentation "Run the MPD (Music Player Daemon)") > (requirement `(user-processes loopback ,@shepherd-requirement)) > @@ -525,7 +586,7 @@ (define (mpd-shepherd-service config) > (and=> #$(maybe-value log-file) > (compose mkdir-p dirname)) > > - (let ((user (getpw #$user))) > + (let ((user (getpw #$username))) > (for-each > (lambda (x) > (when (and x (not (file-exists? x))) > @@ -559,17 +620,11 @@ (define (mpd-shepherd-service config) > > (define (mpd-accounts config) > (match-record config <mpd-configuration> (user group) > - (list (user-group > - (name group) > - (system? #t)) > - (user-account > - (name user) > - (group group) > - (system? #t) > - (comment "Music Player Daemon (MPD) user") > - ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data > - (home-directory "/var/lib/mpd") > - (shell (file-append shadow "/sbin/nologin")))))) > + ;; TODO: deprecation code, to be removed nitpick: Please use a complete sentence for this stand-alone comment. > + (let ((user (if (eq? (user-account-group user) %lazy-group) > + (inject-group-into-user user group) > + user))) > + (list user group)))) > > (define mpd-service-type > (service-type The rest LGTM, with consideration to Liliana's remarks.
diff --git a/doc/guix.texi b/doc/guix.texi index af9f7d78c0..520a65b0b1 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -33491,10 +33491,10 @@ Audio Services @item @code{package} (default: @code{mpd}) (type: file-like) The MPD package. -@item @code{user} (default: @code{"mpd"}) (type: string) +@item @code{user} (type: maybe-user-account) The user to run mpd as. -@item @code{group} (default: @code{"mpd"}) (type: string) +@item @code{group} (type: maybe-user-group) The group to run mpd as. @item @code{shepherd-requirement} (default: @code{()}) (type: list-of-symbol) diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm index 198157a83b..c168d1481c 100644 --- a/gnu/services/audio.scm +++ b/gnu/services/audio.scm @@ -140,6 +140,14 @@ (define (uglify-field-name field-name) (define list-of-symbol? (list-of symbol?)) +;; helpers for deprecated field types, to be removed later +(define %lazy-group (make-symbol "%lazy-group")) + +(define (inject-group-into-user user group) + (user-account + (inherit user) + (group (user-group-name group)))) + ;;; ;;; MPD @@ -164,9 +172,32 @@ (define mpd-serialize-boolean mpd-serialize-field) (define (mpd-serialize-list-of-strings field-name value) #~(string-append #$@(map (cut mpd-serialize-string field-name <>) value))) +(define (mpd-serialize-user-account field-name value) + (mpd-serialize-string field-name (user-account-name value))) + +(define (mpd-serialize-user-group field-name value) + (mpd-serialize-string field-name (user-group-name value))) + (define-maybe string (prefix mpd-)) (define-maybe list-of-strings (prefix mpd-)) (define-maybe boolean (prefix mpd-)) +(define-maybe user-account (prefix mpd-)) +(define-maybe user-group (prefix mpd-)) + +(define %mpd-user + (user-account + (name "mpd") + (group "mpd") + (system? #t) + (comment "Music Player Daemon (MPD) user") + ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data + (home-directory "/var/lib/mpd") + (shell (file-append shadow "/sbin/nologin")))) + +(define %mpd-group + (user-group + (name "mpd") + (system? #t))) ;;; TODO: Procedures for deprecated fields, to be removed. @@ -197,6 +228,33 @@ (define (mpd-serialize-port field-name value) (define-maybe port (prefix mpd-)) +;;; procedures for unsupported value types, to be removed. + +(define (mpd-user-sanitizer value) + (cond ((user-account? value) value) + ((string? value) + (warning (G_ "string value for 'user' is deprecated, use \ +user-account instead~%")) + (user-account + (inherit %mpd-user) + (name value) + ;; XXX: this is to be lazily substituted in (mpd-accounts) + ;; with the value from 'group'. + (group %lazy-group))) + (else + (configuration-field-error #f 'user value)))) + +(define (mpd-group-sanitizer value) + (cond ((user-group? value) value) + ((string? value) + (warning (G_ "string value for 'group' is deprecated, use \ +user-group instead~%")) + (user-group + (inherit %mpd-group) + (name value))) + (else + (configuration-field-error #f 'group value)))) + ;;; ;; Generic MPD plugin record, lists only the most prevalent fields. @@ -347,12 +405,14 @@ (define-configuration mpd-configuration empty-serializer) (user - (string "mpd") - "The user to run mpd as.") + (maybe-user-account %mpd-user) + "The user to run mpd as." + (sanitizer mpd-user-sanitizer)) (group - (string "mpd") - "The group to run mpd as.") + (maybe-user-group %mpd-group) + "The group to run mpd as." + (sanitizer mpd-group-sanitizer)) (shepherd-requirement (list-of-symbol '()) @@ -516,7 +576,8 @@ (define (mpd-shepherd-service config) log-file playlist-directory db-file state-file sticker-file environment-variables) - (let* ((config-file (mpd-serialize-configuration config))) + (let ((config-file (mpd-serialize-configuration config)) + (username (user-account-name user))) (shepherd-service (documentation "Run the MPD (Music Player Daemon)") (requirement `(user-processes loopback ,@shepherd-requirement)) @@ -525,7 +586,7 @@ (define (mpd-shepherd-service config) (and=> #$(maybe-value log-file) (compose mkdir-p dirname)) - (let ((user (getpw #$user))) + (let ((user (getpw #$username))) (for-each (lambda (x) (when (and x (not (file-exists? x))) @@ -559,17 +620,11 @@ (define (mpd-shepherd-service config) (define (mpd-accounts config) (match-record config <mpd-configuration> (user group) - (list (user-group - (name group) - (system? #t)) - (user-account - (name user) - (group group) - (system? #t) - (comment "Music Player Daemon (MPD) user") - ;; MPD can use $HOME (or $XDG_CONFIG_HOME) to place its data - (home-directory "/var/lib/mpd") - (shell (file-append shadow "/sbin/nologin")))))) + ;; TODO: deprecation code, to be removed + (let ((user (if (eq? (user-account-group user) %lazy-group) + (inject-group-into-user user group) + user))) + (list user group)))) (define mpd-service-type (service-type