[PATCH V2 0/1] image.bbclass: fix systemd_preset_all

[PATCH V2 0/1] image.bbclass: fix systemd_preset_all

[Chen Qi]

Changes in V2:
* Check /lib/systemd/systemd under ${IMAGE_ROOTFS} instead of checking
  the existence of systemctl tool in native staging directory.

The following changes since commit bc5f6725af04417dcf5c65981d8b85abee9c61d8:

  Revert "pigz: Add debug for autobuilder errors" (2019-06-30 23:33:45 +0100)

are available in the git repository at:

  git://git.pokylinux.org/poky-contrib ChenQi/image-systemd
  http://git.pokylinux.org/cgit.cgi/poky-contrib/log/?h=ChenQi/image-systemd

Chen Qi (1):
  image.bbclass: fix systemd_preset_all

 meta/classes/image.bbclass | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

-- 
1.9.1

[PATCH V2 1/1] image.bbclass: fix systemd_preset_all

[Chen Qi]

Check the existence of systemd before using systemctl to preset units.
This is because even if 'systemd' is in DISTRO_FEATURES, it's possible
that systemd is not even installed. e.g. container-test-image in
meta-selftest layer.

As systemd DEPENDS on systemd-systemctl-native, the existence of systemd
also ensures the existence of systemd-systemctl-native.

This would fix the following test case when using systemd as the init
manager.

  containerimage.ContainerImageTests.test_expected_files

Also remove the IMAGE_EXTRADEPENDS setting, as nothing references this
variable.

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/classes/image.bbclass | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index d2b2fb9..7daa97e 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -666,10 +666,11 @@ reproducible_final_image_task () {
 }
 
 systemd_preset_all () {
-    systemctl --root="${IMAGE_ROOTFS}" --preset-mode=enable-only preset-all
+    if [ -e ${IMAGE_ROOTFS}${root_prefix}/lib/systemd/systemd ]; then
+	systemctl --root="${IMAGE_ROOTFS}" --preset-mode=enable-only preset-all
+    fi
 }
 
-IMAGE_EXTRADEPENDS += "${@ 'systemd-systemctl-native' if bb.utils.contains('DISTRO_FEATURES', 'systemd', True, False, d) and not bb.utils.contains('IMAGE_FEATURES', 'stateless-rootfs', True, False, d) else ''}"
 IMAGE_PREPROCESS_COMMAND_append = " ${@ 'systemd_preset_all;' if bb.utils.contains('DISTRO_FEATURES', 'systemd', True, False, d) and not bb.utils.contains('IMAGE_FEATURES', 'stateless-rootfs', True, False, d) else ''} reproducible_final_image_task; "
 
 CVE_PRODUCT = ""
-- 
1.9.1

Re: [PATCH V2 1/1] image.bbclass: fix systemd_preset_all

[Peter Kjellerstedt]

> -----Original Message-----
> From: openembedded-core-bounces@lists.openembedded.org <openembedded-
> core-bounces@lists.openembedded.org> On Behalf Of Chen Qi
> Sent: den 1 juli 2019 06:16
> To: openembedded-core@lists.openembedded.org
> Subject: [OE-core] [PATCH V2 1/1] image.bbclass: fix systemd_preset_all
> 
> Check the existence of systemd before using systemctl to preset units.
> This is because even if 'systemd' is in DISTRO_FEATURES, it's possible
> that systemd is not even installed. e.g. container-test-image in
> meta-selftest layer.
> 
> As systemd DEPENDS on systemd-systemctl-native, the existence of systemd
> also ensures the existence of systemd-systemctl-native.
> 
> This would fix the following test case when using systemd as the init
> manager.
> 
>   containerimage.ContainerImageTests.test_expected_files
> 
> Also remove the IMAGE_EXTRADEPENDS setting, as nothing references this
> variable.
> 
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>  meta/classes/image.bbclass | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> index d2b2fb9..7daa97e 100644
> --- a/meta/classes/image.bbclass
> +++ b/meta/classes/image.bbclass
> @@ -666,10 +666,11 @@ reproducible_final_image_task () {
>  }
> 
>  systemd_preset_all () {
> -    systemctl --root="${IMAGE_ROOTFS}" --preset-mode=enable-only preset-all
> +    if [ -e ${IMAGE_ROOTFS}${root_prefix}/lib/systemd/systemd ]; then
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
That should be ${systemd_system_unitdir}, which will also use the correct path 
(it is /lib/systemd/system, not /lib/systemd/systemd).

> +	systemctl --root="${IMAGE_ROOTFS}" --preset-mode=enable-only preset-all
> +    fi
>  }
> 
> -IMAGE_EXTRADEPENDS += "${@ 'systemd-systemctl-native' if bb.utils.contains('DISTRO_FEATURES', 'systemd', True, False, d) and not bb.utils.contains('IMAGE_FEATURES', 'stateless-rootfs', True, False, d) else ''}"
>  IMAGE_PREPROCESS_COMMAND_append = " ${@ 'systemd_preset_all;' if bb.utils.contains('DISTRO_FEATURES', 'systemd', True, False, d) and not bb.utils.contains('IMAGE_FEATURES', 'stateless-rootfs', True, False, d) else ''} reproducible_final_image_task; "
> 
>  CVE_PRODUCT = ""
> --
> 1.9.1

