[Buildroot] [RFC 0/4] legal-info: save the external-toolchain source archive

[Buildroot] [RFC 0/4] legal-info: save the external-toolchain source archive

[Luca Ceresoli]

Hi Thomas, All,

during the latest Buildroot Developer Days in October 2014, prompted by
Thomas De Schampheleire, we discussed a way to improve 'make legal-info' to
collect the external toolchain source code.

The external toolchain is a special package because
TOOLCHAIN_EXTERNAL_SITE/SOURCE points to a file that is indeed a binary
archive. The "actual" source code is usually provided by toolchain vendors
as a separate file.

During the BDD we stated [1]:

  We'll need to add a FOO_ACTUAL_SOURCES to the infrastructure: if set (to a
  tarball URL) it forces 'make legal-info' to download that tarball and
  store in the tarball directory instead of the FOO_SOURCE tarball.

I took the duty of implementing the above idea, and sketched a working
implementation in the next few days. Unfortunately, due to a enduring lack
of time, that work is still sitting half-baked on my hard disks.

I'm not totally happy with the approach, and I'm not even sure this the best
way to reach the goal. So I thought I'd send this RFC and see your comments.
If there is a real interest in going forward, maybe I (or anybody else) can
either complete the job or write a different implementation, as I elaborate
below.

Now to the implementation details.

Patch 1 is a simple cleanup and may be applied even without successive
patches.

The core change is in patch 2. I added two variables to the pkg-generic infra:
FOO_ACTUAL_SOURCE_TARBALL and FOO_ACTUAL_SOURCE_SITE, which default to the
value of the good old FOO_SOURCE and FOO_SITE. Then I use the new variables
for the legal-info processing.

If these variables are overridden, which will be the case for external
toolchains, they trigger an extra $(call DOWNLOAD,...) to fetch the actuel
package source tarball.

Patches 3 and 4 set the TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_TARBALL to
appropriate values in external-toolchain.mk for most toolchains.
TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_SITE is never set because it happens to be
equal to TOOLCHAIN_EXTERNAL_SITE in all the cases I implemented.

The current solution works, but it shows some drawbacks.

First, the "actual source" is downloaded by 'make legal-info', not 'make
source'. This might be good for most users: after all, you don't need the
toolchain source code for daily development, only when releasing. These files
are large, hundreds of MBs, so saving time and bandwidth seems nice. however,
this diverges from the well-defined feature of 'make source', which is
supposed to download everything needed to later work offline.

Additionally, there's no "actual" version of FOO_EXTRA_DOWNLOADS. Thus
Blackfin toolchains, which use that feature, cannot habdled in a complete way.
Of course adding TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_EXTRA_DOWNLOADS is possible,
but I wonder if we want to add so any variables to the package infra.

Finally, there is no provision for extracting the license files from the
downloaded tarball, which would be a nice addition.

Overall, if we want this feature to complete and well-done, I feel it might
be a bit awkward and complex. Especially because this is done for one package
only.

It would be interesting to try a totally different implementation that does
not touch the generic package infra, but does all in hooks inside the
toolchain-external package. I haven't had a look at it, but it might be
simpler and maybe implement EXTRA_DOWNLOADS in a nice way as well.

That's all folks, here's the code. Be aware that it still misses docs and some
toolchains are not yet implemented (musl, Blackfin).

Regards,
Luca

[1] http://www.elinux.org/Buildroot:DeveloperDaysELCE2014#State_of_legal-info_infrastructure.2C_improvements_to_be_made.3F

Luca Ceresoli (4):
  legal-info: remove FOO_MANIFEST_TARBALL and FOO_MANIFEST_SITE defaults
  legal-info: allow to declare the actual sources for binary packages
  toolchain-externel: mass-define actual source tarball for known
    patterns
  toolchain-external: define actual sources for arago toolchains

 Makefile                                           |  1 -
 package/pkg-generic.mk                             | 25 ++++++++++++++++------
 toolchain/toolchain-external/toolchain-external.mk | 10 +++++++++
 3 files changed, 28 insertions(+), 8 deletions(-)

-- 
1.9.1

[Buildroot] [RFC 1/4] legal-info: remove FOO_MANIFEST_TARBALL and FOO_MANIFEST_SITE defaults

[Luca Ceresoli]

When FOO_SOURCE is non-empty, FOO_MANIFEST_TARBALL is always set.
When FOO_SOURCE is empty, FOO_MANIFEST_TARBALL is not set, but also
never used, due to the if below which defuses the whole legal-info
processing for packages that have FOO_SOURCE explicitly set to an
empty string.

So get rid of the default assignment to "not saved".

