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

[Luca Ceresoli via buildroot <buildroot@buildroot.org>]

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