//Peter

Re: [PATCH V2 1/1] image.bbclass: fix systemd_preset_all

[ChenQi]

On 07/02/2019 07:34 AM, Peter Kjellerstedt wrote:
>> -----Original Message-----
>> From: openembedded-core-bounces@lists.openembedded.org <openembedded-
>> core-bounces@lists.openembedded.org> On Behalf Of Chen Qi
>> Sent: den 1 juli 2019 06:16
>> To: openembedded-core@lists.openembedded.org
>> Subject: [OE-core] [PATCH V2 1/1] image.bbclass: fix systemd_preset_all
>>
>> Check the existence of systemd before using systemctl to preset units.
>> This is because even if 'systemd' is in DISTRO_FEATURES, it's possible
>> that systemd is not even installed. e.g. container-test-image in
>> meta-selftest layer.
>>
>> As systemd DEPENDS on systemd-systemctl-native, the existence of systemd
>> also ensures the existence of systemd-systemctl-native.
>>
>> This would fix the following test case when using systemd as the init
>> manager.
>>
>>    containerimage.ContainerImageTests.test_expected_files
>>
>> Also remove the IMAGE_EXTRADEPENDS setting, as nothing references this
>> variable.
>>
>> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>> ---
>>   meta/classes/image.bbclass | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
>> index d2b2fb9..7daa97e 100644
>> --- a/meta/classes/image.bbclass
>> +++ b/meta/classes/image.bbclass
>> @@ -666,10 +666,11 @@ reproducible_final_image_task () {
>>   }
>>
>>   systemd_preset_all () {
>> -    systemctl --root="${IMAGE_ROOTFS}" --preset-mode=enable-only preset-all
>> +    if [ -e ${IMAGE_ROOTFS}${root_prefix}/lib/systemd/systemd ]; then
>                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> That should be ${systemd_system_unitdir}, which will also use the correct path
> (it is /lib/systemd/system, not /lib/systemd/systemd).

I'm checking the systemd binary under ${root_prefix}/lib/systemd, not 
the directory holding units.

Regards,
Chen Qi

>> +	systemctl --root="${IMAGE_ROOTFS}" --preset-mode=enable-only preset-all
>> +    fi
>>   }
>>
>> -IMAGE_EXTRADEPENDS += "${@ 'systemd-systemctl-native' if bb.utils.contains('DISTRO_FEATURES', 'systemd', True, False, d) and not bb.utils.contains('IMAGE_FEATURES', 'stateless-rootfs', True, False, d) else ''}"
>>   IMAGE_PREPROCESS_COMMAND_append = " ${@ 'systemd_preset_all;' if bb.utils.contains('DISTRO_FEATURES', 'systemd', True, False, d) and not bb.utils.contains('IMAGE_FEATURES', 'stateless-rootfs', True, False, d) else ''} reproducible_final_image_task; "
>>
>>   CVE_PRODUCT = ""
>> --
>> 1.9.1
> //Peter
>
>

Re: [PATCH V2 1/1] image.bbclass: fix systemd_preset_all

[Peter Kjellerstedt]