Do it for FOO_MANIFEST_SITE as well: it is pointless to have
FOO_MANIFEST_SITE with an empty FOO_SOURCE in a package. A quick
grep session in the sources confirmed this assumption is indeed
true for the current code.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
---
 package/pkg-generic.mk | 2 --
 1 file changed, 2 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 9643a30..38ef581 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -653,8 +653,6 @@ $(2)_MANIFEST_SITE = $$(call qstrip,$$($(2)_SITE))
 endif
 endif
 endif
-$(2)_MANIFEST_TARBALL ?= not saved
-$(2)_MANIFEST_SITE ?= not saved
 
 # legal-info: produce legally relevant info.
 $(1)-legal-info:
-- 
1.9.1

[Buildroot] [RFC 1/4] legal-info: remove FOO_MANIFEST_TARBALL and FOO_MANIFEST_SITE defaults

[Arnout Vandecappelle]

On 02/01/15 12:43, Luca Ceresoli wrote:
> When FOO_SOURCE is non-empty, FOO_MANIFEST_TARBALL is always set.
> When FOO_SOURCE is empty, FOO_MANIFEST_TARBALL is not set, but also
> never used, due to the if below which defuses the whole legal-info
> processing for packages that have FOO_SOURCE explicitly set to an
> empty string.
> 
> So get rid of the default assignment to "not saved".
> 
> Do it for FOO_MANIFEST_SITE as well: it is pointless to have
> FOO_MANIFEST_SITE with an empty FOO_SOURCE in a package. A quick
> grep session in the sources confirmed this assumption is indeed
> true for the current code.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

> ---
>  package/pkg-generic.mk | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 9643a30..38ef581 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -653,8 +653,6 @@ $(2)_MANIFEST_SITE = $$(call qstrip,$$($(2)_SITE))
>  endif
>  endif
>  endif
> -$(2)_MANIFEST_TARBALL ?= not saved
> -$(2)_MANIFEST_SITE ?= not saved
>  
>  # legal-info: produce legally relevant info.
>  $(1)-legal-info:
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [RFC 1/4] legal-info: remove FOO_MANIFEST_TARBALL and FOO_MANIFEST_SITE defaults

[Peter Korsgaard]

>>>>> "Luca" == Luca Ceresoli <luca@lucaceresoli.net> writes:

 > When FOO_SOURCE is non-empty, FOO_MANIFEST_TARBALL is always set.
 > When FOO_SOURCE is empty, FOO_MANIFEST_TARBALL is not set, but also
 > never used, due to the if below which defuses the whole legal-info
 > processing for packages that have FOO_SOURCE explicitly set to an
 > empty string.

 > So get rid of the default assignment to "not saved".

 > Do it for FOO_MANIFEST_SITE as well: it is pointless to have
 > FOO_MANIFEST_SITE with an empty FOO_SOURCE in a package. A quick
 > grep session in the sources confirmed this assumption is indeed
 > true for the current code.

 > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
 > Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>

Committed, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [RFC 2/4] legal-info: allow to declare the actual sources for binary packages

[Luca Ceresoli]

The FOO_SITE/FOO_SOURCE variables usually point to a tarball containing
source code.

For the downloaded external toolchains this is not true, the "source"
tarball actually contains binaries. This is fine for making Buildroot
work, but for legal-info we really want to ship real source code, not
binaries.

Luckily, some (hopefully all) toolchain vendors publish a downloadable
tarball containing the source code counterpart for their binary
packages.

Here we allow the user to declare the URL of this other tarball in the
pair of variables FOO_ACTUAL_SOURCE_TARBALL (by default equal to
FOO_SOURCE) and FOO_ACTUAL_SOURCE_SITE (by default equal to FOO_SITE).
If the "actual source" package can be downloaded from the same
directory as the binary package, then only FOO_ACTUAL_SOURCE_TARBALL
needs to be set.

Note this change is not strictly toolchain-specific: it might be useful
for other packages that happen to ship binaries in the same way.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
---
 Makefile               |  1 -
 package/pkg-generic.mk | 23 ++++++++++++++++++-----
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 5e0b4f2..9f96016 100644
--- a/Makefile
+++ b/Makefile
@@ -640,7 +640,6 @@ legal-info-prepare: $(LEGAL_INFO_DIR)
 	@$(call legal-manifest,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE,HOST)
 	@$(call legal-manifest,buildroot,$(BR2_VERSION_FULL),GPLv2+,COPYING,not saved,not saved,HOST)
 	@$(call legal-warning,the Buildroot source code has not been saved)
