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

[Andreas Helbech Kleist <andreaskleist@gmail.com>]

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.

> 
> 
> > 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]
> -=-=-=-=-=-=-=-=-=-=-=-
>