[Buildroot] [PATCH] package/pkg-generic: store reall version in legal manifest

[Buildroot] [PATCH] package/pkg-generic: store reall version in legal manifest

[yann.morin]

From: "Yann E. MORIN" <yann.morin@orange.com>

The legal manifest currently stores the $(PKG)_VERSION variable.
However, that variable undergoes a set of changes so that it is
suitable for creating files and Makefile rules; that new value
is purely a technical, internal detail of how Buildroot handles
things.

In the legal manifest, we need access to the real value for the
version, as this is what will allow actual references to the
upstream package. If the version string is mangled, like slashes
replaced with underscores, this introduces ambiguities as to what
exactly the version is.

Change the legal manifest to include the actual, original value as
was set in the .mk file.

Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 package/pkg-generic.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index fe0a7bef38..fb9d9996c8 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -1193,7 +1193,7 @@ else
 endif # other packages
 
 endif # redistribute
-	@$$(call legal-manifest,$$(call UPPERCASE,$(4)),$$($(2)_RAWNAME),$$($(2)_VERSION),$$(subst $$(space)$$(comma),$$(comma),$$($(2)_LICENSE)),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call legal-deps,$(1)))
+	@$$(call legal-manifest,$$(call UPPERCASE,$(4)),$$($(2)_RAWNAME),$$($(2)_DL_VERSION),$$(subst $$(space)$$(comma),$$(comma),$$($(2)_LICENSE)),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call legal-deps,$(1)))
 endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
 	$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))
 
-- 
2.34.1

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/pkg-generic: store reall version in legal manifest

[Luca Ceresoli via buildroot]

Hi Yann,

On Tue, 21 Jan 2025 11:45:32 +0100
<yann.morin@orange.com> wrote:

> From: "Yann E. MORIN" <yann.morin@orange.com>
> 
> The legal manifest currently stores the $(PKG)_VERSION variable.
> However, that variable undergoes a set of changes so that it is
> suitable for creating files and Makefile rules; that new value
> is purely a technical, internal detail of how Buildroot handles
> things.
> 
> In the legal manifest, we need access to the real value for the
> version, as this is what will allow actual references to the
> upstream package. If the version string is mangled, like slashes
> replaced with underscores, this introduces ambiguities as to what
> exactly the version is.
> 
> Change the legal manifest to include the actual, original value as
> was set in the .mk file.
> 
> Signed-off-by: Yann E. MORIN <yann.morin@orange.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Luca Ceresoli <luca.ceresoli@bootlin.com>

Subject line: reall -> real.

It would be nice to shot an actual example in the commit message. I did
a very quick search and couldn't find any package where the problem
happens.

So if there is an upstream package affected I'd list that, otherwise
you can fake an example, as I did to test your patch, e.g.:

  For a package having:

    FOO_VERSION = "12/34"

  the version in the manifest actually is "12_34". With this patch it
  will be "12/34" as expected.

> -	@$$(call legal-manifest,$$(call UPPERCASE,$(4)),$$($(2)_RAWNAME),$$($(2)_VERSION),$$(subst $$(space)$$(comma),$$(comma),$$($(2)_LICENSE)),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call legal-deps,$(1)))
> +	@$$(call legal-manifest,$$(call UPPERCASE,$(4)),$$($(2)_RAWNAME),$$($(2)_DL_VERSION),$$(subst $$(space)$$(comma),$$(comma),$$($(2)_LICENSE)),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call legal-deps,$(1)))
>  endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
>  	$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))

Not strictly specific about your patch, but IMO reusing the _DL_VERSION
variable makes code less readable. The _DL_VERSION name suggests it is
for downloading, right? What about renaming it to e.g. _ORIG_VERSION,
and then using _ORIG_VERSION for both the download, the legal manifest,
pkg-stats and whatever use is currently made of _DL_VERSION?

Other than that looks good, and I'm not strict about the _DL_VERSION so:

Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/pkg-generic: store reall version in legal manifest

[yann.morin]

Luca, Al,

