Re: [OE-core] [PATCH] archiver.bbclass: Fix archiver interaction with kernel recipes

[Robert Yang <liezhi.yang@windriver.com>]

Hi Phil,

On 10/11/24 19:57, Richard Purdie wrote:
> On Fri, 2024-10-11 at 07:40 +0800, Phil Reid wrote:
>> On 10/10/2024 15:17, Richard Purdie wrote:
>>> On Thu, 2024-10-10 at 13:25 +0800, Phil Reid via lists.openembedded.org
>>> wrote:
>>>> Changes to the logic of is_work_shared where made in
>>>> commit: 5fbb4ca8da4f4f1ea426275c45634802dcb5a575
>>>> "archiver.bbclass: Improve work-shared checking"
>>>>
>>>> The resuled in a change of the logic (simplifed here) from:
>>>>     inherits(gcc-source) or inherits(kernel) or (inherits(kernelsrc)
>>>> and srcin(work-shared))
>>>> to just
>>>>     srcin(work-shared)
>>>>
>>>> With INHERIT += "archiver" in the local.conf and a kernel recipe that
>>>> uses
>>>> KERNEL_PACKAGE_NAME. When KERNEL_PACKAGE_NAME is defined the kernel
>>>> source is not placed into work-shared, but the archiver ends up
>>>> deleting the source folder in the work dir, and the build
>>>> subsequently fails.
>>>>
>>>> Restore the previous logic while also mainting the referenced commits
>>>> intent
>>>> to consider all recipes that use work-shared. Logic is now
>>>>     inherits(gcc-source) or inherits(kernel) or srcin(work-shared)
>>>>
>>>> Signed-off-by: Phil Reid <preid@electromag.com.au>
>>>> ---
>>>>    meta/classes/archiver.bbclass | 5 ++++-
>>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> You're effectively reverting that commit whilst changing the logic so
>>> it looks slightly different.
>>>
>>> Was there a problem with gcc-source archiving as I notice the patch
>>> adds gcc-source back too?
>> To be honest I didn't test without the gcc-source, I reverted the commit first after
>> finding that commit to be the cause and modified it so it consider it always considered
>> work-shared path and called it done and the old logic conditions.
>>
>>>
>>> The commit message isn't quite accurate as gcc-source isn't something
>>> which gets inherited.
>> Fair enough.
>>
>>>
>>> Does this only happen with kernel recipes which use
>>> KERNEL_PACKAGE_NAME? That might be the key missing detail which would
>>> allow us to reproduce the failure.
>>>
>>
>> I haven't seen the problem on a virtual/kernel.
>> When KERNEL_PACKAGE_NAME is specified the kernel source isn't put into work-shared.
>>
>>
>> How would you like to see this fixed?
> 
> I deal with many patches a day and part of the review process is to see
> if a change makes sense. So far, this patch does not add up properly.
> What I need is a change which makes sense and is something I can
> review, agree with and merge. What I don't have is time to deep dive
> into every issue and work out the proper fix, much as I would love to.
> So far on this issue, the information I do have isn't adding up
> correctly.
> 
> Taking what you say above, "KERNEL_PACKAGE_NAME is specified the kernel
> source isn't put into work-shared" - that makes sense.
> 
> If the kernel isn't being put into shared workdir, "is_work_shared()"
> should be returning False in this case. Looking at the logic, I believe
> that is true and it will for a KERNEL_PACKAGE_NAME recipe.
> 
> So what you're probably saying is that the code should be using the
> "shared" codepath for the KERNEL_PACKAGE_NAME even though the workdir
> isn't shared. At this point I'm just having to speculate though.
> 
> So how would I like to see this fixed? I'd like to see an explaintion
> of the problem which makes sense and I'd like to see changes which
> don't make the code "worse", i.e. making a "is_work_shared()" function
> return True when it isn't.
> 
> It sounds like there is a deeper problem with the archiver code which
> needs fixing at the root of the issue. Again, I'm back to speculating
> though.

Can you provide a simple reproducer, please? I'd like to look into the issue if
I reproduce it.

// Robert

> 
> Cheers,
> 
> Richard
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
> 
>