From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
To: Liliana Marie Prikler <liliana.prikler@gmail.com>
Cc: 53676@debbugs.gnu.org
Subject: [bug#53676] [PATCH v2 2/4] gnu: pulseaudio: Graft to adjust configuration.
Date: Sat, 26 Feb 2022 01:21:44 -0500 [thread overview]
Message-ID: <87zgmew3k7.fsf@gmail.com> (raw)
In-Reply-To: <103c1a1c803c3d8ae73dffa1e4ee9a71bc28d33b.camel@gmail.com> (Liliana Marie Prikler's message of "Fri, 25 Feb 2022 06:20:37 +0100")
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
next prev parent reply other threads:[~2022-02-26 6:22 UTC|newest]
Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-01 4:13 [bug#53676] [PATCH 0/5] *** PulseAudio service improvements *** Maxim Cournoyer
2022-02-01 4:19 ` [bug#53676] [PATCH 1/5] doc: Fix typo Maxim Cournoyer
2022-02-01 4:19 ` [bug#53676] [PATCH 2/5] services/sound: Normalize pulseaudio-configuration accessor names Maxim Cournoyer
2022-02-01 19:48 ` Liliana Marie Prikler
2022-02-01 20:18 ` Maxim Cournoyer
2022-02-01 21:29 ` Liliana Marie Prikler
2022-02-01 4:19 ` [bug#53676] [PATCH 3/5] gnu: pulseaudio: Graft to adjust configuration Maxim Cournoyer
2022-02-01 19:45 ` Liliana Marie Prikler
2022-02-01 20:20 ` Maxim Cournoyer
2022-02-01 21:37 ` Liliana Marie Prikler
2022-02-02 4:30 ` Maxim Cournoyer
2022-02-02 20:43 ` Liliana Marie Prikler
2022-02-06 6:30 ` [bug#53676] [PATCH 0/5] *** PulseAudio service improvements *** Maxim Cournoyer
2022-02-06 9:07 ` Liliana Marie Prikler
2022-02-24 16:31 ` Maxim Cournoyer
2022-02-24 20:26 ` Liliana Marie Prikler
2022-02-01 4:19 ` [bug#53676] [PATCH 4/5] services: pulseaudio: Add an extra-script-files configuration field Maxim Cournoyer
2022-02-01 19:56 ` Liliana Marie Prikler
2022-02-01 20:27 ` Maxim Cournoyer
2022-02-01 21:26 ` Liliana Marie Prikler
2022-02-02 3:44 ` Maxim Cournoyer
2022-02-02 20:07 ` Liliana Marie Prikler
2022-02-06 7:25 ` Maxim Cournoyer
2022-02-06 8:02 ` Liliana Marie Prikler
2022-02-24 16:25 ` Maxim Cournoyer
2022-02-01 4:19 ` [bug#53676] [PATCH 5/5] services: pulseaudio: Deploy the configuration files to /etc/pulse Maxim Cournoyer
2022-02-01 19:43 ` Liliana Marie Prikler
2022-02-02 22:43 ` Jack Hill
2022-02-07 22:29 ` [bug#53676] [PATCH 0/5] *** PulseAudio service improvements *** Maxim Cournoyer
2022-02-08 5:21 ` Liliana Marie Prikler
2022-02-08 14:25 ` Maxim Cournoyer
2022-02-08 19:31 ` Liliana Marie Prikler
2022-02-08 14:29 ` Maxim Cournoyer
2022-02-08 10:12 ` Maxime Devos
2022-02-08 14:27 ` Maxim Cournoyer
2022-02-24 16:36 ` Maxim Cournoyer
2022-02-24 14:42 ` [bug#53676] [PATCH 5/5] services: pulseaudio: Deploy the configuration files to /etc/pulse Maxim Cournoyer
2022-02-01 19:49 ` [bug#53676] [PATCH 1/5] doc: Fix typo Liliana Marie Prikler
2022-02-01 4:24 ` [bug#53676] [PATCH 0/5] *** PulseAudio service improvements *** Leo Famulari
2022-02-01 20:15 ` Maxim Cournoyer
2022-02-24 16:38 ` [bug#53676] [PATCH v2 1/4] services/sound: Normalize pulseaudio-configuration accessor names Maxim Cournoyer
2022-02-24 16:38 ` [bug#53676] [PATCH v2 2/4] gnu: pulseaudio: Graft to adjust configuration Maxim Cournoyer
2022-02-24 19:47 ` Liliana Marie Prikler
2022-02-24 22:00 ` Maxim Cournoyer
2022-02-25 5:20 ` Liliana Marie Prikler
2022-02-26 6:21 ` Maxim Cournoyer [this message]
2022-02-26 13:19 ` Liliana Marie Prikler
2022-02-26 14:14 ` bug#53676: " Maxim Cournoyer
2022-02-24 16:38 ` [bug#53676] [PATCH v2 3/4] services: pulseaudio: Add an extra-script-files configuration field Maxim Cournoyer
2022-02-24 18:53 ` Maxime Devos
2022-02-24 22:20 ` Maxim Cournoyer
2022-02-24 16:38 ` [bug#53676] [PATCH v2 4/4] services: pulseaudio: Deploy the configuration files to /etc/pulse Maxim Cournoyer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87zgmew3k7.fsf@gmail.com \
--to=maxim.cournoyer@gmail.com \
--cc=53676@debbugs.gnu.org \
--cc=liliana.prikler@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).