[PATCH] scripts/setlocalversion: Reinstate .scmversion support

[Marek Vasut]

On 3/4/25 10:33 PM, Rasmus Villemoes wrote:
> On Tue, Mar 04 2025, Tom Rini <trini at 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 at 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.

Whatever indication vendors might need can be inserted via 
UBOOT_LOCALVERSION . That indication is up to vendors , not up to 
oe-core to decide.

>>>>> 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?

Adapting is fine, breaking change like this is not.

The aforementioned commit removed functionality on which other projects 
depend, that is a bug, sorry.

OE-core can be adapted to whatever new mechanism, but it should be 
adapted first and then the .scmversion can be removed from U-Boot once 
most users converted over, not the other way around.

>>>> 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.

This kind of setup usually fails when some of the random git repos that 
is poorly mirrored or not mirrored at all goes away. Then someone tries 
to build the metalayer and they cannot access the sources anymore, fail.

That is why bundling patches into the OE BSP layers is much better, if 
the user has local clone of the layer, and they depend only on well 
mirrored repositories, even if some random downstream git repository 
goes away, it is unlikely they wouldn't be able to clone upstream 
projects from somewhere and apply patches from the BSP layer on top.

>> 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 is exactly what UBOOT_LOCALVERSION is for , yes. And that 
UBOOT_LOCALVERSION functionality is currently broken and this patch 
fixes it .

> 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.

This is a separate topic, OE-core can be fixed, but existing 
functionality should not be broken.

> 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.
If this script is already out of sync, then extra four lines which are 
well documented won't make it much worse.