legal-info improvements and completeness (branch yem/legal-2)

In some cases, the toolchain sources are now recovered and available in the legal-info output. So, adapt the header to use conditional instead of an definitive negation.

Also update the part about saving the sources: it's not the license list that defines whether sources are installed, but rather whether the package is redistributable or not.

Update the header accordingly.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

Acked-by Luca Ceresoli <[email protected]>

Acked-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

---
Changes v2 -> v3:
- typoes (Arnout)

As we currently download the actual sources as part of saving the legal-info, we do not check the hashes of those downloads.

That's because, during legal-info, there is not package involved, and thus there's no path to an actual .hash file.

However, this precludes legal-info from working in off-line mode. A subsequent patch will make it possible to do so, and actual sources will be downloaded as another classical package download.

This will have two consequences:

- first, we will be able to add hashes for actual sources, so we can ensure their integrity,

- second, and as a direct consequence of the above, when a .hash file is present, it would have to list all the hashes for that package, or that would be treated as an error.

Currently, the only package that falls in this case is the external- toolchain, for which we have means to retrieve the sources for some of the toolchains.

So we just add hashes for those actual external-toolchain sources we may have to download.

Those hashes are not used for now, but they'll come into play a few patches down.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 toolchain/toolchain-external/toolchain-external.hash | 5 +++++
 1 file changed, 5 insertions(+)

toolchain/toolchain-external/toolchain-external.hash +0 -0

index dd7073f..22bbe51 100644

toolchain/toolchain-external/toolchain-external.hash +4 -0

 # NiosII
 sha256 cc47745dc1264fcb8fb98fb1315ab772ab98691396021c455229b58abaf887f5  sourceryg++-2015.11-27-nios2-linux-gnu-i686-pc-linux-gnu.tar.bz2
 # PowerPC

@@ -29,13 +30,17 @@ sha256 8ea78c5988b2bb507534f1ad46aa46659f66b39d55f2fc40e163a90b4195e70f  aarch64
 # ARM toolchains from Texas Instrument's Arago project
 sha256 f2febf3b3c565536461ad4405f1bcb835d75a6afb2a8bec958a1248cb4b81fc7  arago-2011.09-armv7a-linux-gnueabi-sdk.tar.bz2
 sha256 254af7d02eb3bcc8345c78e131700bc995d65b68232caaed21150a5fd1456070  arago-2011.09-armv5te-linux-gnueabi-sdk.tar.bz2
+sha256 25fbf0513ad7322b15cbaae964cafadcbb4c939f2708f57f40b8f9f2d601122b  arago-toolchain-2011.09-sources.tar.bz2
 
 # ARM and Aarch64 toolchains from Linaro
 sha256 0cffac0caea0eb3c8bdddfa14be011ce366680f40aeddbefc7cf23cb6d4f1891  gcc-linaro-arm-linux-gnueabihf-4.9-2014.09_linux.tar.xz
+sha256 eafeb3a5247e9ce31aa35d812e296fba5d5f1443e106d9bef9e38d3ee3ade006  gcc-linaro-arm-linux-gnueabihf-4.9-2014.09_src.tar.bz2
 sha256 34812c5d0556db86259ac6eb3f8bcf4ce8eca3fa8d7180875958492a42e9853f  gcc-linaro-5.1-2015.08-x86_64_arm-linux-gnueabihf.tar.xz
 sha256 4bc9d86390f8fa67a693ba4768ba5b12faaf7dd37c706c05ccd9321e765226e4  gcc-linaro-armeb-linux-gnueabihf-4.9-2014.09_linux.tar.xz
+sha256 bf5d3111dad5aa9aef0e955875fb7fc9e918cb24519af7014dd67a9e581a49b1  gcc-linaro-armeb-linux-gnueabihf-4.9-2014.09_src.tar.bz2
 sha256 24b86799a6c64380c740bf31a700b46e854fc0a821da2341e9868f0196c864de  gcc-linaro-5.1-2015.08-x86_64_armeb-linux-gnueabihf.tar.xz
 sha256 3954f496ab01de67241109e82abfaa9b7625fdab4f05e79e7902e9814a07b832  gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux.tar.xz
+sha256 a7b8f842fdc9d9be22ca4e0c999429780fc6f16ea798b032421b5ec0cfa53b3e  gcc-linaro-aarch64-linux-gnu-4.9-2014.09_src.tar.bz2
 sha256 b9137008744d9009877f662dbac7481d673cdcb1798e727e325a37c98a0f63da  gcc-linaro-5.1-2015.08-x86_64_aarch64-linux-gnu.tar.xz
 
 # Codescape toolchains from Imagination Technologies

