selftest/imagefeatures/overlayfs: Always append to DISTRO_FEATURES

Message ID 20220516112849.448882-1-richard.purdie@linuxfoundation.org
State Accepted, archived
Commit 124b82c32c4545bb216a8249954817f692f9795a
Headers show
Series selftest/imagefeatures/overlayfs: Always append to DISTRO_FEATURES | expand

Commit Message

Richard Purdie May 16, 2022, 11:28 a.m. UTC
Using += unintentionally removes all over entries from DISTRO_FEATURES and
this reduces sstate reusage on the autobuilder. Fix this to speed up builds.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/lib/oeqa/selftest/cases/imagefeatures.py |  2 +-
 meta/lib/oeqa/selftest/cases/overlayfs.py     | 16 ++++++++--------
 2 files changed, 9 insertions(+), 9 deletions(-)

Comments

Jacob Kroon May 16, 2022, 11:43 a.m. UTC | #1
Hi Richard,

On 5/16/22 13:28, Richard Purdie wrote:
> Using += unintentionally removes all over entries from DISTRO_FEATURES and

"over", you mean "other" ? If so, how is that possible ?

> this reduces sstate reusage on the autobuilder. Fix this to speed up builds.

/Jacob
Richard Purdie May 16, 2022, 12:38 p.m. UTC | #2
On Mon, 2022-05-16 at 13:43 +0200, Jacob Kroon wrote:
> Hi Richard,
> 
> On 5/16/22 13:28, Richard Purdie wrote:
> > Using += unintentionally removes all over entries from DISTRO_FEATURES and
> 
> "over", you mean "other" ? If so, how is that possible ?

I mean other, I'll fix the typo. It happens due to:

meta-poky/conf/distro/poky.conf:DISTRO_FEATURES ?= "${DISTRO_FEATURES_DEFAULT} ${POKY_DEFAULT_DISTRO_FEATURES}"

