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

[Ryan Eatmon <reatmon@ti.com>]

On 2/16/2024 2:11 AM, 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.
> 
> /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.

The meta-ti layer kernel recipes set KERNEL_LOCALVERSION.  With this 
change in place the modules directory was being name incorrectly which 
means that our boot testing failed.  For example, if we set:

KERNEL_LOCALVERSION="xxxx"

Then the directory was created:

/lib/modules-xxxx-xxxx

One too many xxxx in the name.

We never saw this issue on master in our testing because they patched 
the file and backed it out a few days later.


>>
>>
>>> 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 (#195740): https://lists.openembedded.org/g/openembedded-core/message/195740
> Mute This Topic: https://lists.openembedded.org/mt/104311283/6551054
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [reatmon@ti.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 

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