Quoted 2 lines

As we currently download the actual sources as part of saving the legal-info, we do not check the hashes of those downloads.

Quoted 2 lines

So we just add hashes for those actual external-toolchain sources we may have to download.

toolchain/toolchain-external/toolchain-external.hash sha256 c65b1b4b918d5185349d62a3b7bf43b4b21e1175c52598ec047ca56b3f11d857 blackfi · line 15

Quoted 9 lines

toolchain/toolchain-external/toolchain-external.hash +1 -0

index dd7073f..22bbe51 100644
@@ -12,6 +12,7 @@ sha256 c65b1b4b918d5185349d62a3b7bf43b4b21e1175c52598ec047ca56b3f11d857  blackfi
 # Mentor's Sourcery CodeBench Lite toolchains
 # ARM
 sha256 39ee0e789034334ecc89af94e838e3a4815400ac5ff980f808f466b04778532e  arm-2014.05-29-arm-none-linux-gnueabi-i686-pc-linux-gnu.tar.bz2
+sha256 e16a5b1e41d7ff1e74161f9405182001bc8d1360d89564e73911032e6966cc0d  arm-2014.05-29-arm-none-linux-gnueabi.src.tar.bz2

Why only this one, and not the six other Sourcery toolchains?

Arnout, All,

On 2016-01-31 23:30 +0100, Arnout Vandecappelle spake thusly:

Quoted 3 lines · Yann E. MORIN

Quoted 2 lines

As we currently download the actual sources as part of saving the legal-info, we do not check the hashes of those downloads.

[--SNIP--]

Quoted 2 lines

Quoted 2 lines

So we just add hashes for those actual external-toolchain sources we may have to download.

[--SNIP--]

Quoted 11 lines

toolchain/toolchain-external/toolchain-external.hash sha256 c65b1b4b918d5185349d62a3b7bf43b4b21e1175c52598ec047ca56b3f11d857 blackfi · line 15

Quoted 9 lines

toolchain/toolchain-external/toolchain-external.hash +1 -0

index dd7073f..22bbe51 100644
@@ -12,6 +12,7 @@ sha256 c65b1b4b918d5185349d62a3b7bf43b4b21e1175c52598ec047ca56b3f11d857  blackfi
 # Mentor's Sourcery CodeBench Lite toolchains
 # ARM
 sha256 39ee0e789034334ecc89af94e838e3a4815400ac5ff980f808f466b04778532e  arm-2014.05-29-arm-none-linux-gnueabi-i686-pc-linux-gnu.tar.bz2
+sha256 e16a5b1e41d7ff1e74161f9405182001bc8d1360d89564e73911032e6966cc0d  arm-2014.05-29-arm-none-linux-gnueabi.src.tar.bz2

Why only this one, and not the six other Sourcery toolchains?

Because I did not add them. ;-)

Seriously, I just forgot. And it takes quite some time to get them all.

I've added hashes for all the CS toolchains now, but maybe there are other toolchains for which we're still missing the hashes for the source tarball. Maybe we can just add them whenever someone stumbles on it?

This macro will try to copy a source file into a destination fdirectory,

by first atempting a hard-link, and falling back to a plain copy.

In some situations, it will be necessary that the destination file is named differently than the source (e.g. due to a re-numbering), so if a third argument is specified, it is treated as the basename of the destination file.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

---
Changes v2 -> v3;
- use "ln" instead of "cp -l"
- accept third argument, as the basename of the destination file
- drop reviewed-by and tested-by tags given in v2, due to the above two changes

Changes RFC -> v1:
- move to pkg-utils (Luca)

---
  package/pkg-utils.mk | 25 +++++++++++++++++++++++++
  1 file changed, 25 insertions(+)

package/pkg-utils.mk +4 -0

index 44bd2c9..6479cd8 100644
@@ -113,6 +113,31 @@ $$(error Package error: use $(2) instead of $(1). Please fix your .mk file)
  endif
  endef
 
+################################################################################
+# hardlink-copy -- hardlink source and destination if possible, otherwise
+# do a simple copy
+#

Quoted 1 line · Yann E. MORIN

This macro will try to copy a source file into a destination fdirectory,

fdirectory -> directory

Quoted 1 line

by first atempting a hard-link, and falling back to a plain copy.

atempting a -> attempting to

With typos fixed:

