[bug#53676] [PATCH v2 2/4] gnu: pulseaudio: Graft to adjust configuration.

[Maxim Cournoyer]

Hi Liliana,

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

> Hi Maxim,
>
> Am Donnerstag, dem 24.02.2022 um 17:00 -0500 schrieb Maxim Cournoyer:
>> Thank you for your continued feedback.  The reason I prefer this
>> simple substitution to a conditional one is two-fold:
>> 
>> 1. It avoids two actors potentially touching the default 'script-
>> file' (the pulseaudio-service-type code as well as the user), which
>> could be unwieldy (do we plug the default.pa.d after their changes to
>> ensure it is there, or before, which means it'd potentially be
>> erased?).  Having it part of the shipped default.pa file makes this
>> simpler to reason with.
> Sure, but all we'd need here is proper documentation.  For the record,
> I would check if a `source /etc/pulse/default.pa.d' is in the user-
> supplied file (even if commented) and append it if not.

OK; I went a bit dumber/safer: when extra-script-files is non-null, the
.include is appended.

>> 2. It allows foreign distribution users to keep their custom user
>> script working even when they use our pulseaudio package (it makes
>> our pulseaudio package behave as intended by upstream).
> That ignores the case where users modify their distro's default.pa
> *and* put stuff into default.pa.d.  This might be necessary in some
> scenarios where the upstream default breaks user expectations.  I'd
> really prefer if foreign distro users just set their environment
> variables, as those work unconditionally as intended.

That sounds a bit hypothetical, but yes, it's a possibility.

>> I wouldn't mind using a feature branch to get the < 2k dependent
>> packages rebuilt as suggested by Leo, if you think that's preferable.
> That would work for the rebuilds, making this not a graft, but I'm
> still concerned whether we really want these semantics or not.  With
> the WebkitGTK bug fixed, we can put our generated default.pa into /etc
> again, making it more debuggable.  My personal opinion is still on
> explicitly declared rather than implicitly assumed.

OK, if we want to add the .include conditionally, I'd go with something
like:

--8<---------------cut here---------------start------------->8---
modified   doc/guix.texi
@@ -21507,7 +21507,10 @@ List of settings to set in @file{daemon.conf}, 
formatted just like
 @var{client-conf}.
 
 @item @code{script-file} (default: @code{(file-append pulseaudio 
"/etc/pulse/default.pa")})
-Script file to use as @file{default.pa}.
+Script file to use as @file{default.pa}.  In case the
+@code{extra-script-files} field below is used, an @code{.include}
+directive pointing to @file{/etc/pulse/default.pa.d} is appended to the
+provided script.
 
 @item @code{extra-script-files} (default: @code{'())})
 A list of file-like objects defining extra PulseAudio scripts to run at
modified   gnu/services/sound.scm
@@ -174,6 +174,21 @@ (define (assert-pulseaudio-script-file-name name)
                      extra-script-files)))
     (file-union "default.pa.d" (zip labels extra-script-files))))
 
+(define (append-include-directive script-file)
+  "Append an include directive to source scripts under 
/etc/pulse/default.pa.d."
+  (computed-file "default.pa"
+                 #~(begin
+                     (use-modules (ice-9 textual-ports))
+                     (define script-text
+                       (call-with-input-file #$script-file get-string-all))
+                     (call-with-output-file #$output
+                       (lambda (port)
+                         (format port (string-append script-text
+                                                     "
+# Added by Guix to include scripts specified in extra-script-files.
+.nofail
+.include /etc/pulse/default.pa.d~%")))))))
+
 (define pulseaudio-etc
   (match-lambda
     (($ <pulseaudio-configuration> client-conf daemon-conf default-script-file
@@ -181,7 +196,10 @@ (define pulseaudio-etc
      `(("pulse"
         ,(file-union
           "pulse"
-          `(("default.pa" ,default-script-file)
+          `(("default.pa"
+             ,(if (null? extra-script-files)
+                  default-script-file
+                  (append-include-directive default-script-file)))
             ("system.pa" ,system-script-file)
             ,@(if (null? extra-script-files)
                   '()
--8<---------------cut here---------------end--------------->8---

A mixed-file as you used previously (combining two .include) could have
been used, but I prefer to have the content directly visible under
/etc/pulse/default.pa (and the shebang line preserved).

This gets rid of the change on the pulseaudio package itself.

What do you think?

Thank you,

Maxim