[PATCH] scripts/setlocalversion: Reinstate .scmversion support

[PATCH] scripts/setlocalversion: Reinstate .scmversion support

[Marek Vasut]

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.

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.

Fixes: 5c02350fa03d ("scripts/setlocalversion: sync with linux v6.9")
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
Cc: Tom Rini <trini@konsulko.com>
Cc: u-boot@lists.denx.de
---
 scripts/setlocalversion | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/setlocalversion b/scripts/setlocalversion
index dbe048210d6..26d121816dd 100755
--- a/scripts/setlocalversion
+++ b/scripts/setlocalversion
@@ -48,6 +48,10 @@ scm_version()
 	done
 
 	cd "$srctree"
+	if test -e .scmversion; then
+		cat .scmversion
+		return
+	fi
 
 	if test -n "$(git rev-parse --show-cdup 2>/dev/null)"; then
 		return
-- 
2.47.2

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

[Rasmus Villemoes]

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? It means that U-Boot is built from
v2024.10-321-gabcd1234 plus some entirely unknown additional patches,
which can be anything from fixing a typo in a printf string to
completely overhauling everything.

> 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 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}"

work for you?

Rasmus

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

[Marek Vasut]

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.

> which can be anything from fixing a typo in a printf string to
> completely overhauling everything.
> 
>> 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.

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

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2902 bytes --]

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.
> 
> > which can be anything from fixing a typo in a printf string to
> > completely overhauling everything.
> > 
> > > 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.
> 
> > 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.

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.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

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

[Rasmus Villemoes]

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

Re: [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@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.

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.

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

[Marek Vasut]

On 3/4/25 3:42 PM, Tom Rini wrote:

[...]

>>> 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.
> 
> 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.
That still does not mean the repository is dirty , it only means the 
patches are tracked in a different git tree (the OE layer one).

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

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1852 bytes --]

On Sun, Mar 02, 2025 at 07:30:28PM +0100, Marek Vasut 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.
> 
> 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.
> 
> Fixes: 5c02350fa03d ("scripts/setlocalversion: sync with linux v6.9")
> Signed-off-by: Marek Vasut <marex@denx.de>

So, I've talked with Marek off-list, a lot. We have a few sets of
problems, both with U-Boot and with OE. In U-Boot, we don't have the
software-clout to force other projects to adapt to our whims, unlike the
linux kernel. So as much as I don't like re-introducing this, we need to
for a transition window at least their localversion support is just not
functioning. But from the U-Boot point of view, we need to get OE (and
anyone else out there!) to stop relying on .scmversion. It will go away
again. The other problem is OE (and anyone else!) that was relying on
this mechanism to patch out "-dirty" and not at the same time provide a
meaningful version distinction. This also needs to be fixed in other
projects.

So with all of that said, I've applied this to u-boot/master now, and in
some number of years once whatever OE LTS doesn't rely on this at least,
we'll rip it out again.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]