[bug#53676] [PATCH 3/5] gnu: pulseaudio: Graft to adjust configuration.

[Maxim Cournoyer]

Hello again,

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

> Am Dienstag, dem 01.02.2022 um 15:20 -0500 schrieb Maxim Cournoyer:
>> Hi Liliana,
>> 
>> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>> 
>> > Am Montag, dem 31.01.2022 um 23:19 -0500 schrieb Maxim Cournoyer:
>> > > * gnu/packages/pulseaudio.scm (pulseaudio/fixed): New variable.
>> > > (pulseaudio)[replacement]: Graft package with it.
>> > > ---
>> > >  gnu/packages/pulseaudio.scm | 18 ++++++++++++++++++
>> > >  1 file changed, 18 insertions(+)
>> > > 
>> > > diff --git a/gnu/packages/pulseaudio.scm
>> > > b/gnu/packages/pulseaudio.scm
>> > > index fe028b5202..f529717ee1 100644
>> > > --- a/gnu/packages/pulseaudio.scm
>> > > +++ b/gnu/packages/pulseaudio.scm
>> > > @@ -178,6 +178,7 @@ (define-public libsamplerate
>> > >  (define-public pulseaudio
>> > >    (package
>> > >      (name "pulseaudio")
>> > > +    (replacement pulseaudio/fixed)
>> > >      (version "15.0")
>> > >      (source (origin
>> > >               (method url-fetch)
>> > > @@ -269,6 +270,23 @@ (define-public pulseaudio
>> > >      ;; 'LICENSE' for details.
>> > >      (license l:gpl2+)))
>> > >  
>> > > +(define pulseaudio/fixed
>> > > +  (package
>> > > +    (inherit pulseaudio)
>> > > +    (arguments
>> > > +     (substitute-keyword-arguments (package-arguments
>> > > pulseaudio)
>> > > +       ((#:phases phases)
>> > > +        `(modify-phases ,phases
>> > > +           (add-after 'unpack 'customize-default-script
>> > > +             (lambda _
>> > > +               (call-with-port
>> > > +                (open-file "src/daemon/default.pa.in" "a")
>> > > +                (lambda (port)
>> > > +                  (format port "~%\
>> > > +### Include extra script files configured via the pulseaudio-
>> > > service-type.
>> > > +.nofail
>> > > +.include /etc/pulse/default.pa.d~%")))))))))))
>> > > +
>> > Note that there should be a .fail afterwards.  
>> 
>> Hmm.  I simply duplicated the existing two lines used by PulseAudio
>> itself.  I believe they do not care because it appears completely at
>> the end of the file.
> I think we would need to care though, because one could write a gexp
> that appends to default.pa, but then has unclear semantics.

If someone was to append something to default.pa (the exact one shipped
with PulseAudio), they'd have to add the .fail themselves to undo
PulseAudio's own .nofail, right?  I don't see why we should go out of
our way to change that.

With the proposed 'extra-script-files', I'd argue that appending
something to default.pa should be considered an anti-pattern; as the new
field would be the more natural option to *extend* 'default.pa' (and
having a field to override default.pa is still useful if you don't like
any of the default behavior).

>> > As Leo pointed out, we shouldn't do too many "feature grafts", so
>> > instead it might be worth moving this part to pulseaudio-service-
>> > type in some way, no?
>> 
>> I'd prefer to keep it like this, for simplicity; we need to rebuild
>> the world soon anyway to fix a Rust CVE, so we can batch things
>> easily.
> Can you define "simplicity" here?  In my opinion, services/stuff.scm or
> /etc/config.scm provide an easier point of change/extension than
> packages do -- particularly also because pulseaudio-service-type (even
> with this patch set) does not allow changing the pulseaudio package.

The default behavior of default.pa is to allow loading extra files from
from 'pulsesysconfdir', which in our case corresponds to output/etc of
pulseaudio; e.g.:

--8<---------------cut here---------------start------------->8---
### Allow including a default.pa.d directory, which if present, can be used
### for additional configuration snippets.
.nofail
.include 
/gnu/store/7xwgz4bavb1i8sfx1lm55hlrr3ngjkdx-pulseaudio-15.0/etc/pulse/default.pa.d
--8<---------------cut here---------------end--------------->8---

That's not very useful, but is preserved in case pulseaudio ever decides
to drop their own scripts in there.  Adjusting this path is more natural
and straightforwardly done from the package description than from the
service, in my opinion.

Does that make sense?

Thanks,

Maxim