> -----Original Message-----
> From: openembedded-core-bounces@lists.openembedded.org <openembedded-
> core-bounces@lists.openembedded.org> On Behalf Of ChenQi
> Sent: den 2 juli 2019 03:39
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
> core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH V2 1/1] image.bbclass: fix
> systemd_preset_all
> 
> On 07/02/2019 07:34 AM, Peter Kjellerstedt wrote:
> >> -----Original Message-----
> >> From: openembedded-core-bounces@lists.openembedded.org
> <openembedded-
> >> core-bounces@lists.openembedded.org> On Behalf Of Chen Qi
> >> Sent: den 1 juli 2019 06:16
> >> To: openembedded-core@lists.openembedded.org
> >> Subject: [OE-core] [PATCH V2 1/1] image.bbclass: fix
> systemd_preset_all
> >>
> >> Check the existence of systemd before using systemctl to preset
> units.
> >> This is because even if 'systemd' is in DISTRO_FEATURES, it's
> possible
> >> that systemd is not even installed. e.g. container-test-image in
> >> meta-selftest layer.
> >>
> >> As systemd DEPENDS on systemd-systemctl-native, the existence of
> systemd
> >> also ensures the existence of systemd-systemctl-native.
> >>
> >> This would fix the following test case when using systemd as the
> init
> >> manager.
> >>
> >>    containerimage.ContainerImageTests.test_expected_files
> >>
> >> Also remove the IMAGE_EXTRADEPENDS setting, as nothing references
> this
> >> variable.
> >>
> >> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> >> ---
> >>   meta/classes/image.bbclass | 5 +++--
> >>   1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
> >> index d2b2fb9..7daa97e 100644
> >> --- a/meta/classes/image.bbclass
> >> +++ b/meta/classes/image.bbclass
> >> @@ -666,10 +666,11 @@ reproducible_final_image_task () {
> >>   }
> >>
> >>   systemd_preset_all () {
> >> -    systemctl --root="${IMAGE_ROOTFS}" --preset-mode=enable-only
> preset-all
> >> +    if [ -e ${IMAGE_ROOTFS}${root_prefix}/lib/systemd/systemd ];
> then
> >                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > That should be ${systemd_system_unitdir}, which will also use the
> correct path
> > (it is /lib/systemd/system, not /lib/systemd/systemd).
> 
> I'm checking the systemd binary under ${root_prefix}/lib/systemd, not
> the directory holding units.

Right, my bad. Still, then the path above should be
"${IMAGE_ROOTFS}${systemd_unitdir}/systemd".

> Regards,
> Chen Qi
> 
> >> +	systemctl --root="${IMAGE_ROOTFS}" --preset-mode=enable-only
> preset-all
> >> +    fi
> >>   }
> >>
> >> -IMAGE_EXTRADEPENDS += "${@ 'systemd-systemctl-native' if
> bb.utils.contains('DISTRO_FEATURES', 'systemd', True, False, d) and not
> bb.utils.contains('IMAGE_FEATURES', 'stateless-rootfs', True, False, d)
> else ''}"
> >>   IMAGE_PREPROCESS_COMMAND_append = " ${@ 'systemd_preset_all;' if
> bb.utils.contains('DISTRO_FEATURES', 'systemd', True, False, d) and not
> bb.utils.contains('IMAGE_FEATURES', 'stateless-rootfs', True, False, d)
> else ''} reproducible_final_image_task; "
> >>
> >>   CVE_PRODUCT = ""
> >> --
> >> 1.9.1
> > //Peter

//Peter

Re: [PATCH V2 1/1] image.bbclass: fix systemd_preset_all

[ChenQi]

