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

Ryan Eatmon
2024-02-21T14:39:17Z

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