-	@$(call legal-warning,the toolchain has not been saved)
 	@cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config
 
 legal-info: dirs legal-info-clean legal-info-prepare $(TARGETS_LEGAL_INFO) \
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 38ef581..7a477af 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -648,12 +648,18 @@ ifneq ($$($(2)_SITE_METHOD),local)
 ifneq ($$($(2)_SITE_METHOD),override)
 # Packages that have a tarball need it downloaded beforehand
 $(1)-legal-info: $(1)-source $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
-$(2)_MANIFEST_TARBALL = $$($(2)_SOURCE)
-$(2)_MANIFEST_SITE = $$(call qstrip,$$($(2)_SITE))
 endif
 endif
 endif
 
+# If FOO_ACTUAL_SOURCE_TARBALL is explicitly defined, it means FOO_SOURCE is
+# indeed a binary (e.g. external toolchain) and FOO_ACTUAL_SOURCE_TARBALL/_SITE
+# point to the actual sources tarball. Use the actual sources for legal-info.
+# For msot packages the FOO_SITE/FOO_SOURCE pair points to real source code,
+# so these are the defaults for FOO_ACTUAL_*.
+$(2)_ACTUAL_SOURCE_TARBALL ?= $$($(2)_SOURCE)
+$(2)_ACTUAL_SOURCE_SITE    ?= $$(call qstrip,$$($(2)_SITE))
+
 # legal-info: produce legally relevant info.
 $(1)-legal-info:
 # Packages without a source are assumed to be part of Buildroot, skip them.
@@ -684,13 +690,20 @@ else
 # Other packages
 
 ifeq ($$($(2)_REDISTRIBUTE),YES)
+ifeq ($$($(2)_ACTUAL_SOURCE_TARBALL),)
+	@$(call legal-warning,the toolchain source code has not been saved)
+else
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
+	$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE:/=)/$$($(2)_ACTUAL_SOURCE_TARBALL))
+endif
 # Copy the source tarball (just hardlink if possible)
-	@cp -l $$(DL_DIR)/$$($(2)_SOURCE) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))) 2>/dev/null || \
-	   cp $$(DL_DIR)/$$($(2)_SOURCE) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
+	@cp -l $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))) 2>/dev/null || \
+	    cp $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
+endif # ($$($(2)_ACTUAL_SOURCE_TARBALL),)
 endif # redistribute
 
 endif # other packages
-	@$$(call legal-manifest,$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_MANIFEST_TARBALL),$$($(2)_MANIFEST_SITE),$$(call UPPERCASE,$(4)))
+	@$$(call legal-manifest,$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call UPPERCASE,$(4)))
 endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
 	$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))
 
-- 
1.9.1

[Buildroot] [RFC 2/4] legal-info: allow to declare the actual sources for binary packages

[Arnout Vandecappelle]

On 02/01/15 12:43, Luca Ceresoli wrote:
> The FOO_SITE/FOO_SOURCE variables usually point to a tarball containing
> source code.
> 
> For the downloaded external toolchains this is not true, the "source"
> tarball actually contains binaries. This is fine for making Buildroot
> work, but for legal-info we really want to ship real source code, not
> binaries.
> 
> Luckily, some (hopefully all) toolchain vendors publish a downloadable
> tarball containing the source code counterpart for their binary
> packages.
> 
> Here we allow the user to declare the URL of this other tarball in the
> pair of variables FOO_ACTUAL_SOURCE_TARBALL (by default equal to
> FOO_SOURCE) and FOO_ACTUAL_SOURCE_SITE (by default equal to FOO_SITE).
> If the "actual source" package can be downloaded from the same
> directory as the binary package, then only FOO_ACTUAL_SOURCE_TARBALL
> needs to be set.
> 
> Note this change is not strictly toolchain-specific: it might be useful
> for other packages that happen to ship binaries in the same way.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 It is adding even more complexity to the already too complex legal-info target,
but it doesn't look too bad. Although I would indeed prefer a pre-legal-info
hook. But even that wouldn't be enough, because the source reference in the
manifest would still be wrong...


 Some minor comments below.

