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

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

[Peter Kjellerstedt]

> -----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

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

[Mikko.Rapeli]

On Wed, Oct 16, 2019 at 12:45:20PM +0000, Peter Kjellerstedt wrote:
> > -----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.)

Thanks, I'll send a v2.

-Mikko

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

[Mikko Rapeli]

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
-- 
1.9.1