Reviewed-by Luca Ceresoli <[email protected]>

[Ran 'make legal-info' with output dir on {the same,another} fs]

Tested-by Luca Ceresoli <[email protected]>

This macro will try to copy a source file into a destination fdirectory, by first atempting a hard-link, and falling back to a plain copy.

In some situations, it will be necessary that the destination file is named differently than the source (e.g. due to a re-numbering), so if a third argument is specified, it is treated as the basename of the destination file.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

---
Changes v2 -> v3;
- use "ln" instead of "cp -l"
- accept third argument, as the basename of the destination file
- drop reviewed-by and tested-by tags given in v2, due to the above two changes

Changes RFC -> v1:
- move to pkg-utils (Luca)

---
 package/pkg-utils.mk | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

package/pkg-utils.mk +10 -0

index 44bd2c9..6479cd8 100644
@@ -113,6 +113,31 @@ $$(error Package error: use $(2) instead of $(1). Please fix your .mk file)
 endif
 endef
 
+################################################################################
+# hardlink-copy -- hardlink source and destination if possible, otherwise
+# do a simple copy
+#
+# argument 1 is the source *file*
+# argument 2 is the destination *directory*
+# argument 3 is the basename of the destination file (optional, defaults to
+#            the basename of the source file.
+#
+# examples:

package/pkg-utils.mk +5 -0

+		  cp $(1) $(2)$(if $(3),/$(strip $(3))); \
+		} \
+	}
+endef
+
 #
 # legal-info helper functions
 #

Quoted 47 lines · Yann E. MORIN

This macro will try to copy a source file into a destination fdirectory, by first atempting a hard-link, and falling back to a plain copy.

In some situations, it will be necessary that the destination file is named differently than the source (e.g. due to a re-numbering), so if a third argument is specified, it is treated as the basename of the destination file.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

---
Changes v2 -> v3;
- use "ln" instead of "cp -l"
- accept third argument, as the basename of the destination file
- drop reviewed-by and tested-by tags given in v2, due to the above two changes

Changes RFC -> v1:
- move to pkg-utils (Luca)

---
  package/pkg-utils.mk | 25 +++++++++++++++++++++++++
  1 file changed, 25 insertions(+)

package/pkg-utils.mk +15 -0

index 44bd2c9..6479cd8 100644
@@ -113,6 +113,31 @@ $$(error Package error: use $(2) instead of $(1). Please fix your .mk file)
  endif
  endef
 
+################################################################################
+# hardlink-copy -- hardlink source and destination if possible, otherwise
+# do a simple copy
+#
+# argument 1 is the source *file*
+# argument 2 is the destination *directory*
+# argument 3 is the basename of the destination file (optional, defaults to
+#            the basename of the source file.
+#
+# examples:
+#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir/)
+#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir/,new-name)
+#
+# Note: we make that a single command, so se can:
+#  - use '$(Q)' in front of it and properly silence the whole macro,

package/pkg-utils.mk $$(error Package error: use $(2) instead of $(1). Please fix your .mk file) · line 135

Quoted 5 lines

package/pkg-utils.mk +5 -0