> ---
>  Makefile               |  1 -
>  package/pkg-generic.mk | 23 ++++++++++++++++++-----
>  2 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 5e0b4f2..9f96016 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -640,7 +640,6 @@ legal-info-prepare: $(LEGAL_INFO_DIR)
>  	@$(call legal-manifest,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE,HOST)
>  	@$(call legal-manifest,buildroot,$(BR2_VERSION_FULL),GPLv2+,COPYING,not saved,not saved,HOST)
>  	@$(call legal-warning,the Buildroot source code has not been saved)
> -	@$(call legal-warning,the toolchain has not been saved)
>  	@cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config
>  
>  legal-info: dirs legal-info-clean legal-info-prepare $(TARGETS_LEGAL_INFO) \
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 38ef581..7a477af 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -648,12 +648,18 @@ ifneq ($$($(2)_SITE_METHOD),local)
>  ifneq ($$($(2)_SITE_METHOD),override)
>  # Packages that have a tarball need it downloaded beforehand
>  $(1)-legal-info: $(1)-source $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
> -$(2)_MANIFEST_TARBALL = $$($(2)_SOURCE)
> -$(2)_MANIFEST_SITE = $$(call qstrip,$$($(2)_SITE))
>  endif
>  endif
>  endif
>  
> +# If FOO_ACTUAL_SOURCE_TARBALL is explicitly defined, it means FOO_SOURCE is
> +# indeed a binary (e.g. external toolchain) and FOO_ACTUAL_SOURCE_TARBALL/_SITE
> +# point to the actual sources tarball. Use the actual sources for legal-info.
> +# For msot packages the FOO_SITE/FOO_SOURCE pair points to real source code,
> +# so these are the defaults for FOO_ACTUAL_*.
> +$(2)_ACTUAL_SOURCE_TARBALL ?= $$($(2)_SOURCE)
> +$(2)_ACTUAL_SOURCE_SITE    ?= $$(call qstrip,$$($(2)_SITE))
> +
>  # legal-info: produce legally relevant info.
>  $(1)-legal-info:
>  # Packages without a source are assumed to be part of Buildroot, skip them.
> @@ -684,13 +690,20 @@ else
>  # Other packages
>  
>  ifeq ($$($(2)_REDISTRIBUTE),YES)
> +ifeq ($$($(2)_ACTUAL_SOURCE_TARBALL),)
> +	@$(call legal-warning,the toolchain source code has not been saved)

 So, this only works for the toolchain :-)

 If you keep this approach rather than the pre-legal-info-hook, then you
probably want to use $(1) instead of toolchain. 'toolchain-external' should be
sufficiently understandable.

> +else
> +ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
> +	$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE:/=)/$$($(2)$($(PKG)_SITE:/=)_ACTUAL_SOURCE_TARBALL))

 I think the $($(PKG)_SITE:/=) construct was just introduced because for some
packages, the _SITE ends with a / and that should be stripped, and we were too
lazy to fix the packages. Hm, looks like all the the external toolchain _SITEs
end with a /...


 Regards,
 Arnout

> +endif
>  # Copy the source tarball (just hardlink if possible)
> -	@cp -l $$(DL_DIR)/$$($(2)_SOURCE) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))) 2>/dev/null || \
> -	   cp $$(DL_DIR)/$$($(2)_SOURCE) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
> +	@cp -l $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))) 2>/dev/null || \
> +	    cp $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
> +endif # ($$($(2)_ACTUAL_SOURCE_TARBALL),)
>  endif # redistribute
>  
>  endif # other packages
> -	@$$(call legal-manifest,$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_MANIFEST_TARBALL),$$($(2)_MANIFEST_SITE),$$(call UPPERCASE,$(4)))
> +	@$$(call legal-manifest,$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call UPPERCASE,$(4)))
>  endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
>  	$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))
>  
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [RFC 2/4] legal-info: allow to declare the actual sources for binary packages

[Arnout Vandecappelle]

On 02/02/15 22:47, Arnout Vandecappelle wrote:
> On 02/01/15 12:43, Luca Ceresoli wrote:
>> The FOO_SITE/FOO_SOURCE variables usually point to a tarball containing
>> source code.
>>
>> For the downloaded external toolchains this is not true, the "source"
>> tarball actually contains binaries. This is fine for making Buildroot
>> work, but for legal-info we really want to ship real source code, not
>> binaries.
>>
>> Luckily, some (hopefully all) toolchain vendors publish a downloadable
>> tarball containing the source code counterpart for their binary
>> packages.
>>
>> Here we allow the user to declare the URL of this other tarball in the
>> pair of variables FOO_ACTUAL_SOURCE_TARBALL (by default equal to
>> FOO_SOURCE) and FOO_ACTUAL_SOURCE_SITE (by default equal to FOO_SITE).
>> If the "actual source" package can be downloaded from the same
>> directory as the binary package, then only FOO_ACTUAL_SOURCE_TARBALL
>> needs to be set.
>>
>> Note this change is not strictly toolchain-specific: it might be useful
>> for other packages that happen to ship binaries in the same way.
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> 
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
>  It is adding even more complexity to the already too complex legal-info target,
> but it doesn't look too bad. Although I would indeed prefer a pre-legal-info
> hook. But even that wouldn't be enough, because the source reference in the
> manifest would still be wrong...

 Since my comments aren't serious and the pre-legal-info-hook is probably not