which interacts badly with "+=" :(

I'm not happy about the situation with variable interactions but still
not entirely sure the best way to proceed.

Cheers,

Richard
Jacob Kroon May 16, 2022, 12:52 p.m. UTC | #3
On 5/16/22 14:38, richard.purdie@linuxfoundation.org wrote:
> On Mon, 2022-05-16 at 13:43 +0200, Jacob Kroon wrote:
>> Hi Richard,
>>
>> On 5/16/22 13:28, Richard Purdie wrote:
>>> Using += unintentionally removes all over entries from DISTRO_FEATURES and
>>
>> "over", you mean "other" ? If so, how is that possible ?
> 
> I mean other, I'll fix the typo. It happens due to:
> 
> meta-poky/conf/distro/poky.conf:DISTRO_FEATURES ?= "${DISTRO_FEATURES_DEFAULT} ${POKY_DEFAULT_DISTRO_FEATURES}"
> 
> which interacts badly with "+=" :(
> 
> I'm not happy about the situation with variable interactions but still
> not entirely sure the best way to proceed.
> 

That is surprising to me. I tried this snippet in a Makefile:

foo ?= bar1 
 
 
 

foo += bar2 
 
 
 

$(info $(foo))

which produces:

bar1 bar2

which is what I'd expect (maybe because I'm more used to make than bitbake).

Aligning bitbake's and make's handling of variables would make a lot of 
sense to me...

Jacob
Richard Purdie May 16, 2022, 12:54 p.m. UTC | #4
On Mon, 2022-05-16 at 14:52 +0200, Jacob Kroon wrote:
> On 5/16/22 14:38, richard.purdie@linuxfoundation.org wrote:
> > On Mon, 2022-05-16 at 13:43 +0200, Jacob Kroon wrote:
> > > Hi Richard,
> > > 
> > > On 5/16/22 13:28, Richard Purdie wrote:
> > > > Using += unintentionally removes all over entries from DISTRO_FEATURES and
> > > 
> > > "over", you mean "other" ? If so, how is that possible ?
> > 
> > I mean other, I'll fix the typo. It happens due to:
> > 
> > meta-poky/conf/distro/poky.conf:DISTRO_FEATURES ?= "${DISTRO_FEATURES_DEFAULT} ${POKY_DEFAULT_DISTRO_FEATURES}"
> > 
> > which interacts badly with "+=" :(
> > 
> > I'm not happy about the situation with variable interactions but still
> > not entirely sure the best way to proceed.
> > 
> 
> That is surprising to me. I tried this snippet in a Makefile:
> 
> foo ?= bar1  
> foo += bar2 
> 
> $(info $(foo))
> 
> which produces:
> 
> bar1 bar2
> 
> which is what I'd expect (maybe because I'm more used to make than bitbake).
> 
> Aligning bitbake's and make's handling of variables would make a lot of 
> sense to me...

The amount of silent breakage that would cause though... :(

Cheers,

Richard
Quentin Schulz May 16, 2022, 1:01 p.m. UTC | #5
Hi Jacob,

On 5/16/22 14:52, Jacob Kroon wrote:
> On 5/16/22 14:38, richard.purdie@linuxfoundation.org wrote:
>> On Mon, 2022-05-16 at 13:43 +0200, Jacob Kroon wrote:
>>> Hi Richard,
>>>
>>> On 5/16/22 13:28, Richard Purdie wrote:
>>>> Using += unintentionally removes all over entries from 
>>>> DISTRO_FEATURES and
>>>
>>> "over", you mean "other" ? If so, how is that possible ?
>>
>> I mean other, I'll fix the typo. It happens due to:
>>
>> meta-poky/conf/distro/poky.conf:DISTRO_FEATURES ?= 
>> "${DISTRO_FEATURES_DEFAULT} ${POKY_DEFAULT_DISTRO_FEATURES}"
>>
>> which interacts badly with "+=" :(
>>
>> I'm not happy about the situation with variable interactions but still
>> not entirely sure the best way to proceed.
>>
> 
> That is surprising to me. I tried this snippet in a Makefile:
> 
> foo ?= bar1
> 
> 
> 
> foo += bar2
> 

Same for Bitbake.

However:
foo += bar2

foo ?= bar1

will return " bar2", while make returns "bar2" (note the missing leading 
space).

The issue is where the ?= is located. If it is in a recipe, 
configuration files need to use :append otherwise they will override ?= 
(as configuration files are parsed before recipes).

Cheers,
Quentin
Jacob Kroon May 16, 2022, 1:18 p.m. UTC | #6
On 5/16/22 15:01, Quentin Schulz wrote:
> Hi Jacob,
> 
> On 5/16/22 14:52, Jacob Kroon wrote:
>> On 5/16/22 14:38, richard.purdie@linuxfoundation.org wrote:
>>> On Mon, 2022-05-16 at 13:43 +0200, Jacob Kroon wrote:
>>>> Hi Richard,
>>>>
>>>> On 5/16/22 13:28, Richard Purdie wrote:
>>>>> Using += unintentionally removes all over entries from 
>>>>> DISTRO_FEATURES and
>>>>
>>>> "over", you mean "other" ? If so, how is that possible ?
>>>
>>> I mean other, I'll fix the typo. It happens due to:
>>>
>>> meta-poky/conf/distro/poky.conf:DISTRO_FEATURES ?= 
>>> "${DISTRO_FEATURES_DEFAULT} ${POKY_DEFAULT_DISTRO_FEATURES}"
>>>
>>> which interacts badly with "+=" :(
>>>
>>> I'm not happy about the situation with variable interactions but still
>>> not entirely sure the best way to proceed.
>>>
>>
>> That is surprising to me. I tried this snippet in a Makefile:
>>
>> foo ?= bar1
>>
>>
>>
>> foo += bar2
>>
> 
> Same for Bitbake.
> 
> However:
> foo += bar2
> 
> foo ?= bar1
> 
> will return " bar2", while make returns "bar2" (note the missing leading 
> space).
> 
> The issue is where the ?= is located. If it is in a recipe, 
> configuration files need to use :append otherwise they will override ?= 
> (as configuration files are parsed before recipes).
> 

Right, I see. So the "+="-operator is behaving as expected, but in this 
case the order of the statements forces usage of the append-override :-/

Jacob

Patch

diff --git a/meta/lib/oeqa/selftest/cases/imagefeatures.py b/meta/lib/oeqa/selftest/cases/imagefeatures.py
index 6b94ace4eba0..6d010b3e3a7e 100644
--- a/meta/lib/oeqa/selftest/cases/imagefeatures.py
+++ b/meta/lib/oeqa/selftest/cases/imagefeatures.py
@@ -235,7 +235,7 @@  USERADD_GID_TABLES += "files/static-group"
 DISTRO_FEATURES:append = " pam opengl wayland"
 
 # Switch to systemd
-DISTRO_FEATURES += "systemd"
+DISTRO_FEATURES:append = " systemd"
 VIRTUAL-RUNTIME_init_manager = "systemd"
 VIRTUAL-RUNTIME_initscripts = ""
 VIRTUAL-RUNTIME_syslog = ""
diff --git a/meta/lib/oeqa/selftest/cases/overlayfs.py b/meta/lib/oeqa/selftest/cases/overlayfs.py
index ce1d2f1ec37c..96beb8b869bc 100644
--- a/meta/lib/oeqa/selftest/cases/overlayfs.py
+++ b/meta/lib/oeqa/selftest/cases/overlayfs.py
@@ -55,7 +55,7 @@  inherit overlayfs
 
         config = """
 IMAGE_INSTALL:append = " overlayfs-user"
-DISTRO_FEATURES += "systemd overlayfs"
+DISTRO_FEATURES:append = " systemd overlayfs"
 """
 
         self.write_config(config)
@@ -94,7 +94,7 @@  OVERLAYFS_QA_SKIP[mnt-overlay] = "mount-configured"
 
         config = """
 IMAGE_INSTALL:append = " overlayfs-user"
-DISTRO_FEATURES += "systemd overlayfs"
+DISTRO_FEATURES:append = " systemd overlayfs"
 """
 
         self.write_config(config)
@@ -112,7 +112,7 @@  DISTRO_FEATURES += "systemd overlayfs"
 
         config = """
 IMAGE_INSTALL:append = " overlayfs-user"
-DISTRO_FEATURES += "systemd overlayfs"
+DISTRO_FEATURES:append = " systemd overlayfs"
 """
 
         wrong_machine_config = """
@@ -136,7 +136,7 @@  OVERLAYFS_MOUNT_POINT[usr-share-overlay] = "/usr/share/overlay"
 
         config = """
 IMAGE_INSTALL:append = " overlayfs-user systemd-machine-units"
-DISTRO_FEATURES += "systemd overlayfs"
+DISTRO_FEATURES:append = " systemd overlayfs"
 
 # Use systemd as init manager
 VIRTUAL-RUNTIME_init_manager = "systemd"
@@ -271,7 +271,7 @@  class OverlayFSEtcRunTimeTests(OESelftestTestCase):
         """
 
         configBase = """
-DISTRO_FEATURES += "systemd"
+DISTRO_FEATURES:append = " systemd"
 
 # Use systemd as init manager
 VIRTUAL-RUNTIME_init_manager = "systemd"
@@ -313,7 +313,7 @@  OVERLAYFS_ETC_DEVICE = "/dev/mmcblk0p1"
         """
 
         config = """
-DISTRO_FEATURES += "systemd"
+DISTRO_FEATURES:append = " systemd"
 
 # Use systemd as init manager
 VIRTUAL-RUNTIME_init_manager = "systemd"
@@ -349,7 +349,7 @@  INHERIT += "overlayfs-etc"
         """
 
         config = f"""
-DISTRO_FEATURES += "systemd"
+DISTRO_FEATURES:append = " systemd"
 
 # Use systemd as init manager
 VIRTUAL-RUNTIME_init_manager = "systemd"
@@ -391,7 +391,7 @@  OVERLAYFS_ETC_DEVICE = "/dev/sda3"
         """
 
         config = """
-DISTRO_FEATURES += "systemd"
+DISTRO_FEATURES:append = " systemd"
 
 # Use systemd as init manager
 VIRTUAL-RUNTIME_init_manager = "systemd"