On 07/03/2019 05:04 AM, Peter Kjellerstedt wrote:
>> -----Original Message-----
>> From: openembedded-core-bounces@lists.openembedded.org <openembedded-
>> core-bounces@lists.openembedded.org> On Behalf Of ChenQi
>> Sent: den 2 juli 2019 03:39
>> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
>> core@lists.openembedded.org
>> Subject: Re: [OE-core] [PATCH V2 1/1] image.bbclass: fix
>> systemd_preset_all
>>
>> On 07/02/2019 07:34 AM, Peter Kjellerstedt wrote:
>>>> -----Original Message-----
>>>> From: openembedded-core-bounces@lists.openembedded.org
>> <openembedded-
>>>> core-bounces@lists.openembedded.org> On Behalf Of Chen Qi
>>>> Sent: den 1 juli 2019 06:16
>>>> To: openembedded-core@lists.openembedded.org
>>>> Subject: [OE-core] [PATCH V2 1/1] image.bbclass: fix
>> systemd_preset_all
>>>> Check the existence of systemd before using systemctl to preset
>> units.
>>>> This is because even if 'systemd' is in DISTRO_FEATURES, it's
>> possible
>>>> that systemd is not even installed. e.g. container-test-image in
>>>> meta-selftest layer.
>>>>
>>>> As systemd DEPENDS on systemd-systemctl-native, the existence of
>> systemd
>>>> also ensures the existence of systemd-systemctl-native.
>>>>
>>>> This would fix the following test case when using systemd as the
>> init
>>>> manager.
>>>>
>>>>     containerimage.ContainerImageTests.test_expected_files
>>>>
>>>> Also remove the IMAGE_EXTRADEPENDS setting, as nothing references
>> this
>>>> variable.
>>>>
>>>> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>>>> ---
>>>>    meta/classes/image.bbclass | 5 +++--
>>>>    1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
>>>> index d2b2fb9..7daa97e 100644
>>>> --- a/meta/classes/image.bbclass
>>>> +++ b/meta/classes/image.bbclass
>>>> @@ -666,10 +666,11 @@ reproducible_final_image_task () {
>>>>    }
>>>>
>>>>    systemd_preset_all () {
>>>> -    systemctl --root="${IMAGE_ROOTFS}" --preset-mode=enable-only
>> preset-all
>>>> +    if [ -e ${IMAGE_ROOTFS}${root_prefix}/lib/systemd/systemd ];
>> then
>>>                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> That should be ${systemd_system_unitdir}, which will also use the
>> correct path
>>> (it is /lib/systemd/system, not /lib/systemd/systemd).
>> I'm checking the systemd binary under ${root_prefix}/lib/systemd, not
>> the directory holding units.
> Right, my bad. Still, then the path above should be
> "${IMAGE_ROOTFS}${systemd_unitdir}/systemd".

In current OE, ${systemd_unitdir} = ${root_prefix}/lib/systemd.
And in systemd recipe, we have:
[ ! -e ${D}/init ] && ln -s ${rootlibexecdir}/systemd/systemd ${D}/init
It does not write as `ln -s ${systemd_unitdir}/systemd/systemd'.
I don't want to use a directory whose name indicates 'unit' when 
checking a binary.

Regards,
Chen Qi

>> Regards,
>> Chen Qi
>>
>>>> +	systemctl --root="${IMAGE_ROOTFS}" --preset-mode=enable-only
>> preset-all
>>>> +    fi
>>>>    }
>>>>
>>>> -IMAGE_EXTRADEPENDS += "${@ 'systemd-systemctl-native' if
>> bb.utils.contains('DISTRO_FEATURES', 'systemd', True, False, d) and not
>> bb.utils.contains('IMAGE_FEATURES', 'stateless-rootfs', True, False, d)
>> else ''}"
>>>>    IMAGE_PREPROCESS_COMMAND_append = " ${@ 'systemd_preset_all;' if
>> bb.utils.contains('DISTRO_FEATURES', 'systemd', True, False, d) and not
>> bb.utils.contains('IMAGE_FEATURES', 'stateless-rootfs', True, False, d)
>> else ''} reproducible_final_image_task; "
>>>>    CVE_PRODUCT = ""
>>>> --
>>>> 1.9.1
>>> //Peter
> //Peter
>
>

Re: [PATCH V2 1/1] image.bbclass: fix systemd_preset_all

[Peter Kjellerstedt]

> -----Original Message-----
> From: ChenQi <Qi.Chen@windriver.com>
> Sent: den 3 juli 2019 03:49
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
> core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH V2 1/1] image.bbclass: fix
> systemd_preset_all
> 
> On 07/03/2019 05:04 AM, Peter Kjellerstedt wrote:
> >> -----Original Message-----
> >> From: openembedded-core-bounces@lists.openembedded.org
> <openembedded-
> >> core-bounces@lists.openembedded.org> On Behalf Of ChenQi
> >> Sent: den 2 juli 2019 03:39
> >> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
> >> core@lists.openembedded.org
> >> Subject: Re: [OE-core] [PATCH V2 1/1] image.bbclass: fix
> >> systemd_preset_all
> >>
> >> On 07/02/2019 07:34 AM, Peter Kjellerstedt wrote:
> >>>> -----Original Message-----
> >>>> From: openembedded-core-bounces@lists.openembedded.org
> >> <openembedded-
> >>>> core-bounces@lists.openembedded.org> On Behalf Of Chen Qi
> >>>> Sent: den 1 juli 2019 06:16
> >>>> To: openembedded-core@lists.openembedded.org
> >>>> Subject: [OE-core] [PATCH V2 1/1] image.bbclass: fix
> >> systemd_preset_all
> >>>> Check the existence of systemd before using systemctl to preset
> >> units.
> >>>> This is because even if 'systemd' is in DISTRO_FEATURES, it's
> >> possible
> >>>> that systemd is not even installed. e.g. container-test-image in
> >>>> meta-selftest layer.
> >>>>
> >>>> As systemd DEPENDS on systemd-systemctl-native, the existence of
> >> systemd
> >>>> also ensures the existence of systemd-systemctl-native.
> >>>>
> >>>> This would fix the following test case when using systemd as the
> >> init
> >>>> manager.
> >>>>
> >>>>     containerimage.ContainerImageTests.test_expected_files
> >>>>
> >>>> Also remove the IMAGE_EXTRADEPENDS setting, as nothing references
> >> this
> >>>> variable.
> >>>>
> >>>> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> >>>> ---
> >>>>    meta/classes/image.bbclass | 5 +++--
> >>>>    1 file changed, 3 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/meta/classes/image.bbclass
> b/meta/classes/image.bbclass
> >>>> index d2b2fb9..7daa97e 100644
> >>>> --- a/meta/classes/image.bbclass
> >>>> +++ b/meta/classes/image.bbclass
> >>>> @@ -666,10 +666,11 @@ reproducible_final_image_task () {
> >>>>    }
> >>>>
> >>>>    systemd_preset_all () {
> >>>> -    systemctl --root="${IMAGE_ROOTFS}" --preset-mode=enable-only
> >> preset-all
> >>>> +    if [ -e ${IMAGE_ROOTFS}${root_prefix}/lib/systemd/systemd ];
> >> then
> >>>                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >>> That should be ${systemd_system_unitdir}, which will also use the
> >> correct path
> >>> (it is /lib/systemd/system, not /lib/systemd/systemd).
> >> I'm checking the systemd binary under ${root_prefix}/lib/systemd,
> not
> >> the directory holding units.
> > Right, my bad. Still, then the path above should be
> > "${IMAGE_ROOTFS}${systemd_unitdir}/systemd".
> 
> In current OE, ${systemd_unitdir} = ${root_prefix}/lib/systemd.
> And in systemd recipe, we have:
> [ ! -e ${D}/init ] && ln -s ${rootlibexecdir}/systemd/systemd ${D}/init
> It does not write as `ln -s ${systemd_unitdir}/systemd/systemd'.
> I don't want to use a directory whose name indicates 'unit' when
> checking a binary.

Well, the name of the systemd_unitdir variable is misleading, as it 
does not in fact refer to a unit directory. The corresponding name in 
the systemd.pc file is systemdutildir, which I guess would have been 
a better name for the variable too (albeit a bit late to change it now).

Anyway, it is not very likely that the path to the systemd binary will  
change, so if you prefer to use the ${root_prefix}/lib/systemd path 
instead of ${systemd_unitdir}, then go ahead. 

> Regards,
> Chen Qi
> 
> >> Regards,
> >> Chen Qi
> >>
> >>>> +	systemctl --root="${IMAGE_ROOTFS}" --preset-mode=enable-only
> >> preset-all
> >>>> +    fi
> >>>>    }
> >>>>
> >>>> -IMAGE_EXTRADEPENDS += "${@ 'systemd-systemctl-native' if
> >> bb.utils.contains('DISTRO_FEATURES', 'systemd', True, False, d) and
> not
> >> bb.utils.contains('IMAGE_FEATURES', 'stateless-rootfs', True, False,
> d)
> >> else ''}"
> >>>>    IMAGE_PREPROCESS_COMMAND_append = " ${@ 'systemd_preset_all;'
> if
> >> bb.utils.contains('DISTRO_FEATURES', 'systemd', True, False, d) and
> not
> >> bb.utils.contains('IMAGE_FEATURES', 'stateless-rootfs', True, False,
> d)
> >> else ''} reproducible_final_image_task; "
> >>>>    CVE_PRODUCT = ""
> >>>> --
> >>>> 1.9.1
> >>> //Peter
> > //Peter

//Peter