feasible after all, you can just repost (with my Reviewed-by tag) if you don't
feel like changing anything.


 Regards,
 Arnout

[snip]


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [RFC 2/4] legal-info: allow to declare the actual sources for binary packages

[Luca Ceresoli]

Dear Arnout,

Arnout Vandecappelle wrote:
> On 02/01/15 12:43, Luca Ceresoli wrote:
>> The FOO_SITE/FOO_SOURCE variables usually point to a tarball containing
>> source code.
>>
>> For the downloaded external toolchains this is not true, the "source"
>> tarball actually contains binaries. This is fine for making Buildroot
>> work, but for legal-info we really want to ship real source code, not
>> binaries.
>>
>> Luckily, some (hopefully all) toolchain vendors publish a downloadable
>> tarball containing the source code counterpart for their binary
>> packages.
>>
>> Here we allow the user to declare the URL of this other tarball in the
>> pair of variables FOO_ACTUAL_SOURCE_TARBALL (by default equal to
>> FOO_SOURCE) and FOO_ACTUAL_SOURCE_SITE (by default equal to FOO_SITE).
>> If the "actual source" package can be downloaded from the same
>> directory as the binary package, then only FOO_ACTUAL_SOURCE_TARBALL
>> needs to be set.
>>
>> Note this change is not strictly toolchain-specific: it might be useful
>> for other packages that happen to ship binaries in the same way.
>>
>> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
>> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
>
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>
>   It is adding even more complexity to the already too complex legal-info target,
> but it doesn't look too bad. Although I would indeed prefer a pre-legal-info
> hook. But even that wouldn't be enough, because the source reference in the
> manifest would still be wrong...

That's right.
Maybe this can be overcome in some (probably ugly) way.

Raise your hand if you'd like to see:
   FOO_TOOLCHAIN_EXTERNAL_URL_TO_WRITE_IN_LEGAL_MANIFEST = <...>

Did you discuss this patches with the other developers at the Buildroot
meeting earlier this week? Overall, what do people think about this
approach? Would it be even worth trying the hook-based implementation?

Concerning the legal-info target complexity I fully agree with you.
Some day I'd like to try to move the whole legal-info machinery out in
a helper script, much like what Yann did for the download mechanisms.
Not sure it's feasible, but it would probably make things cleaner.

>> @@ -684,13 +690,20 @@ else
>>   # Other packages
>>
>>   ifeq ($$($(2)_REDISTRIBUTE),YES)
>> +ifeq ($$($(2)_ACTUAL_SOURCE_TARBALL),)
>> +	@$(call legal-warning,the toolchain source code has not been saved)
>
>   So, this only works for the toolchain :-)
>
>   If you keep this approach rather than the pre-legal-info-hook, then you
> probably want to use $(1) instead of toolchain. 'toolchain-external' should be
> sufficiently understandable.

Sure, will do.

>
>> +else
>> +ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
>> +	$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE:/=)/$$($(2)$($(PKG)_SITE:/=)_ACTUAL_SOURCE_TARBALL))
>
>   I think the $($(PKG)_SITE:/=) construct was just introduced because for some
> packages, the _SITE ends with a / and that should be stripped, and we were too
> lazy to fix the packages. Hm, looks like all the the external toolchain _SITEs
> end with a /...

So I'll remove all of those '/'s from toolchain-external.mk first...

Regards,
-- 
Luca

[Buildroot] [RFC 2/4] legal-info: allow to declare the actual sources for binary packages

[Luca Ceresoli]

Hi,

for the records, I have not forgot this patchset...

Luca Ceresoli wrote:
> Dear Arnout,
>
> Arnout Vandecappelle wrote:
[...]
>>> +else
>>> +ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
>>> +    $(call
>>> DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE:/=)/$$($(2)$($(PKG)_SITE:/=)_ACTUAL_SOURCE_TARBALL))
>>>
>>
>>   I think the $($(PKG)_SITE:/=) construct was just introduced because
>> for some
>> packages, the _SITE ends with a / and that should be stripped, and we
>> were too
>> lazy to fix the packages. Hm, looks like all the the external
>> toolchain _SITEs
>> end with a /...
>
> So I'll remove all of those '/'s from toolchain-external.mk first...

