Re: [PATCH 3/3] pulseaudio: fix to manage user services corretly

[Tanu Kaskinen <tanuk@iki.fi>]

On Fri, 2016-09-09 at 20:10 +0200, Pau Espin Pedrol wrote:
> Hi,
> 
> I think I didn't express myself correctly. Please note I did the
> longer investigations quite a while ago and I'm mainly talking from
> memory, so I may be wrong in some of the assumptions.
> 
> From systemd.bbclass, you can see this:
>             if service.find('.socket') != -1:
>                 # for *.socket add *.service and *@.service
>                 service_base = service.replace('.socket', '')
>                 systemd_add_files_and_parse(pkg_systemd, path,
> service_base + '.service', keys)
>                 systemd_add_files_and_parse(pkg_systemd, path,
> service_base + '@.service', keys)
> 
> 
> So, for installation purposes, no .service is required in
> SYSTEMD_USER_SERVICE. Setting it .socket should also install (but not
> enable) the .service file together with the .socket one into the
> image, as actually the .socket one usually depends on the .service
> file at runtime.
> 
> That being said, of course it's not the same behavior if you add only
> one of them, the other or both:
> 1- Adding only .service -> It will install + enable the .service
> 2- Adding only the .socket -> It will install both. It will enable .socket
> 3- Adding .service + .socket -> It will install both. It will enable both.
> 
> Usually, you want either only the .service to be enabled (1, which
> will start it automatically at startup of user session), or only the
> .socket enabled (2, which will start only the socket at startup of
> user session, and only start pulseaudio process when required by some
> pulseaudio client). The third case, that is, installing + enabling
> both is usually not a good idea, at least it makes no sense to me.
> 
> So, we should consider only "1" or "2". I would personally go for 2nd
> option, and probably disable pulseaudio own autospawn system as
> explained by Tanu. This way we don't start pulseaudio unless when it's
> actually needed.
> 
> That is:
> +SYSTEMD_PACKAGES = "${PN}-server"
> +SYSTEMD_USER_SERVICE_${PN}-server = "pulseaudio.socket"
> 
> And by the way, you  should be able to remove
> "${systemd_user_unitdir}/*" from FILES_${PN}-server as it should be
> handled automatically by systemd.bbclass. If that's not the case, then
> probably there's some error in systemd.bbclass you should look into.
> 
> Hope I explained myself better now. Regards!

Thanks for the explanation. Part of my confusion was that I incorrectly
assumed that systemd.bbclass couldn't guess that the pulseaudio-server
package contains systemd units (due to the -server suffix), and
therefore it seemed to me that SYSTEMD_USER_SERVICE is expected to
contain all units, but that problem is fixed by the SYSTEMD_PACKAGES
variable.

Still, I think SYSTEMD_USER_SERVICE is a misleading variable name, if
the actual purpose of that variable is only to control which units to
enable during installation. Something like SYSTEMD_USER_ENABLE_UNITS
would be better. However, this problem is older than these patches (due
to SYSTEMD_SERVICE for system units), so I don't suggest that the
patches should be changed because of this. It indeed seems like the
best course of action would be to only add the pulseaudio.socket unit
to SYSTEMD_USER_SERVICE. Patching client.conf to disable autospawning
in case systemd is enabled would be a good addition, although not
strictly necessary.

-- 
Tanu