Re: [OE-core][kirkstone 7/8] kernel: fix localversion in v6.3+

[Ryan Eatmon <reatmon@ti.com>]

On 2/22/2024 4:39 AM, Andreas Helbech Kleist wrote:
> On Wed, 2024-02-21 at 08:40 -0600, Ryan Eatmon wrote:
>>
>> On 2/16/2024 7:51 AM, Andreas Helbech Kleist wrote:
>>> On Fri, 2024-02-16 at 09:11 +0100, Andreas Helbech Kleist wrote:
>>>> On Thu, 2024-02-15 at 19:45 -0600, Ryan Eatmon via
>>>> lists.openembedded.org wrote:
>>>>>
>>>>> On 2/15/2024 7:43 PM, Steve Sakoman wrote:
>>>>>> On Thu, Feb 15, 2024 at 3:20 PM Ryan Eatmon <reatmon@ti.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>> With this patch in place we are seeing a breakage on our kernel builds.
>>>>>>> This patch was cherry picked from master, but the next patch on this
>>>>>>> file in master removes the LOCALVERSION setting...
>>>>>>>
>>>>>>> https://git.openembedded.org/openembedded-core/commit/meta/classes-recipe/kernel-arch.bbclass?h=master-next&id=b378eec156998eea55ba61e59103cb34fab0d07c
>>>>>>>
>>>>>>>
>>>>>>> There is a disconnect here.  Why did we change kirkstone, a stable LTS
>>>>>>> version with a partial patch series based on master?
>>>>>>
>>>>>> The original kirkstone backport request for this patch was sent to the
>>>>>> list for review on February 9.
>>>>>>
>>>>>> There were no comments or objections, so I added it to the patch test
>>>>>> queue.  No issues were encountered on the autobuilder, so I sent this
>>>>>> patch (along with the rest of the patch queue) to the list for a
>>>>>> second review opportunity on February 12.  Once again there were no
>>>>>> comments or objections, so it was merged today February 15.
>>>>>>
>>>>>> I follow this process because I'm not smart enough, nor do I have time
>>>>>> enough to thoroughly research all implications of every patch.  I rely
>>>>>> on autobuilder testing and community review to minimize breakage.
>>>>>>
>>>>>> This normally works quite well, but this time it didn't.  I'll revert
>>>>>> this patch and if someone would like to resubmit a proper series to
>>>>>> deal with the issue it is trying to fix I will consider it and run it
>>>>>> through the same process.
>>>>>
>>>>> I can appreciate that.  My comment was more for the person who sent the
>>>>> patch in the first place.  You are doing a great job.  I will try and
>>>>> pay attention for a replacement patch coming in and review as well.
>>>>
>>>> I'm sorry about that. I need to use a 6.6 kernel on kirkstone, and this
>>>> patch fixed my usecase. It looked like a simple bugfix, and I was not
>>>> aware that there was a later fix for this change.
>>>>
>>>> Is there any way I can reproduce the error you saw Ryan?
>>>>
>>>> And would it make sense to add an automated test for this usecase?
>>>>
>>>> I'll test my usecase with both patches backported and re-submit.
>>>
>>> I've now looked into it and can make it work in my setup by backporting
>>> these three commits from master:
>>>
>>>     kernel.bbclass: introduce KERNEL_LOCALVERSION
>>>    
>>> https://git.openembedded.org/openembedded-core/commit/?h=master-next&id=229435a52f36ddec5f85fb6d5ccd42044b688397
>>>
>>>     kernel: fix localversion in v6.3+
>>>    
>>> https://git.openembedded.org/openembedded-core/commit/?h=master-next&id=765b13b7305c8d2f222cfc66d77c02e6a088c691
>>>
>>>     kernel: make LOCALVERSION consistent between recipes
>>>    
>>> https://git.openembedded.org/openembedded-core/commit/?h=master-next&id=b378eec156998eea55ba61e59103cb34fab0d07c
>>>
>>> The cherry-pick's apply cleanly without any conflicts.
>>>
>>> Would they be appropriate for kirkstone? I'm wondering if there would
>>> be any issue with backporting the KERNEL_LOCALVERSION variable
>>> specifically.
>>>
>>> If I hear nothing before the middle of next week, I'll submit the patch
>>> series (CC'ing you Ryan).
>>>
>>
>> Can you send me the patch series that you want to want to send in
>> against kirkstone?  I'll apply them locally and run some tests on our
>> setup to make sure they don't cause an issue.  I don't foresee them
>> being a problem, but I'm more than happy to verify.
> 
> I send them to the list and CC'ed you, hope that is ok.

Yep.  I'll test them and reply to the patches.


> See
> https://lore.kernel.org/openembedded-core/20240222103545.2970387-1-andreaskleist@gmail.com/T/#t
> 
>>> /Andreas
>>>>
>>>> PS: This is my first submission to oe-core, so I'm just starting to
>>>> learn. I have no idea what or how the autobuilder tests, but I'm
>>>> willing to learn.
>>>>
>>>>>
>>>>>
>>>>>> Steve
>>>>>>
>>>>>>
>>>>>>> On 2/12/2024 7:54 AM, Steve Sakoman wrote:
>>>>>>>> From: Bruce Ashfield <bruce.ashfield@gmail.com>
>>>>>>>>
>>>>>>>> During testing of the v6.4 reference kernel, it was noticed that
>>>>>>>> on-target modules no longer matched the magic value of the running
>>>>>>>> kernel.
>>>>>>>>
>>>>>>>> This was due to a different localversion in the cross built kernel
>>>>>>>> and the scripts / resources created on target.
>>>>>>>>
>>>>>>>> This was due to changes in the setlocalversion script introduced
>>>>>>>> in the v6.3 series.
>>>>>>>>
>>>>>>>> The .scmversion file is no longer used (or packaged) to inhibit
>>>>>>>> the addition of a "+" (through querying of the git status of the
>>>>>>>> kernel) or the setting of a local version.
>>>>>>>>
>>>>>>>> We recently introduced the KERNEL_LOCALVERSION variable to allow
>>>>>>>> recipes to place a value in .scmversion, so we extend the use of
>>>>>>>> that variable to kernel-arch.bbclass and use it to set the
>>>>>>>> exported variable LOCALVERSION.
>>>>>>>>
>>>>>>>> We must do it at the kernel-arch level, as the variable must be
>>>>>>>> exported in any kernel build to ensure that setlocalversion always
>>>>>>>> correctly sets the localversion.
>>>>>>>>
>>>>>>>> Signed-off-by: Bruce Ashfield <bruce.ashfield@gmail.com>
>>>>>>>> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
>>>>>>>>
>>>>>>>> Cherry-picked from master 765b13b7305c8d2f222cfc66d77c02e6a088c691
>>>>>>>>
>>>>>>>> Signed-off-by: Andreas Helbech Kleist <andreaskleist@gmail.com>
>>>>>>>> Signed-off-by: Steve Sakoman <steve@sakoman.com>
>>>>>>>> ---
>>>>>>>>      meta/classes/kernel-arch.bbclass |  7 +++++++
>>>>>>>>      meta/classes/kernel.bbclass      | 10 ++++++++--
>>>>>>>>      2 files changed, 15 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/meta/classes/kernel-arch.bbclass b/meta/classes/kernel-arch.bbclass
>>>>>>>> index 4cd08b96fb..0a79dea0af 100644
>>>>>>>> --- a/meta/classes/kernel-arch.bbclass
>>>>>>>> +++ b/meta/classes/kernel-arch.bbclass
>>>>>>>> @@ -66,3 +66,10 @@ KERNEL_LD = "${CCACHE}${HOST_PREFIX}ld.bfd ${HOST_LD_KERNEL_ARCH}"
>>>>>>>>      KERNEL_AR = "${CCACHE}${HOST_PREFIX}ar ${HOST_AR_KERNEL_ARCH}"
>>>>>>>>      TOOLCHAIN ?= "gcc"
>>>>>>>>
>>>>>>>> +# 6.3+ requires the variable LOCALVERSION to be set to not get a "+" in
>>>>>>>> +# the local version. Having it empty means nothing will be added, and any
>>>>>>>> +# value will be appended to the local kernel version. This replaces the
>>>>>>>> +# use of .scmversion file for setting a localversion without using
>>>>>>>> +# the CONFIG_LOCALVERSION option.
>>>>>>>> +KERNEL_LOCALVERSION ??= ""
>>>>>>>> +export LOCALVERSION ?= "${KERNEL_LOCALVERSION}"
>>>>>>>> diff --git a/meta/classes/kernel.bbclass b/meta/classes/kernel.bbclass
>>>>>>>> index 5951347361..940f1a3cf4 100644
>>>>>>>> --- a/meta/classes/kernel.bbclass
>>>>>>>> +++ b/meta/classes/kernel.bbclass
>>>>>>>> @@ -418,7 +418,7 @@ do_compile_kernelmodules() {
>>>>>>>>          if (grep -q -i -e '^CONFIG_MODULES=y$' ${B}/.config); then
>>>>>>>>                  oe_runmake -C ${B} ${PARALLEL_MAKE} modules ${KERNEL_EXTRA_ARGS}
>>>>>>>>
>>>>>>>> -             # Module.symvers gets updated during the
>>>>>>>> +             # Module.symvers gets updated during the
>>>>>>>>                  # building of the kernel modules. We need to
>>>>>>>>                  # update this in the shared workdir since some
>>>>>>>>                  # external kernel modules has a dependency on
>>>>>>>> @@ -635,7 +635,13 @@ kernel_do_configure() {
>>>>>>>>          # $ scripts/setlocalversion . => +
>>>>>>>>          # $ make kernelversion => 2.6.37
>>>>>>>>          # $ make kernelrelease => 2.6.37+
>>>>>>>> -     touch ${B}/.scmversion ${S}/.scmversion
>>>>>>>> +     # See kernel-arch.bbclass for post v6.3 removal of the extra
>>>>>>>> +     # + in localversion. .scmversion is no longer used, and the
>>>>>>>> +     # variable LOCALVERSION must be used
>>>>>>>> +     if [ ! -e ${B}/.scmversion -a ! -e ${S}/.scmversion ]; then
>>>>>>>> +             echo ${KERNEL_LOCALVERSION} > ${B}/.scmversion
>>>>>>>> +             echo ${KERNEL_LOCALVERSION} > ${S}/.scmversion
>>>>>>>> +     fi
>>>>>>>>
>>>>>>>>          if [ "${S}" != "${B}" ] && [ -f "${S}/.config" ] && [ ! -f "${B}/.config" ]; then
>>>>>>>>                  mv "${S}/.config" "${B}/.config"
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Ryan Eatmon                reatmon@ti.com
>>>>>>> -----------------------------------------
>>>>>>> Texas Instruments, Inc.  -  LCPD  -  MGTS
>>>>>
>>>>> -=-=-=-=-=-=-=-=-=-=-=-
>>>>> Links: You receive all messages sent to this group.
>>>>> View/Reply Online (#195736): https://lists.openembedded.org/g/openembedded-core/message/195736
>>>>> Mute This Topic: https://lists.openembedded.org/mt/104311283/7501392
>>>>> Group Owner: openembedded-core+owner@lists.openembedded.org
>>>>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [andreaskleist@gmail.com]
>>>>> -=-=-=-=-=-=-=-=-=-=-=-
>>>>>
>>>>
>>>
>>
> 

-- 
Ryan Eatmon                reatmon@ti.com
-----------------------------------------
Texas Instruments, Inc.  -  LCPD  -  MGTS