I started working on Arnout's comment above and sent a patchset to
cleanup _SITE URLs with a trailing '/'. I sent a patchset in March
(http://lists.busybox.net/pipermail/buildroot/2015-March/121502.html)
for that.

That patchset was partially applied, but an improvement was asked before
it could be fully applied.

So the present patchset sits on the bottom of my stack, until I have
time to implement the requested improvement...

I marked this patched as "Changes Requested" in patchwork, and will
send a v2 when possible, along with the other improvements requested
in this thread (which hopefully won't push again on my stack!).

-- 
Luca

[Buildroot] [RFC 3/4] toolchain-externel: mass-define actual source tarball for known patterns

[Luca Ceresoli]

For some externel toolchain vendors the actual source code URL can be simply
derived from the binary file URL.

Here we obtain TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_TARBALL for all Mentor and
Linaro toolchains with a few $(subst) calls.

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
---
 toolchain/toolchain-external/toolchain-external.mk | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
index b07b16c..91edd4a 100644
--- a/toolchain/toolchain-external/toolchain-external.mk
+++ b/toolchain/toolchain-external/toolchain-external.mk
@@ -392,6 +392,14 @@ TOOLCHAIN_EXTERNAL_SITE = $(dir $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_URL)))
 TOOLCHAIN_EXTERNAL_SOURCE = $(notdir $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_URL)))
 endif
 
+# Some toolchain vendors have a regular file naming pattern.
+# For them, mass-define _ACTUAL_SOURCE_TARBALL based _SITE.
+ifneq ($(findstring sourcery.mentor.com/public/gnu_toolchain,$(TOOLCHAIN_EXTERNAL_SITE)),)
+TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_TARBALL ?= $(subst -i686-pc-linux-gnu.tar.bz2,.src.tar.bz2,$(subst -i686-pc-linux-gnu-i386-linux.tar.bz2,-i686-pc-linux-gnu.src.tar.bz2,$(TOOLCHAIN_EXTERNAL_SOURCE)))
+else ifneq ($(findstring http://releases.linaro.org,$(TOOLCHAIN_EXTERNAL_SITE)),)
+TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_TARBALL ?= $(subst _linux.tar.xz,_src.tar.bz2,$(TOOLCHAIN_EXTERNAL_SOURCE))
+endif
+
 # In fact, we don't need to download the toolchain, since it is already
 # available on the system, so force the site and source to be empty so
 # that nothing will be downloaded/extracted.
-- 
1.9.1

[Buildroot] [RFC 3/4] toolchain-externel: mass-define actual source tarball for known patterns

[Arnout Vandecappelle]

On 02/01/15 12:43, Luca Ceresoli wrote:
> For some externel toolchain vendors the actual source code URL can be simply
> derived from the binary file URL.
> 
> Here we obtain TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_TARBALL for all Mentor and
> Linaro toolchains with a few $(subst) calls.
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> ---
>  toolchain/toolchain-external/toolchain-external.mk | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
> index b07b16c..91edd4a 100644
> --- a/toolchain/toolchain-external/toolchain-external.mk
> +++ b/toolchain/toolchain-external/toolchain-external.mk
> @@ -392,6 +392,14 @@ TOOLCHAIN_EXTERNAL_SITE = $(dir $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_URL)))
>  TOOLCHAIN_EXTERNAL_SOURCE = $(notdir $(call qstrip,$(BR2_TOOLCHAIN_EXTERNAL_URL)))
>  endif
>  
> +# Some toolchain vendors have a regular file naming pattern.
> +# For them, mass-define _ACTUAL_SOURCE_TARBALL based _SITE.

 I actually think we should refactor the external toolchains to factor out these
common parts of the SITE and SOURCE, but that's for another day :-)


> +ifneq ($(findstring sourcery.mentor.com/public/gnu_toolchain,$(TOOLCHAIN_EXTERNAL_SITE)),)
> +TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_TARBALL ?= $(subst -i686-pc-linux-gnu.tar.bz2,.src.tar.bz2,$(subst -i686-pc-linux-gnu-i386-linux.tar.bz2,-i686-pc-linux-gnu.src.tar.bz2,$(TOOLCHAIN_EXTERNAL_SOURCE)))

 More importantly: could you split this huge line? Make removes all whitespace
at the beginning of the line after a backslash-newline, so there is no reason
not to split it.


