Re: [PATCH] scripts/setlocalversion: Reinstate .scmversion support

[Rasmus Villemoes <ravi@prevas.dk>]

On Tue, Mar 04 2025, Tom Rini <trini@konsulko.com> wrote:

> On Tue, Mar 04, 2025 at 03:28:17PM +0100, Marek Vasut wrote:
>> On 3/4/25 10:24 AM, Rasmus Villemoes wrote:
>> > On Sun, Mar 02 2025, Marek Vasut <marex@denx.de> wrote:
>> > 
>> > > The .scmversion is used by oe-core to append U-Boot version string.
>> > > 
>> > > LOCALVERSION is not fully compatible replacement as it adds trailing
>> > > "-dirty" string at the end of version string in case the U-Boot git
>> > > tree contains uncommitted changes. This behavior itself is correct.
>> > > However, OE builds do clone U-Boot sources from git and may apply
>> > > additional patches on top, which are not tracked in U-Boot git tree,
>> > > but rather in the OE metalayer git tree, which leads to the addition
>> > > of "-dirty" string as well.
>> > > 
>> > 
>> > ... which is then _also_ correct, no?
>> 
>> No
>> 
>> > It means that U-Boot is built from
>> > v2024.10-321-gabcd1234 plus some entirely unknown additional patches,
>> 
>> This is not true, the patches are known and tracked in the OE layers.
>> 
>> They are not uncommited ad-hoc local changes.

Perhaps, but the resulting binary has no indication of what those
patches are or whether they are indeed under some external revision
control.

>> > > The .scmversion used by oe-core used to replace the version string
>> > > suffix fully, including the "-dirty" string. Reinstate support for
>> > > the .scmversion to let OE core do exactly that as it used to do it.
>> > 
>> > No, please don't. Let's not let this script deviate from upstream linux
>> > again, and let's instead try to fix the bug in oe-core
>> 
>> There is no bug in oe-core.
>> 
>> U-Boot commit Fixes: 5c02350fa03d ("scripts/setlocalversion: sync with linux
>> v6.9") introduced breaking change, the removal of .scmversion support . This
>> patch fixes it. It is as simple as that.

Huh? Since when isn't adapting to changes in the upstream project the
responsibility of distros and downstream meta-build systems? 

>> > that (ab)uses
>> > this legacy .scmversion file. The fundamental bug is that oe-core
>> > creates these .scmversion files unconditionally, even if they end up
>> > empty, making CONFIG_LOCALVERSION_AUTO completely useless. I have long
>> > since worked around those bugs in my own u-boot and linux recipes, but
>> > I'd much rather be able to eventually drop those workarounds.
>> > 
>> > Why doesn't doing what kernel.bbclass has done,
>> > 
>> > export LOCALVERSION = "${UBOOT_LOCALVERSION}"
>> U-Boot and Linux builds in OE-core work differently.
>> 
>> Linux uses SCC to apply patches onto a git tree (i.e. it behaves like "git
>> am *patch") and therefore the "-dirty" suffix is not added.
>> 
>> U-Boot recipe uses plain bitbake do_patch to apply patches, and therefore
>> the build tree is effectively dirty and the "-dirty" suffix is added. The
>> .scmversion overrides the dirty suffix.

I certainly don't; our u-boot and linux recipes both point at some
specific SRCREV, with any necessary patches maintained in a git repo. No
patches "maintained" in bitbake metadata.

> Yes, but that's Rasmus' point isn't it?
> 0592671fbba8c96df043d537d1f6415492de886e (v2025.01 hash) in oe-core
> isn't the same as in mainline because there's patches on top and it's
> non-obvious.

Exactly. When I have some customer device and need to debug some U-Boot
or linux problem, I want the U-Boot banner/uname -r to tell me exactly
what revision that was built from. That's why the .scmversion munging in
oe-core has always bugged me (because they've always added an _empty_
file, making CONFIG_LOCALVERSION_AUTO useless), and as I've said I've
had workarounds for that in place forever.

I strongly prefer to keep the setlocalversion script in sync with linux
as far as possible (there's a '| tr ...' due to U-Boot not being catched
up with all of kbuild) and not have to remember to forward-port patches
when re-syncing, but I'll defer to Tom now.

Rasmus