Re: [OE-Core][PATCH v3 1/2] testimage: move TESTIMAGE_FAILED_QA_ARTIFACTS default to core-image

["Alexis Lothoré" <alexis.lothore@bootlin.com>]

On 2/6/24 13:00, Richard Purdie wrote:
> On Tue, 2024-02-06 at 12:09 +0100, Alexis Lothoré wrote:
>> So I've did multiple attempts to make this work, and I see no way to define the
>> default value in testimage.bbclass with "=", and add content to it in
>> core-image-ptest.bbwith "+=". My understanding is that testimage is always
>> parsed after core-image-ptest.bb, so there is no nice way to set its default
>> value in there:
>> - "=" will always overwrite ptests artifacts set with "+="
>> - "?=" or "??=" will always be overwritten by ptests artifacts set with "+="
>> My configuration relies on IMAGE_CLASSES += 'testimage', which is also done in CI.
> 
> The issue is this:
> 
> classes-recipe/image.bbclass:inherit_defer ${IMGCLASSES}
>
> since this is deferred to after the recipe is parsed.

Ah, thank you for the explanation, I was not aware of this defer mechanism.

>> Still, I understand your concern about keeping default value in
>> testimage.bbclass. The only compromise I see here to both keep the default value
>> in testimage and be able to override it at user level is not to enrich it in
>> core-image-ptest but to redefine it:
>>
>> diff --git a/meta/recipes-core/images/core-image-ptest.bb
>> b/meta/recipes-core/images/core-image-ptest.bb
>> index fb96c542c0a3..4a90181afd83 100644
>> --- a/meta/recipes-core/images/core-image-ptest.bb
>> +++ b/meta/recipes-core/images/core-image-ptest.bb
>> @@ -43,5 +43,8 @@ python () {
>>          raise bb.parse.SkipRecipe("No class extension set")
>>  }
>>
>> -# Include ptest directory in artifacts to retrieve if there is a failed test
>> -TESTIMAGE_FAILED_QA_ARTIFACTS += "${libdir}/${MCNAME}/ptest"
>> +TESTIMAGE_FAILED_QA_ARTIFACTS = "\
>> +    ${localstatedir}/log \
>> +    ${sysconfdir}/version \
>> +    ${sysconfdir}/os-release \
>> +    ${libdir}/${MCNAME}/ptest"
>>
>> That's for example what is done with TEST_SUITES. In this setup, we can keep a
>> default value in testimage, and override it as needed with append/prepend/remove
>> syntax at user level.
>>
>> I'll wait a bit in case there's any comment or concern about this approach, and
>> then send a new version.
> 
> I see two options:
> 
> a) Make testimage.bbclass a +=
> 
> or
> 
> b) Put the new entry into testimage.bbclass even if it is ptest
> specific.
> 
> 
> I'm leaning to doing both, make it a += and just put it there by
> default in case someone includes ptests in a non ptest image.

Ok, sounds good to me too and ptests artifact retrieval seems to behave properly
even when defined in testimage. I'll submit a new version which keeps the
default artifact list in testimage, and adds immediately ptests artifacts to it.

Thanks for the guidance :)

Alexis

-- 
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com