From debbugs-submit-bounces@debbugs.gnu.org Fri Mar 24 11:55:54 2023 Received: (at 62298) by debbugs.gnu.org; 24 Mar 2023 15:55:54 +0000 Received: from localhost ([127.0.0.1]:41185 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pfjli-0005cf-8F for submit@debbugs.gnu.org; Fri, 24 Mar 2023 11:55:54 -0400 Received: from mail-vs1-f46.google.com ([209.85.217.46]:36529) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pfjld-0005cP-Dm for 62298@debbugs.gnu.org; Fri, 24 Mar 2023 11:55:53 -0400 Received: by mail-vs1-f46.google.com with SMTP id dc30so1371687vsb.3 for <62298@debbugs.gnu.org>; Fri, 24 Mar 2023 08:55:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679673334; h=mime-version:user-agent:message-id:in-reply-to:date:references :subject:cc:to:from:from:to:cc:subject:date:message-id:reply-to; bh=lyDuC44buCZGKN3Ar8lVgRSmiPfGtLdn0XY1gRdfkG4=; b=UwN6DgtsrAYcredyroX6vYyC2Zs5oJ4e77wi8H2swSw4l+tP8ViX96nzO3hyzESAux bNvG9Or0nyit8+TvAuP5wPfrdiw688EDRYZscKS1rmTnSv1CJwmLwrWNqB0g4l6z+TN5 iokKMxEuiPFpBvKGEUB7YV1vDffE60jLhfmjp3NzqT/5WaPVUl1PUUg/ggnrvtaJOceE MnZ+mUZyqpehL+AJG36fQOCmDUWJCjkfPF5loBEj8pNRESA7i2WG47rlo9SRAgjkjvEE Tghl9hf2EFGZx+l/V4JqhpbTB8tcW0jFALpq9FXHfl37AshVtbP8KoTZ1j3IeE2IqXNI Ofsw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679673334; h=mime-version:user-agent:message-id:in-reply-to:date:references :subject:cc:to:from:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=lyDuC44buCZGKN3Ar8lVgRSmiPfGtLdn0XY1gRdfkG4=; b=255KewGXmwfLP2vhpmRVygMMCeJ4fELxX6AtJ2j/Xml+hLBpBFlWFu+qZCkRM1Hdfd baUdoBm0kzhJ6pWs/W0MrvRkGACnWBx993uwjq53CsSA8BgClggoMjlDnz5n9gO3Y9VT S3mCo5jgjhkgz2oASH0cDv73p3vuaPyujaMnUeBZJ1cdnAR7bH6beN0SqLTFHqx+IyDc Ht8+vzNfbeAkl5Hpur6i/IzFqcKwNmK3Uot53RqVT0sI0xZhhniZNY8g4g9KU9GJTfdo OFFm7G0ygv8r5uaxAzEGh2ASnZkAAC7beAJJNtQT8uvWmCYmqmQdeJt5c8Pfk6jPEnxK oxng== X-Gm-Message-State: AAQBX9e41HpJVZIp4pfkgP9p4BUk+qkDj+K8HiQ22Wv9hM0MshyS3off 99YxsNnYKXaTFrTsuWHOAsQrhAUpAxha0g== X-Google-Smtp-Source: AKy350YyuaYiHyRUpqtAP66Xj3wgRyssjAUlkd9CGj/TrfNZTk1JWlXvyeaugbaMAucpkohifu3wcA== X-Received: by 2002:ad4:5c88:0:b0:5ad:47a1:1fd8 with SMTP id o8-20020ad45c88000000b005ad47a11fd8mr4511683qvh.50.1679671866968; Fri, 24 Mar 2023 08:31:06 -0700 (PDT) Received: from hurd (dsl-155-54.b2b2c.ca. [66.158.155.54]) by smtp.gmail.com with ESMTPSA id ob19-20020a0562142f9300b005dd8b9345ecsm760915qvb.132.2023.03.24.08.31.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Mar 2023 08:31:06 -0700 (PDT) From: Maxim Cournoyer To: Bruno Victal Subject: Re: [PATCH v2 7/8] services: mpd: Use user-account (resp. user-group) for user (resp. group) fields. References: Date: Fri, 24 Mar 2023 11:31:05 -0400 In-Reply-To: (Bruno Victal's message of "Thu, 23 Mar 2023 15:02:17 +0000") Message-ID: <87bkkiyxmu.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Score: 0.0 (/) X-Debbugs-Envelope-To: 62298 Cc: 62298@debbugs.gnu.org, ludo@gnu.org, liliana.prikler@gmail.com X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.0 (-) Hello! Bruno Victal 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 . > > * 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 (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. -- Thanks, Maxim