On 2025-01-21 16:55 +0100, Luca Ceresoli spake thusly:
> On Tue, 21 Jan 2025 11:45:32 +0100
> <yann.morin@orange.com> wrote:
[--SNIP--]
> > Change the legal manifest to include the actual, original value as
> > was set in the .mk file.
 [--SNIP--]
> Subject line: reall -> real.

:-)

> It would be nice to shot an actual example in the commit message. I did
> a very quick search and couldn't find any package where the problem
> happens.

There is no package in the Buildroot tree that is affected, when the
version string is a constant. There are a few packages (linux, uboot...)
where the user can specify a custom version from a custom git tree, and
those could contain a colon or a slash, even a space (but let's be
honest, that space would break so many other things everywhere!).

Also, packages from br2-external trees may have versions with any such
character. For example, I'm looking at Apache ant, where the tags are
something like "rel/1.10.15" for example:
    https://github.com/apache/ant/tags

Finally, private packages (used from a br2-external tree) may have such
tags as well..

[--SNIP--]
> > -	@$$(call legal-manifest,$$(call UPPERCASE,$(4)),$$($(2)_RAWNAME),$$($(2)_VERSION),$$(subst $$(space)$$(comma),$$(comma),$$($(2)_LICENSE)),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call legal-deps,$(1)))
> > +	@$$(call legal-manifest,$$(call UPPERCASE,$(4)),$$($(2)_RAWNAME),$$($(2)_DL_VERSION),$$(subst $$(space)$$(comma),$$(comma),$$($(2)_LICENSE)),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call legal-deps,$(1)))
> >  endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
> >  	$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))
> Not strictly specific about your patch, but IMO reusing the _DL_VERSION
> variable makes code less readable. The _DL_VERSION name suggests it is
> for downloading, right? What about renaming it to e.g. _ORIG_VERSION,
> and then using _ORIG_VERSION for both the download, the legal manifest,
> pkg-stats and whatever use is currently made of _DL_VERSION?

Vast subject. I'm not going to revisit that; my non-work alter-ego
previously tried, and that was not easy *at all*; talk to the people
who were in Vienna if you want a bit of details (one point is that
some packages may already make use of _DL_VERSION, especially in
br2-external trees, so we want to keep that as well, but I can't
remember the key problem I stumbled uppon, just that I went "Ouch,
burnt! No touching!").

> Other than that looks good, and I'm not strict about the _DL_VERSION so:
> 
> Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Thanks! :-)

Regards,
Yann E. MORIN.

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
|                 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
| yann.morin (at) orange.com           |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/pkg-generic: store reall version in legal manifest

[Luca Ceresoli via buildroot]

Hi Yann,

On Tue, 21 Jan 2025 17:16:37 +0100
yann.morin@orange.com wrote:

> Luca, Al,
> 
> On 2025-01-21 16:55 +0100, Luca Ceresoli spake thusly:
> > On Tue, 21 Jan 2025 11:45:32 +0100
> > <yann.morin@orange.com> wrote:  
> [--SNIP--]
> > > Change the legal manifest to include the actual, original value as
> > > was set in the .mk file.  
>  [--SNIP--]
> > Subject line: reall -> real.  
> 
> :-)
> 
> > It would be nice to shot an actual example in the commit message. I did
> > a very quick search and couldn't find any package where the problem
> > happens.  
> 
> There is no package in the Buildroot tree that is affected, when the
> version string is a constant. There are a few packages (linux, uboot...)
> where the user can specify a custom version from a custom git tree, and
> those could contain a colon or a slash, even a space (but let's be
> honest, that space would break so many other things everywhere!).
> 
> Also, packages from br2-external trees may have versions with any such
> character. For example, I'm looking at Apache ant, where the tags are
> something like "rel/1.10.15" for example:
>     https://github.com/apache/ant/tags
> 
> Finally, private packages (used from a br2-external tree) may have such
> tags as well..

Sure, make sense. If it were me, I'd add an example with one of those
packages for which one can specify the version in a configuration
setting, like U-Boot or Linux, in case you send a v2.