> +else ifneq ($(findstring http://releases.linaro.org,$(TOOLCHAIN_EXTERNAL_SITE)),)
> +TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_TARBALL ?= $(subst _linux.tar.xz,_src.tar.bz2,$(TOOLCHAIN_EXTERNAL_SOURCE))

 Same here.


 Regards,
 Arnout


> +endif
> +
>  # In fact, we don't need to download the toolchain, since it is already
>  # available on the system, so force the site and source to be empty so
>  # that nothing will be downloaded/extracted.
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [RFC 4/4] toolchain-external: define actual sources for arago toolchains

[Luca Ceresoli]

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
---
 toolchain/toolchain-external/toolchain-external.mk | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
index 91edd4a..ff79c3b 100644
--- a/toolchain/toolchain-external/toolchain-external.mk
+++ b/toolchain/toolchain-external/toolchain-external.mk
@@ -259,6 +259,7 @@ TOOLCHAIN_EXTERNAL_SOURCE = arm-2014.05-29-arm-none-linux-gnueabi-i686-pc-linux-
 else ifeq ($(BR2_TOOLCHAIN_EXTERNAL_ARAGO_ARMV7A_201109),y)
 TOOLCHAIN_EXTERNAL_SITE = http://software-dl.ti.com/sdoemb/sdoemb_public_sw/arago_toolchain/2011_09/exports/
 TOOLCHAIN_EXTERNAL_SOURCE = arago-2011.09-armv7a-linux-gnueabi-sdk.tar.bz2
+TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_TARBALL = arago-toolchain-2011.09-sources.tar.bz2
 define TOOLCHAIN_EXTERNAL_FIXUP_CMDS
 	mv $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/arago-2011.09/armv7a/* $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/
 	rm -rf $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/arago-2011.09/
@@ -266,6 +267,7 @@ endef
 else ifeq ($(BR2_TOOLCHAIN_EXTERNAL_ARAGO_ARMV5TE_201109),y)
 TOOLCHAIN_EXTERNAL_SITE = http://software-dl.ti.com/sdoemb/sdoemb_public_sw/arago_toolchain/2011_09/exports/
 TOOLCHAIN_EXTERNAL_SOURCE = arago-2011.09-armv5te-linux-gnueabi-sdk.tar.bz2
+TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_TARBALL = arago-toolchain-2011.09-sources.tar.bz2
 define TOOLCHAIN_EXTERNAL_FIXUP_CMDS
 	mv $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/arago-2011.09/armv5te/* $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/
 	rm -rf $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/arago-2011.09/
-- 
1.9.1

[Buildroot] [RFC 4/4] toolchain-external: define actual sources for arago toolchains

[Arnout Vandecappelle]

On 02/01/15 12:43, Luca Ceresoli wrote:
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>

 You could actually do the same as for Linaro and Sourcery, but really I prefer
if it is specified explicitly. So

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>


 Regards,
 Arnout


> ---
>  toolchain/toolchain-external/toolchain-external.mk | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/toolchain/toolchain-external/toolchain-external.mk b/toolchain/toolchain-external/toolchain-external.mk
> index 91edd4a..ff79c3b 100644
> --- a/toolchain/toolchain-external/toolchain-external.mk
> +++ b/toolchain/toolchain-external/toolchain-external.mk
> @@ -259,6 +259,7 @@ TOOLCHAIN_EXTERNAL_SOURCE = arm-2014.05-29-arm-none-linux-gnueabi-i686-pc-linux-
>  else ifeq ($(BR2_TOOLCHAIN_EXTERNAL_ARAGO_ARMV7A_201109),y)
>  TOOLCHAIN_EXTERNAL_SITE = http://software-dl.ti.com/sdoemb/sdoemb_public_sw/arago_toolchain/2011_09/exports/
>  TOOLCHAIN_EXTERNAL_SOURCE = arago-2011.09-armv7a-linux-gnueabi-sdk.tar.bz2
> +TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_TARBALL = arago-toolchain-2011.09-sources.tar.bz2
>  define TOOLCHAIN_EXTERNAL_FIXUP_CMDS
>  	mv $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/arago-2011.09/armv7a/* $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/
>  	rm -rf $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/arago-2011.09/
> @@ -266,6 +267,7 @@ endef
>  else ifeq ($(BR2_TOOLCHAIN_EXTERNAL_ARAGO_ARMV5TE_201109),y)
>  TOOLCHAIN_EXTERNAL_SITE = http://software-dl.ti.com/sdoemb/sdoemb_public_sw/arago_toolchain/2011_09/exports/
>  TOOLCHAIN_EXTERNAL_SOURCE = arago-2011.09-armv5te-linux-gnueabi-sdk.tar.bz2
> +TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_TARBALL = arago-toolchain-2011.09-sources.tar.bz2
>  define TOOLCHAIN_EXTERNAL_FIXUP_CMDS
>  	mv $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/arago-2011.09/armv5te/* $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/
>  	rm -rf $(TOOLCHAIN_EXTERNAL_INSTALL_DIR)/arago-2011.09/
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [RFC 0/4] legal-info: save the external-toolchain source archive

[Arnout Vandecappelle]

On 02/01/15 12:43, Luca Ceresoli wrote:
[snip]
> The current solution works, but it shows some drawbacks.
> 
> First, the "actual source" is downloaded by 'make legal-info', not 'make
> source'. This might be good for most users: after all, you don't need the
> toolchain source code for daily development, only when releasing. These files
> are large, hundreds of MBs, so saving time and bandwidth seems nice. however,
> this diverges from the well-defined feature of 'make source', which is
> supposed to download everything needed to later work offline.

 I think that 'make legal-info' is indeed sufficiently different from the
'normal build' that it is OK for it to not be covered by 'make source'. So I
wouldn't worry about this drawback.



> Additionally, there's no "actual" version of FOO_EXTRA_DOWNLOADS. Thus
> Blackfin toolchains, which use that feature, cannot habdled in a complete way.
> Of course adding TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_EXTRA_DOWNLOADS is possible,
> but I wonder if we want to add so any variables to the package infra.

 I think it would be pretty easy for FOO_ACTUAL_SOURCE to be a list of files
rather than a single file, and that would nicely work around this issue.


> Finally, there is no provision for extracting the license files from the
> downloaded tarball, which would be a nice addition.

 But that's also something that could easily be added later. And actually, I
would expect that there would be a license file in the binary tarball already.
For instance, for the Sourcery toolchains it seems to be in
share/doc/<tuple>/LICENSE.txt - although that text turns out to be pretty bogus,
it doesn't even contain a reference to the GPL...


> 
> Overall, if we want this feature to complete and well-done, I feel it might
> be a bit awkward and complex. Especially because this is done for one package
> only.
> 
> It would be interesting to try a totally different implementation that does
> not touch the generic package infra, but does all in hooks inside the
> toolchain-external package. I haven't had a look at it, but it might be
> simpler and maybe implement EXTRA_DOWNLOADS in a nice way as well.

 Actually, yes, it would be a lot simpler with PRE_LEGAL_INFO_HOOKS. And then it
is also possible to extract the source tarball(s) in the build directory and
extract the license files.


 Regards,
 Arnout


> That's all folks, here's the code. Be aware that it still misses docs and some
> toolchains are not yet implemented (musl, Blackfin).
> 
> Regards,
> Luca
> 
> [1] http://www.elinux.org/Buildroot:DeveloperDaysELCE2014#State_of_legal-info_infrastructure.2C_improvements_to_be_made.3F
> 
> Luca Ceresoli (4):
>   legal-info: remove FOO_MANIFEST_TARBALL and FOO_MANIFEST_SITE defaults
>   legal-info: allow to declare the actual sources for binary packages
>   toolchain-externel: mass-define actual source tarball for known
>     patterns
>   toolchain-external: define actual sources for arago toolchains
> 
>  Makefile                                           |  1 -
>  package/pkg-generic.mk                             | 25 ++++++++++++++++------
>  toolchain/toolchain-external/toolchain-external.mk | 10 +++++++++
>  3 files changed, 28 insertions(+), 8 deletions(-)
> 


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [RFC 0/4] legal-info: save the external-toolchain source archive

[Luca Ceresoli]

Dear Arnout,

thanks for the review.
I guess it means there is some interest in having this feature.

Arnout Vandecappelle wrote:
> On 02/01/15 12:43, Luca Ceresoli wrote:
> [snip]
>> The current solution works, but it shows some drawbacks.
>>
>> First, the "actual source" is downloaded by 'make legal-info', not 'make
>> source'. This might be good for most users: after all, you don't need the
>> toolchain source code for daily development, only when releasing. These files
>> are large, hundreds of MBs, so saving time and bandwidth seems nice. however,
>> this diverges from the well-defined feature of 'make source', which is
>> supposed to download everything needed to later work offline.
>
>   I think that 'make legal-info' is indeed sufficiently different from the
> 'normal build' that it is OK for it to not be covered by 'make source'. So I
> wouldn't worry about this drawback.

Good to see we are on the same page. Mentioning this behaviour in the
docs should be enough.

>> Additionally, there's no "actual" version of FOO_EXTRA_DOWNLOADS. Thus
>> Blackfin toolchains, which use that feature, cannot habdled in a complete way.
>> Of course adding TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_EXTRA_DOWNLOADS is possible,
>> but I wonder if we want to add so any variables to the package infra.
>
>   I think it would be pretty easy for FOO_ACTUAL_SOURCE to be a list of files
> rather than a single file, and that would nicely work around this issue.

Good idea. It would work well provided that all files are hosted on the
same directory. This seems to be the case for the Blackfin toolchains.

After all, it is the same limitation we have for FOO_EXTRA_DOWNLOADS.

Regards,
-- 
Luca