+#  - use '|| exit 1' after it, so we can exit on error in compound commands.
+################################################################################
+define hardlink-copy
+	{ mkdir -p $(2) && \
+		{ ln $(1) $(2)$(if $(3),/$(strip $(3))) 2>/dev/null || \

Shouldn't this be ln -f (and maybe cp -f) to make sure it behaves the same when
you run it a second time?

Hi,

Quoted 54 lines · Arnout Vandecappelle

Quoted 53 lines · Yann E. MORIN

This macro will try to copy a source file into a destination fdirectory, by first atempting a hard-link, and falling back to a plain copy.

In some situations, it will be necessary that the destination file is named differently than the source (e.g. due to a re-numbering), so if a third argument is specified, it is treated as the basename of the destination file.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

---
Changes v2 -> v3;
- use "ln" instead of "cp -l"
- accept third argument, as the basename of the destination file
- drop reviewed-by and tested-by tags given in v2, due to the above two changes

Changes RFC -> v1:
- move to pkg-utils (Luca)

---
  package/pkg-utils.mk | 25 +++++++++++++++++++++++++
  1 file changed, 25 insertions(+)

package/pkg-utils.mk +17 -0

index 44bd2c9..6479cd8 100644
@@ -113,6 +113,31 @@ $$(error Package error: use $(2) instead of $(1). Please
 fix your .mk file)
  endif
  endef
 
+################################################################################
 
+# hardlink-copy -- hardlink source and destination if possible, otherwise
+# do a simple copy
+#
+# argument 1 is the source *file*
+# argument 2 is the destination *directory*
+# argument 3 is the basename of the destination file (optional, defaults to
+#            the basename of the source file.
+#
+# examples:
+#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir/)
+#   $(call
 hardlink-copy,/path/to/source/file,/path/to/destination/dir/,new-name)
+#
+# Note: we make that a single command, so se can:
+#  - use '$(Q)' in front of it and properly silence the whole macro,
+#  - use '|| exit 1' after it, so we can exit on error in compound commands.
+################################################################################

Quoted 6 lines

package/pkg-utils.mk $$(error Package error: use $(2) instead of $(1). Please fix your .mk file) · line 135

Quoted 3 lines

package/pkg-utils.mk +3 -0

+define hardlink-copy
+    { mkdir -p $(2) && \
+        { ln $(1) $(2)$(if $(3),/$(strip $(3))) 2>/dev/null || \

Shouldn't this be ln -f (and maybe cp -f) to make sure it behaves the same when
you run it a second time?

Indeed it's true. Not an issue for 'make legal-info' since it wipes the output dir before copying file in it, but the macro is generic so it has to handle this case.

This definitely applies to ln. cp normally doesn't need it, but it does not hurt.

Move the declarations of _ACTUAL_SOURCE and _ACTUAL_SITE earlier, so that they are close to where _SOURCE and _SITE are handled.

This looks so far like a purely cosmetic change, but makes more sense with the follow-up patch, where we'll need them earlier in the file.

Quoted 5 lines · Yann E. MORIN

Move the declarations of _ACTUAL_SOURCE and _ACTUAL_SITE earlier, so that they are close to where _SOURCE and _SITE are handled.

This looks so far like a purely cosmetic change, but makes more sense with the follow-up patch, where we'll need them earlier in the file.

Not sure in which patch you need this...

but it's nice to have
$(2)_SOURCE and $(2)_ACTUAL_SOURCE_* defined at the same place, thus:

Reviewed-by Luca Ceresoli <[email protected]>

Tested-by Luca Ceresoli <[email protected]>

Move the declarations of _ACTUAL_SOURCE and _ACTUAL_SITE earlier, so that they are close to where _SOURCE and _SITE are handled.

This looks so far like a purely cosmetic change, but makes more sense with the follow-up patch, where we'll need them earlier in the file.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
Changes v2 -> v3:
- move that to its own patch (Arnout)

---
 package/pkg-generic.mk | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

package/pkg-generic.mk +8 -8

index 893ee07..5958029 100644
@@ -427,6 +427,14 @@ ifndef $(2)_SOURCE
  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 most 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))
+
 ifndef $(2)_PATCH
  ifdef $(3)_PATCH
   $(2)_PATCH = $$($(3)_PATCH)

@@ -763,14 +771,6 @@ 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 most 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.

Almost all packages which are saved for legal-info have their source archives downloaded as part of 'make source', which makes an off-line build completely possible [0].

However, for the pre-configured external toolchains, the source tarball is different, as the main tarball is a binary package. And that source tarball is only downloaded during the legal-info phase, which makes it inconvenient for full off-line builds.

We fix that by adding a new rule, $(1)-legal-source which only $(1)-all-source depends on, so that we only download it for a top-level 'make source', not as part of the standard download mechanism (i.e. only what is really needed to build).

This way, we can do a complete [0] off-line build and are still able to generate legal-info, while at the same time we do not incur any download overhead during a simple build.

Also, we previously downloaded the _ACTUAL_SOURCE_TARBALL when it was not empty. However, since _ACTUAL_SOURCE_TARBALL defaults to the value of _SOURCE, it can not be empty when _SOURCE is not. Thus, we'd get a spurious report of a missing hash for the tarball, since it was not in a standard package rule (configure, build, install..) and thus would miss the PKG and PKGDIR variables to find the .hash file.

We fix that in this commit as well, by:

- setting PKG and PKGDIR just for the -legal-source rule;

- only downloading _ACTUAL_SOURCE_TARBALL if it is not empty *and* not the same as _SOURCE (to avoid a second report about the hash).

[0] Save for nodejs which invarriably wants to download stuff at build time. Sigh... :-( Fixing that is work for another time...

package/pkg-generic.mk +3 -0

@@ -771,6 +775,20 @@ endif
  endif
  endif
 
+# Download actual sources if they differ from the extracted sources
+# (e.g. for external toolchains)
+#

Quoted 29 lines · Yann E. MORIN

Almost all packages which are saved for legal-info have their source archives downloaded as part of 'make source', which makes an off-line build completely possible [0].

However, for the pre-configured external toolchains, the source tarball is different, as the main tarball is a binary package. And that source tarball is only downloaded during the legal-info phase, which makes it inconvenient for full off-line builds.

We fix that by adding a new rule, $(1)-legal-source which only $(1)-all-source depends on, so that we only download it for a top-level 'make source', not as part of the standard download mechanism (i.e. only what is really needed to build).

This way, we can do a complete [0] off-line build and are still able to generate legal-info, while at the same time we do not incur any download overhead during a simple build.

Also, we previously downloaded the _ACTUAL_SOURCE_TARBALL when it was not empty. However, since _ACTUAL_SOURCE_TARBALL defaults to the value of _SOURCE, it can not be empty when _SOURCE is not. Thus, we'd get a spurious report of a missing hash for the tarball, since it was not in a standard package rule (configure, build, install..) and thus would miss the PKG and PKGDIR variables to find the .hash file.

We fix that in this commit as well, by:

- setting PKG and PKGDIR just for the -legal-source rule;

Quoted 5 lines

- only downloading _ACTUAL_SOURCE_TARBALL if it is not empty *and* not the same as _SOURCE (to avoid a second report about the hash).

[0] Save for nodejs which invarriably wants to download stuff at build time. Sigh... :-( Fixing that is work for another time...

Thanks for the detailed comment!
I didn't even find any typo... :) but I have a note below.

Quoted 7 lines

package/pkg-generic.mk +3 -0

@@ -771,6 +775,20 @@ endif
 endif
 endif
 
+# Download actual sources if they differ from the extracted sources
+# (e.g. for external toolchains)
+#

package/pkg-generic.mk endif · line 790

Quoted 10 lines

package/pkg-generic.mk +10 -0

+# We need to provide PKG and PKGDIR, because there's no .stamp file for
+# the legal-info step.
+$(1)-legal-source:	PKG=$(2)
+$(1)-legal-source:	PKGDIR=$(pkgdir)
+$(1)-legal-source:
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),)
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
+	$$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))
+endif # actual sources != sources
+endif # actual sources != ""

This created a side effect that is harmless, but a bit unpleasant.

Before this patch, running 'make source' with all sources already fetched did not produce any output. Now, when using a package that defines _ACTUAL_SOURCE_SITE, it does:

$ make source
arm-2014.05-29-arm-none-linux-gnueabi.src.tar.bz2: OK (sha256: [...])
$

This is caused by not having a stamp file for the actual source tarball. I suppose we should create a .stamp_actual_downloaded.

For the rest the patch looks good and it works.

Luca, All,

On 2016-01-30 15:56 +0100, Luca Ceresoli spake thusly:

Quoted 30 lines

Quoted 29 lines · Yann E. MORIN

Almost all packages which are saved for legal-info have their source archives downloaded as part of 'make source', which makes an off-line build completely possible [0].

However, for the pre-configured external toolchains, the source tarball is different, as the main tarball is a binary package. And that source tarball is only downloaded during the legal-info phase, which makes it inconvenient for full off-line builds.

We fix that by adding a new rule, $(1)-legal-source which only $(1)-all-source depends on, so that we only download it for a top-level 'make source', not as part of the standard download mechanism (i.e. only what is really needed to build).

This way, we can do a complete [0] off-line build and are still able to generate legal-info, while at the same time we do not incur any download overhead during a simple build.

Also, we previously downloaded the _ACTUAL_SOURCE_TARBALL when it was not empty. However, since _ACTUAL_SOURCE_TARBALL defaults to the value of _SOURCE, it can not be empty when _SOURCE is not. Thus, we'd get a spurious report of a missing hash for the tarball, since it was not in a standard package rule (configure, build, install..) and thus would miss the PKG and PKGDIR variables to find the .hash file.

We fix that in this commit as well, by:

- setting PKG and PKGDIR just for the -legal-source rule;

Quoted 8 lines

Quoted 5 lines

- only downloading _ACTUAL_SOURCE_TARBALL if it is not empty *and* not the same as _SOURCE (to avoid a second report about the hash).

[0] Save for nodejs which invarriably wants to download stuff at build time. Sigh... :-( Fixing that is work for another time...

Thanks for the detailed comment!
I didn't even find any typo... :) but I have a note below.

Hehe! ;-)

Quoted 24 lines

Quoted 7 lines

package/pkg-generic.mk +3 -0

@@ -771,6 +775,20 @@ endif
  endif
  endif
 
+# Download actual sources if they differ from the extracted sources
+# (e.g. for external toolchains)
+#

package/pkg-generic.mk endif · line 790

Quoted 10 lines

package/pkg-generic.mk +10 -0

+# We need to provide PKG and PKGDIR, because there's no .stamp file for
+# the legal-info step.
+$(1)-legal-source:	PKG=$(2)
+$(1)-legal-source:	PKGDIR=$(pkgdir)
+$(1)-legal-source:
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),)
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
+	$$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))
+endif # actual sources != sources
+endif # actual sources != ""

This created a side effect that is harmless, but a bit unpleasant.

Before this patch, running 'make source' with all sources already fetched did not produce any output. Now, when using a package that defines _ACTUAL_SOURCE_SITE, it does:

Quoted 6 lines

$ make source
arm-2014.05-29-arm-none-linux-gnueabi.src.tar.bz2: OK (sha256: [...])
$

This is caused by not having a stamp file for the actual source tarball. I suppose we should create a .stamp_actual_downloaded.

Hmmm.. Indeed, that's not very nice... :-( I'll see to use a stamp file.

Side note in this respect: is _ACTUAL_SOURCE supposed to support multiple archives in there? If yes, then the current code (before that patch) is not correct, as it does not treat _ACTUAL_SOURCE as a list and does not iterate over it when calling the DOWNLOAD macro.

I think it would make sense to support to support a list in there, but that is work for a separate patch.

Yann E. MORIN wrote:

Quoted 7 lines

Side note in this respect: is _ACTUAL_SOURCE supposed to support multiple archives in there? If yes, then the current code (before that patch) is not correct, as it does not treat _ACTUAL_SOURCE as a list and does not iterate over it when calling the DOWNLOAD macro.

I think it would make sense to support to support a list in there, but that is work for a separate patch.

It would be nice, but I don't think there is any actual use case in the foreseeable future.

Currently, the legal-info infra only saves the source archive of a package. However, that's not enough as we may apply some patches on packages sources.

We do suggest users to also redistribute the Buildroot sources as part of their compliance distribution, so the patches bundled in Buildroot would indeed be included in the compliance distribution.

However, that's still not enough, since we may download some patches, or the user may use a global patch directory. Patches in there might not end up in the compliance distribution, and there are risks of non-conformity.

So, always include patches alongside the source archive.

To ensure reproducibility, we also generate a series file, so patches can be re-applied in the correct order.

We get the list of patches to include from the list of patches that were applied by the package infrastructure (via the apply-patches support script). So, we need to get packages properly extracted and patched before we can save their legal-info, not just in the case they define _LICENSE_FILES.

Update the legal-info header accordingly.

Note: this means that, when a package is not patched and defines no LICENSE_FILES, we will extract and patch it for nothing. There is no easy way to know whether we have to patch a package or not. We can only either duplicate the logic to detect patches (bad) or rely on the infra actually patching the package. Also, a vast majority of packages are either patched, or define _LICENSE_FILES, so it is best and easiest to always extract and patch them prior to legal-info.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

Tested-by Luca Ceresoli <[email protected]>

---
Changes v2 -> v3:
- also mention that patches have been saved (Luca)

Changes v1 -> v2:
- don't recompute rawname-version needlessly (Luca)

---
  package/pkg-generic.mk           | 13 ++++++++-----
  support/legal-info/README.header |  9 +++++----
  2 files changed, 13 insertions(+), 9 deletions(-)

package/pkg-generic.mk +8 -5

index 7ad3bc3..c5b66db 100644
@@ -759,12 +759,10 @@ $(2)_MANIFEST_LICENSE_FILES = $$($(2)_LICENSE_FILES)
  endif
  $(2)_MANIFEST_LICENSE_FILES ?= not saved
 
-# If the package declares _LICENSE_FILES, we need to extract it,
-# for overriden, local or normal remote packages alike, whether
-# we want to redistribute it or not.
-ifneq ($$($(2)_LICENSE_FILES),)
+# We need to extract and patch a package to be able to retrieve its
+# license files (if any) and the list of patches applied to it (if
+# any).
  $(1)-legal-info: $(1)-patch
-endif
 
  # We only save the sources of packages we want to redistribute, that are
  # non-local, and non-overriden. So only store, in the manifest, the tarball

@@ -826,6 +824,11 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
  	$$(Q)$$(call hardlink-copy,\
  		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
  		     $$($(2)_REDIST_SOURCES_DIR))
+# Copy patches and generate the series file
+	$$(Q)while read f; do \
+		$$(call hardlink-copy,$$$${f},$$($(2)_REDIST_SOURCES_DIR)) || exit 1; \
+		printf "%s\n" "$$$${f##*/}" >>$$($(2)_REDIST_SOURCES_DIR)/series || exit 1; \
+	done <$$($(2)_DIR)/.applied_patches_list
  endif # redistribute
 
  endif # other packages

support/legal-info/README.header +0 -0

index d07c45d..0b5aa66 100644

Patches we save can come from various locations;
- bundled with Buildroot
- downloaded
- from one or more global-patch-dir

It is possible that two patches lying into different locations have the same basename, like so (first is bundled, second is from an hypothetical global-patch-dir):

     package/foo/0001-fix-Makefile.patch
     /path/to/my/patches/foo/0001-fix-Makefile.patch

In that case, we'd save onlt the second patch, overwriting the first.

We fix that by forcibly prefixing saved patches with a new numbering, to guarantee the unicity of saved files.

The unfortunate side-effects are that:
- most saved patches will be twice-numbered,
- the series file is now redundant.

While the former is mostly not nice-looking, we shouldn't try to strip any leading numbering, as that might not be a numbering (e.g. 42-retries.patch which is a patch add 42 retries).

The latter is not really problematic. A lot of tools (and people) can work with, and prefer to have a series file.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

---
Note that I've made this a separate change, not because the patch would be too complex if squahed with the previous, but rather to have a more detailed commit log about the reason for the renumbering; squashing the two changes together would make for a really long commit log, at the risk of being more difficult to follow.

---
  package/pkg-generic.mk | 13 ++++++++++---
  1 file changed, 10 insertions(+), 3 deletions(-)

package/pkg-generic.mk +0 -3

index c5b66db..f6132b3 100644
@@ -825,9 +825,16 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
  		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
  		     $$($(2)_REDIST_SOURCES_DIR))
  # Copy patches and generate the series file
-	$$(Q)while read f; do \
-		$$(call hardlink-copy,$$$${f},$$($(2)_REDIST_SOURCES_DIR)) || exit 1; \
-		printf "%s\n" "$$$${f##*/}" >>$$($(2)_REDIST_SOURCES_DIR)/series || exit 1; \

Quoted 11 lines · Yann E. MORIN

Patches we save can come from various locations;
- bundled with Buildroot
- downloaded
- from one or more global-patch-dir

It is possible that two patches lying into different locations have the same basename, like so (first is bundled, second is from an hypothetical global-patch-dir):

    package/foo/0001-fix-Makefile.patch
    /path/to/my/patches/foo/0001-fix-Makefile.patch

Quoted 1 line

In that case, we'd save onlt the second patch, overwriting the first.

onlt -> only

Indeed for legal-info it would be a serious issue if we silently overwrite a patch, resulting in _silently_ missing a patch.

As an alternative solution, Arnout proposed to error out if two patches have the same basename. Even though if would avoid the ugly prefixing, it would unnecessarily break builds without a need (as far as the development is concerned).

Quoted 5 lines

package/pkg-generic.mk +5 -0

+# Because patches may come from various places (bundled in Buildroot,
+# from one or more global-patch-dir), there might be collisions on the
+# basenames of those files.
+# We add a new numbering to each patch to ensure unicity of the filenames.
+	$$(Q)cpt=1; while read f; do \

package/pkg-generic.mk ifeq ($$($(2)_REDISTRIBUTE),YES) · line 837

Quoted 5 lines

package/pkg-generic.mk +5 -0

+		_c=$$$$(printf "%04d" $$$${cpt}); \
+		$$(call hardlink-copy,$$$${f},$$($(2)_REDIST_SOURCES_DIR),\
+			$$$${_c}-$$$${f##*/}) || exit 1; \
+		printf "%s\n" "$$$${_c}-$$$${f##*/}" >>$$($(2)_REDIST_SOURCES_DIR)/series || exit 1; \
+		: $$$$((cpt++)); \

I must admit this is not the most readable piece of code I have seen so far... ;)

I cannot do anything about the '$$$$'s, but at least I can suggest to rename the variables:

- cpt -> patch_num
- _c -> prefix_num

But it works fine, thus:

Tested-by Luca Ceresoli <[email protected]>

Some packages, like perl, download extra files that end up as part of the source that Buildroot builds. Up until now, those files were not saved in the legal-info output.

Add those files to the legal-info output.

The unfortunate side-effect is that we will also save the secondary archive for the external blackfin toolchains; however, we already do save the binary release of some external toolchains when they do not provide actual source archives.

This is inherently bad, as those are not source archives, but solving this is a bigger concern, for another series...

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Reviewed-by Luca Ceresoli <[email protected]>

Having a hash of the saved files can be interesting for the recipient to verify the integrity of the files.

We remove the warning file earlier, to exclude it from the hash list.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

It might be necessary to not even mention a package in the output of legal-info:

- virtual package have virtually nothing to save in the legal-info

     output;

- for Buildroot itself, host-gcc-initial and host-gcc-final are not real packages, they are just two different steps of the same package, gcc;

- for proprietary packages, it might not even be legal to even mention them, being under NDA or some other such restrictive conditions.

Introduce the new FOO_LEGAL_INGORE variable that a package can set to 'YES' (default to 'NO') to indicate that the package should be completely ignored from the legal-info output, in which case the package is not metionned in the maniufest, its source archive,

patches and license files are not saved into legal-info/ .

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

---
Changes v1 -> v2:
- introduce a new variable, instead of making _REDISTRIBUTE a tri-state (Thomas, Peter, Luca)

---
  docs/manual/adding-packages-generic.txt |  9 +++++++++
  package/pkg-generic.mk                  | 15 +++++++++++++++
  2 files changed, 24 insertions(+)

docs/manual/adding-packages-generic.txt +0 -0

index 1f9c7bb..1b07a14 100644
@@ -421,6 +421,15 @@ information is (assuming the package name is +libfoo+) :
    non-opensource packages: Buildroot will not save the source code for this

package/pkg-generic.mk +13 -0

index b59af9e..9c02372 100644
@@ -490,6 +490,14 @@ endif
 
  $(2)_REDISTRIBUTE		?= YES
 
+ifndef $(2)_LEGAL_IGNORE
+ ifdef $(3)_LEGAL_IGNORE
+  $(2)_LEGAL_IGNORE = $$($(3)_LEGAL_IGNORE)
+ endif
+endif
+
+$(2)_LEGAL_IGNORE		?= NO
+
  $(2)_REDIST_SOURCES_DIR = $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))/$$($(2)_RAWNAME_VERSION)
 
  # When a target package is a toolchain dependency set this variable to

@@ -762,7 +770,10 @@ $(2)_MANIFEST_LICENSE_FILES ?= not saved
  # We need to extract and patch a package to be able to retrieve its
  # license files (if any) and the list of patches applied to it (if
  # any).
+# But not if we want to ignore that package completely.
+ifneq ($$($(2)_LEGAL_IGNORE),YES)
  $(1)-legal-info: $(1)-patch
+endif
 
  # We only save the sources of packages we want to redistribute, that are
  # non-local, and non-overriden. So only store, in the manifest, the tarball

@@ -792,6 +803,8 @@ endif # actual sources != ""
 
  # legal-info: produce legally relevant info.
  $(1)-legal-info:
+ifneq ($$($(2)_LEGAL_IGNORE),YES)
+
  # Packages without a source are assumed to be part of Buildroot, skip them.
  	$$(foreach hook,$$($(2)_PRE_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))
  ifneq ($$(call qstrip,$$($(2)_SOURCE)),)

Quoted 2 lines · Yann E. MORIN

It might be necessary to not even mention a package in the output of legal-info:

Reviewed-by Luca Ceresoli <[email protected]>

Tested-by Luca Ceresoli <[email protected]>

Virtual packages really have nothing to save for legal-info, so we just ignore them.

Currently, packages are only extracted if they define _LICENSE_FILES. Follow-up patches will always extract and patch non-ignored packages, so we would potentially see spurious "extracting" and "patching" messages for virtual packages on the console, which would be a bit strange. Ignoring virtual packages will get us a cleaner log.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

---
Changes v1 -> v2:
- use the new _LEGAL_IGNORE variable (Thomas, Luca, Peter)

---
  docs/manual/adding-packages-virtual.txt | 3 +++
  package/pkg-virtual.mk                  | 2 ++
  2 files changed, 5 insertions(+)

docs/manual/adding-packages-virtual.txt +0 -0

index a5f17a2..11a497d 100644