> [--SNIP--]
> > > -	@$$(call legal-manifest,$$(call UPPERCASE,$(4)),$$($(2)_RAWNAME),$$($(2)_VERSION),$$(subst $$(space)$$(comma),$$(comma),$$($(2)_LICENSE)),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call legal-deps,$(1)))
> > > +	@$$(call legal-manifest,$$(call UPPERCASE,$(4)),$$($(2)_RAWNAME),$$($(2)_DL_VERSION),$$(subst $$(space)$$(comma),$$(comma),$$($(2)_LICENSE)),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call legal-deps,$(1)))
> > >  endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
> > >  	$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))  
> > Not strictly specific about your patch, but IMO reusing the _DL_VERSION
> > variable makes code less readable. The _DL_VERSION name suggests it is
> > for downloading, right? What about renaming it to e.g. _ORIG_VERSION,
> > and then using _ORIG_VERSION for both the download, the legal manifest,
> > pkg-stats and whatever use is currently made of _DL_VERSION?  
> 
> Vast subject. I'm not going to revisit that; my non-work alter-ego
> previously tried, and that was not easy *at all*; talk to the people
> who were in Vienna if you want a bit of details (one point is that
> some packages may already make use of _DL_VERSION, especially in
> br2-external trees, so we want to keep that as well, but I can't
> remember the key problem I stumbled uppon, just that I went "Ouch,
> burnt! No touching!").

I hereby forget about having suggest such an improvement. O:-)

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/pkg-generic: store reall version in legal manifest

[yann.morin]

Luca, All,

On 2025-01-21 19:10 +0100, Luca Ceresoli spake thusly:
> On Tue, 21 Jan 2025 17:16:37 +0100
> yann.morin@orange.com wrote:
> > On 2025-01-21 16:55 +0100, Luca Ceresoli spake thusly:
> > > On Tue, 21 Jan 2025 11:45:32 +0100
> > > <yann.morin@orange.com> wrote:  
[--SNIP--]
> > > It would be nice to shot an actual example in the commit message. I did
> > > a very quick search and couldn't find any package where the problem
> > > happens.  
[--SNIP--]
> > Finally, private packages (used from a br2-external tree) may have such
> > tags as well..
> Sure, make sense. If it were me, I'd add an example with one of those
> packages for which one can specify the version in a configuration
> setting, like U-Boot or Linux, in case you send a v2.

Sorry, maybe I was not clear about linux and uboot: they were just given
as an example where a slash could snick in; I don't know of any publicly
accessible git repositories that has such tags for those pacakges. There
is however Apache's ant that does have such tags (although it is not
(yet) pacjaged in Buildroot).

I'll send a v2 with a rewording of my previous explanations.

Thanks!

[--SNIP--]
> > > Not strictly specific about your patch, but IMO reusing the _DL_VERSION
> > > variable makes code less readable. [--SNIP--]
> > Vast subject. [--SNIP--]

Ah, I think I remember some details now: the consensus was not about
renaming _DL_VERSION, but getting rid of it, stop mangling _VERSION and
keep it untouched, and instead introduce _VERSION_SANITISED (or
whatev'). That was causing issues of which the details still evade me
for now...

> I hereby forget about having suggest such an improvement. O:-)

No worries, that was already something that was indeed discussed because
_DL_VERSION is indeed ugly; it specifically prevents having things like
an actual version as advertised upstream (which can get tracked by
releasemonitoring.org), vs. the tag in git that often gets a 'v' prefix
(or in the Apache ant case, a whole 'rel/' prefix). Hic sunt dracones.

Regards,
Yann E. MORIN.

-- 
                                        ____________
.-----------------.--------------------:       _    :------------------.
|  Yann E. MORIN  | Real-Time Embedded |    __/ )   | /"\ ASCII RIBBON |
|                 | Software  Designer |  _/ - /'   | \ / CAMPAIGN     |
| +33 638.411.245 '--------------------: (_    `--, |  X  AGAINST      |
| yann.morin (at) orange.com           |_="    ,--' | / \ HTML MAIL    |
'--------------------------------------:______/_____:------------------'

____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot