Re: [OE-core] [master][PATCH 4/5] package / packagedata bbclass: Change the way PRAUTO and AUTOINC are handled

["Mark Hatle" <mark.hatle@kernel.crashing.org>]

On 8/25/20 9:14 AM, Mark Hatle wrote:
> 
> 
> On 8/25/20 5:56 AM, Richard Purdie wrote:
>> On Mon, 2020-08-24 at 18:29 -0500, Mark Hatle wrote:
>>> A related change was also needed for the kernel.bbclass.  The kernel
>>>
>>> needs to get the AUTOINC values used by the various PV/PKGV, but with
>>> this new system we don't want to call auto_pr each time -- instead
>>> call the read_subpackage_metadata to ensure a consistent result.
>>>
>>> This also fixes an issue in the old version where the kernel
>>> sometimes
>>> caused the PR to be incremented twice, as the auto_pr was called
>>> multiple times from different tasks.
>>
>> [...]
>>
>>>
>>> diff --git a/meta/classes/kernel.bbclass
>>> b/meta/classes/kernel.bbclass
>>> index e2ceb6a333..4449452065 100644
>>> --- a/meta/classes/kernel.bbclass
>>> +++ b/meta/classes/kernel.bbclass
>>> @@ -416,7 +416,7 @@ kernel_do_install() {
>>>  	install -d ${D}${sysconfdir}/modules-load.d
>>>  	install -d ${D}${sysconfdir}/modprobe.d
>>>  }
>>> -do_install[prefuncs] += "package_get_auto_pr"
>>> +do_install[prefuncs] += "read_subpackage_metadata"

I talked with Bruce on this.  We see no reason do_install[prefuncs] is required
any longer.

It was when that line was written, but when the kernel was refactored to permit
multiple kernel builds and such the need for this went away.

>>>  # Must be ran no earlier than after do_kernel_checkout or else
>>> Makefile won't be in ${S}/Makefile
>>>  do_kernel_version_sanity_check() {
>>> @@ -749,7 +749,7 @@ kernel_do_deploy() {
>>>  		done
>>>  	fi
>>>  }
>>> -do_deploy[prefuncs] += "package_get_auto_pr"
>>> +do_deploy[prefuncs] += "read_subpackage_metadata"
>>>  
>>>  addtask deploy after do_populate_sysroot do_packagedata
>>
>> I don't think this can be right. I understand why the kernel was
>> expanding these early but at this point do_package hasn't happened so
>> reading the subpackage metadata is a null op at best and potentially
>> changes a lot more in the data store.
>>
>> I suspect this is not doing the right thing and will be something that
>> comes back to bite us so we need something more careful here and I'd
>> like to better understand exactly what change to the variables
>> do_install and do_deploy need.
> 
> I don't know exactly why it's doing this.  But I believe it is so that the
> PV/PKGV is expanded (AUTOINC translated).  I don't see any other reason for it
> to be expanded in either do_install or do_deploy.
> 
> Best I can find is:
> 
> kernel-artifact-names.bbclass:KERNEL_ARTIFACT_NAME ?=
> "${PKGE}-${PKGV}-${PKGR}-${MACHINE}${IMAGE_VERSION_SUFFIX}"
> 
> (That is then used in other places.)
> 
> So the PKGV replacement is desired to be that early.
> 
> I'll look into it further.

In do_deploy, I see multiple usages of the KERNEL_ARTIFACT_NAME (sometimes with
alternative variables.. but in the end both PKGV and PKGR are used here.)

do_deploy is after do_package/do_packagedata (where the values are assigned and
stored).  So it should be safe for us to use the read_subpackage_metadata (or
something new if desired) to read back in the stored PKGV (AUTOINC) and PKGR
(PRAUTO) values.

--Mark

> --Mark
> 
>> Cheers,
>>
>> Richard
>>
>>
>>