[pulseaudio-discuss] [PATCH v2 1/2] conf-parser: add support for .d directories

[Tanu Kaskinen]

On Mon, 2015-12-07 at 13:40 -0300, Felipe Sateler wrote:
> On 6 December 2015 at 20:51, Tanu Kaskinen <tanuk at iki.fi> wrote:
> > On Sat, 2015-12-05 at 19:50 +0200, Tanu Kaskinen wrote:
> > > On Sat, 2015-12-05 at 12:24 -0300, Felipe Sateler wrote:
> > > > On 4 December 2015 at 09:46, Tanu Kaskinen <tanuk at iki.fi> wrote:
> > > > > On Fri, 2015-12-04 at 09:29 -0300, Felipe Sateler wrote:
> > > > > > On 4 December 2015 at 02:58, Tanu Kaskinen <tanuk at iki.fi> wrote:
> > > > > > > + * If use_dot_d is true, then before parsing the file named by the filename
> > > > > > > + * argument, the function will parse all files ending with ".conf" in
> > > > > > > + * alphabetical order from a directory whose name is filename + ".d", if such
> > > > > > > + * directory exists.
> > > > > > 
> > > > > > This is opposite to how other software (eg systemd) work: first the
> > > > > > main file is read, and then the .d/*.conf files are read to override
> > > > > > the configurations.
> > > > > > 
> > > > > > So if a distro default has an uncommented line, the .d should override
> > > > > > that line. This would allow (in the future) to extend the search path
> > > > > > to /usr/share/pulseaudio, and ship the defaults (uncommented) there.
> > > > > > Then local configuration can be done via dropin files.
> > > > > 
> > > > > All advice given to users so far has been to modify the main files,
> > > > > because nothing else has existed. If users put their changes to the
> > > > > main files and distros put their changes to the .d files, that will be
> > > > > compatible with existing advice on the internet.
> > > > 
> > > > The alternative does not preclude editing the main file either.
> > > > 
> > > > Maybe it would be best to step back a bit. Desirable properties for
> > > > the configuration loading are (IMO, of course):
> > > > 
> > > > 1. Any upstream default should be easily overridable by a downstream
> > > >    distributor.
> > > > 2. Any upstream/distribution default should be easily overridable by a
> > > >    local admin.
> > > > 3. Any upstream/distribution/admin default should be easily
> > > >    overridable by a local user.
> > > > 4. Admin overriding a setting should not require writing all the options again.
> > > > 5. Local user overriding a setting should not require writing all the
> > > >     options again.
> > > > 6. Third party packages should be able to provide configuration,
> > > >     without overriding other options.
> > > > 7. Local admin/user should be able to override such configuration.
> > > > 8. Local admin changes to configuration should be clearly visible.
> > > 
> > > I agree on all of these.
> > > 
> > > > Currently, pulseaudio only covers 1-3. Your proposed patch addresses
> > > > point 6, but only in the case the distro default is "everything
> > > > commented".
> > > 
> > > I guess you mean that my proposal breaks if the distro overrides
> > > upstream defaults in the main file, because packages can't override
> > > those in .d files.
> > > 
> > > My suggestion is to not patch the main files in distros. Instead, put
> > > distro defaults that diverge from the upstream defaults to .d files.
> > > Use 00- prefix to make these defaults have the lowest precedence, and
> > > use higher numbers in third-party packages.
> > > 
> > > > If the load order is inverted, then there is a path to achieving all 8
> > > > properties:
> > > > 
> > > > - If the dropins were read after the main configuration file, then 6
> > > >   would be fully addressed.
> > > > - If then the path loading is extended to load dropins in the entire
> > > >   search path, keeping only the highest precedence one, then point 4 and
> > > >   5 would be addressed.
> > > 
> > > I guess you mean merging the file list in all .d directories before
> > > starting processing. For example:
> > > 
> > > $datadir/client.conf.d/10-foo.conf
> > > $sysconfdir/client.conf.d/30-bar.conf
> > > $userconfdir/client.conf.d/20-blub.conf
> > > 
> > > The processing order would be 10-foo.conf -> 20-blub.conf -> 30-
> > > bar.conf. That ordering is needed when dealing with scripts (like udev
> > > configuration): 10-foo.conf may set some variable that the admin uses
> > > as a condition in 30-bar.conf. If the user is happy with the code in
> > > 30-bar.conf, but wants to change the variable set in 10-foo.conf before
> > > 30-bar.conf uses it, the user has to be able to insert code in between.
> > > 
> > > When dealing with plain key-value stuff without conditional constructs,
> > > however, this kind of "insert in between" support is not needed. In
> > > this situation I prefer to just load everything from $datadir first,
> > > then from $sysconfdir, and finally from $userconfdir. Simple to
> > > understand, simple to implement.
> 
> Hmm. This looks very reasonable. However, it would not allow using the
> same scheme for loading .pa files (and thus a module package could
> ship a .pa.d/ snippet to load itself).
> 
> > > 
> > > > - If then the loading stops looking at the search path at the first
> > > >   file with same name found, then point 7 would be addressed.
> > > > - And then point 8 can be implemented by shipping the full
> > > >   configuration file, uncommented, in $datadir. Thus only admin changes
> > > >   would live in /etc, and local user changes would live in ~/.config.
> > > > 
> > > > Not inverting the change, though, makes it cumbersome to provide a
> > > > distro default that can be overriden in a package: some arbitrary
> > > > prefix should be determined so that packages ship their defaults
> > > > ordered after that.
> > > 
> > > I'm not sure what you mean. Do you mean that if distro defaults are in
> > > client.conf.d/00-distrodefaults.conf, it's hard to make sure that
> > > packages ship files that are ordered after that?
> 
> 
> I mean that 00-distrodefaults would be a convention, and I'm pretty
> sure not everyone will follow it.
> 
> > > 
> > > > This will necessarily be distro-dependent. It also
> > > > makes it harder to have properties 4 and 5, since a setting written in
> > > > the main file necessarily requires modifying or copying it.
> > > 
> > > Why would overriding a setting in the main file require modifying or
> > > copying the file? Do you refer to the masking, i.e. if
> > > $userconfir/client.conf exists, $sysconfdir/client.conf won't be read?
> > > I want to get rid of the masking eventually. Both files should be read.
> 
> I don't think I agree here. Especially if this can be extended to .pa
> files later.

The .pa files will have to use different logic. Or more preferably, we
should get rid of those files altogether. I believe there's a consensus
among the maintainers that it would be good to replace the startup
scripts with regular configuration files.

> > > > This is the way many other packages work (eg, systemd and dbus), so
> > > > people already familiar with that configuration would be surprised by
> > > > pa's scheme.
> > > 
> > > You mean that $userconfdir/mainfile.conf masks
> > > $sysconfdir/mainfile.conf in systemd? If so, I certainly don't want to
> > > (continue to) copy systemd.
> > > 
> > > > There is a small wrinkle to observe with this load order: if a
> > > > package ships a setting in a dropin file, then the user needs to
> > > > override that dropin (or ship a new one with a higher ordering) for
> > > > his change to work, because the dropin is processed after the main
> > > > file. For example, a dropin in /etc that sets autospawn=no, will take
> > > > precedence over the main file in ~/.config/pulse that says
> > > > autospawn=yes.
> > > > 
> > > > This can be addressed by making the dropins from later in the
> > > > search path be loaded before the main file being read (eg, a dropin in
> > > > /etc would be loaded before reading ~/.config/pulse/client.conf). This
> > > > is not done AFAIK in other software; I'm not sure it is a good idea
> > > > to deviate.
> > > 
> > > I had some trouble following what scheme you were talking about in
> > > different parts of your mail,
> 
> Sorry, I'll try to be clearer in the future.
> 
> > > but I got the impression that you
> > > basically have two objections to my patch: the scheme is different from
> > > systemd,
> 
> Not only systemd, also udev and dbus. Loading dropins after the main
> file is also how (at least on debian) apache and Xsession load
> dropins.
> 
> The only counterexample to this pattern that I've seen is apt when
> loading the apt.conf.d snippets.
> 
> > > and it doesn't work when upstream ships its defaults in the
> > > main files. The solution to the latter is simple: ship the distro
> > > defaults in .d files.
> > > 
> > > Being different from systemd is a valid concern. However, if I
> > > understood corrently, the "systemd scheme" implies masking with the
> > > main files just like we currently do, and I think that sucks badly
> > > enough to justify not caring about the systemd scheme.
> > 
> > I'm starting to have doubts about the superiority of loading the main
> > file after the .d files... You have advocated putting the upstream
> > defaults to the main file under $datadir instead of having everything
> > commented out. I have dismissed that, because it doesn't have any
> > particular benefit when talking about client.conf and daemon.conf,
> > since they can be empty as well. But it occurred to me that the alsa
> > mixer files can't be empty by default. Overriding the alsa mixer
> > defaults from .d files wouldn't work with my original proposal.
> 
> And also for {system,default}.pa.
> 
> > Third-party packages aren't likely to extend the alsa mixer files
> > (these patches won't allow that anyway), but in principle I don't see
> > any reason why we couldn't some day have non-empty default
> > configuration files that should be overridable from .d files.
> 
> Since I joined this list, I have seen suggestions to modify the paths
> a few times. So supporting /etc overrides fror mixer paths could be
> useful.

Yes. There's probably no need for .d files, however (but I'm not
against it if people would prefer enabling .d files for the mixer files
as well).

> > The problem with loading the main file before the .d files is that user
> > configuration should be writable to the main file without worrying that
> > the changes don't have any effect due to overrides in .d files that the
> > user isn't aware of. That was the main reason why I wrote the patch as
> > I did. But now I realize that in practice .d files don't get written to
> > $userconfdir behind the user's back, because the .d files get installed
> > in $sysconfdir.
> 
> Ideally, in $datadir if shipped by distro packages.
> 
> > If $userconfdir has .d files, they are written by the
> > user, and if that happens, we can assume that the user knows the
> > semantics.
> > 
> > The same would apply to the admin-controlled files in $sysconfdir, if
> > we supported $datadir as a configuration source, but we don't. That
> > means that there may be confusion due to .d files being installed in
> > $sysconfdir, which override the admin config in the main file. That
> > makes me think that it would be best to first implement loading
> > configuration from $datadir, and also remove the masking, and only then
> > apply these patches (modified to reverse the file loading order). Is it
> > a good idea to wait for that to happen, though? Or should we apply
> > these patches first (again, modified to reverse order), and just live
> > with the possible confusion that .d files in $sysconfdir can cause for
> > admins?
> 
> I don't think the confusion would be large, because this is way many
> other software works. Adding a comment to the default all-comments
> configuration file would be helpful to avoid confusion as well.

Ok, I'll submit v3 with reversed ordering, and hopefully we can get
that in 8.0.

-- 
Tanu