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

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

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.

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