Re: [PATCH] systemd.bbclass: enable all services specified in ${SYSTEMD_SERVICE}

From: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
To: Mikko Rapeli <mikko.rapeli@bmw.de>,
	"openembedded-core@lists.openembedded.org"
	<openembedded-core@lists.openembedded.org>
Subject: Re: [PATCH] systemd.bbclass: enable all services specified in ${SYSTEMD_SERVICE}
Date: Wed, 16 Oct 2019 12:45:20 +0000	[thread overview]
Message-ID: <d98547503ca345b68753531ef83704b2@XBOX03.axis.com> (raw)

> -----Original Message-----
> From: openembedded-core-bounces@lists.openembedded.org <openembedded-
> core-bounces@lists.openembedded.org> On Behalf Of Mikko Rapeli
> Sent: den 16 oktober 2019 14:32
> To: openembedded-core@lists.openembedded.org
> Subject: [OE-core] [PATCH] systemd.bbclass: enable all services
> specified in ${SYSTEMD_SERVICE}
> 
> This has been the traditional way of enabling systemd services.
> It may conflict with presets feature, but other layers, image classes
> and recipes add services to be enabled using SYSTEMD_SERVICE
> variable also with read-only rootfs, e.g. IMAGE_FEATURES has
> stateless-rootfs and systemd_preset_all task is not executed.
> 
> Fixes startup of custom services from our recipes using custom
> image classes with various BSP layers. In the worst case even
> serial console getty service wasn't starting due to dependency
> no not enabled services.
> 
> Signed-off-by: Mikko Rapeli <mikko.rapeli@bmw.de>
> ---
>  meta/classes/systemd.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/classes/systemd.bbclass
> b/meta/classes/systemd.bbclass
> index 1dca099..ae03c6f 100644
> --- a/meta/classes/systemd.bbclass
> +++ b/meta/classes/systemd.bbclass
> @@ -33,7 +33,7 @@ if type systemctl >/dev/null 2>/dev/null; then
>  	if [ "${SYSTEMD_AUTO_ENABLE}" = "enable" ]; then
>  		for service in ${SYSTEMD_SERVICE_ESCAPED}; do
>  			case "${service}" in
> -			*@*)
> +			*)
>  				systemctl ${OPTS} enable "${service}"
>  				;;
>  			esac

Not much point in leaving the case statement if it only has a 
capture all case. I.e., the above simplifies to:

	if [ "${SYSTEMD_AUTO_ENABLE}" = "enable" ]; then
		for service in ${SYSTEMD_SERVICE_ESCAPED}; do
			systemctl ${OPTS} enable "$service"

(also note the change of "${service}" to "$service" to avoid using 
${...} for shell variables where not necessary as this causes them 
to unnecessarily end up in the bitbake hash for the function.)

//Peter