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

Hello All!

This series brings improvements to the legal-info infrastructure, so that we provide the most complete and correct content in the output of legal-info.

Currently, our legal-info ouput is missing two types of files that might be important to have:
- patches
- extra downloads

This series is split in 6 consecutive parts, each depending on the previous ones:

- patches 1-2 fix the Linaro pre-built toolchains source and add hashes to all actual sources for pre-built toolchains

- patches 3-6 make sure legal-info will work in off-line mode;

- pathces 7-9 reorganise the legal-info directory structure to accomodate for the fact that more than one source archive/file may be saved for each package;

- patches 10-13 actually save the patches and extra downloads in the

    legal-info output;

- patch 14 adds a list of hashes for all files in the legal-info

    output;

- patches 15-16 add the possibility to ignore packages from the legal-info output; this is then used to ignore virtual packages.

Why save patches?
-----------------

So far, we've shuffled the patches under the rag, assuming the user would provide the Buildroot source tree with the compliance delivery, so that our bundled patches would automatically be included.

However, that's not enough, as not all patches may be in the Buildroot source tree. That's the case for at least two types of patches:
- patches that are downloaded,
- patches from a global patch directory.

In either case, those patches must be provided in the output of legal-info, because they are not part of Buildroot, so distributing Buidlroot would not be enough.

Patches that are referenced from Buidlroot (like patches retrieved at download time from a http://-or-such scheme to a publicly-reachable location) would probably be OK-ish, even if not to the letter of the compliance requirements.

That's not so much the case for patches from a global patch dir, since those would be completely ignored and usually unreachable from a recipient of the compliance delivery.

So we must save those two types of patches in the output of legal-info. Because it would be a bit silly to only save the non-bundled patches, we just save all of them, whether bundled in Buildroot, downloaded or from a global patch dir alike.

Note about saving patches
-------------------------

Buildroot can apply patches from at least three different locations:
- patches bundled in Buildroot itself,
- patches download either with FOO_PATCH of FOO_EXTRA_DOWNLOAD
- patches from one or more BR2_GLOBAL_PATCH_DIR

Since those patches are stored in three different locations, it is perfectly legit that two patches in two different locations have the same basename.

However, when saving the patches, the second to be saved would overwrite the previous one, and would thus cause troubles down the road:

- the second patch would be applied earlier due to it being referenced by the series file, and thus may not apply; even if it applies cleanly, it would still be listed a second time in the series, and thus would fail to apply that second time,

- the compliance distribution would not longer be compliant

Hence we have two options at our disposal:

- rename the patch files so no two patches have the same name; this is easily achieved by prefixing them with a monotonically-incremented counter, or

- detect that not two patches have the same basename and fail at apply-patch time if that is the case.

This series implements the first solution. The immediate drawback is that most patches will have two indexes, while the immediate advantage is that it accepts input patches with the same basename.

The alternative second solution, although tested and implemented (but not submitted) has an immediate drawback of breaking existing working setups with patches with the same basename, while there is no real advantage, except not double-numbering the patches for legal-info.

Why save extra downloads?
-------------------------

Some packages are using extra-downloads to complement the content of the main archive. That's the case for Perl, for which the cross-compilation "enabler" is downloaded as a secondary archive via extra downloads. The Blackfin external toolchains also use extra downloads to download a secondary archive with the sysroot.

Even though the Blackfin sysroot archive is not really a source, we still need to provide it along with the main archive, otherwise it's completely impossible to compile with just the "main" toolchain.

As for the Perl case, however, we're "only" downloading a buildsystem infrastructure (AFAIU), but without it, it is completely impossible to cross-compile Perl at all.

So, in both cases, we also need to save the extra downloads.

Changes v3 -> v4:
- add hashes for sources to more CodeSourcery pre-built toolchains (Arnout)
- fix hardlink-or-copy when the destination may already exist (Arnout, Luca)
- handle downloading actual sources with a stamp file (Luca)
- rephrase manual about ignoring packages (Luca)
- typoes (Luca)
- add first patch, to fix fetching sources for Linaro pre-built toolchains

Changes v2 -> v3:
- re-order variables in their own patch (Arnout)
- update legal-info header about the patches (Luca)
- add hashes for external toolchains sources (Luca, Arnout)
- misc and typoes (Arnout, Luca)
- enhance the hardlink-or-copy macro

Changes v1 -> v2:
- keep only the core legal-info patches, drop the gcc/binutils/gdb changes (they'll be reworked later, let's focus on the important and easier parts first)
- drop the tristate REDISTRIBUTE, introduce another boolean _LEGAL_IGNORE (Thomas, Peter, Luca)
- drop the post-legal-info Perl hook, it's no longer needed thanks to saving extra downloads (Thomas, Luca)
- compute the rawname-version tuple only once, instead of five times

    (Luca)

- reorder patches (Luca)
- slight commit log rephrasing and corrections (Luca)

Regards,
Yann E. MORIN.

The following changes since commit 9d05cb121a93aa0501f4c7369881d304063c84da:

python-smbus-cffi: new package (2016-02-03 22:34:20 +0100)

are available in the git repository at:

git://git.busybox.net/~ymorin/git/buildroot yem/legal-2

for you to fetch changes up to 758fb04b86bec833cd306ab5678d18504920bd54:

core/pkg-virtual: ignore from legal-info output (2016-02-03 22:49:40 +0100)

----------------------------------------------------------------
Yann E. MORIN (16):
toolchain/external: newer Linaro toolchains do not provide source code toolchain/external: add hashes for actual sources core/pkg-utils: add macro to hardlink-or-copy core/legal-info: use the macro to install source archives core/pkg-generic: reorder variables definitions for legal-info core/legal-info: ensure legal-info works in off-line mode core/pkg-generic: add variable to store the package rawname-version core/legal-info: install source archives in their own sub-dir core/legal-info: add package version to license directory core/apply-patches: store full path of applied patches core/legal-info: also save patches
core/legal-info: renumber saved patches
core/legal-info: also save extra downloads core/legal-info: generate a hash of all saved files core/legal-info: allow ignoring packages from the legal-info core/pkg-virtual: ignore from legal-info output

 Makefile                                           |  8 +-
 docs/manual/adding-packages-generic.txt            |  9 +++
 docs/manual/adding-packages-virtual.txt            |  5 ++
 package/pkg-generic.mk                             | 94 +++++++++++++++++-----
 package/pkg-utils.mk                               | 29 +++++++
 package/pkg-virtual.mk                             |  2 +
 support/legal-info/README.header                   |  9 ++-
 support/scripts/apply-patches.sh                   | 11 ++-
 .../toolchain-external/toolchain-external.hash     | 13 +++
 toolchain/toolchain-external/toolchain-external.mk |  6 +-
 10 files changed, 153 insertions(+), 33 deletions(-)

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Currently, we have a pattern-matching that automatically derives the the source tarball filename from the binary tarball filename.

However, the latest Linaro toolchains no longer follow that scheme (and do not even readily provide the sources...).

Remove the generic pattern-matching, and explicitly set the source tarball name for those toolchains that do have a source tarball readily available.

---
 toolchain/toolchain-external/toolchain-external.mk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index 461c603..6c3022a 100644
@@ -297,6 +297,7 @@ else ifeq ($(BR2_TOOLCHAIN_EXTERNAL_LINARO_ARM),y)
 ifeq ($(HOSTARCH),x86)
 TOOLCHAIN_EXTERNAL_SITE = http://releases.linaro.org/14.09/components/toolchain/binaries
 TOOLCHAIN_EXTERNAL_SOURCE = gcc-linaro-arm-linux-gnueabihf-4.9-2014.09_linux.tar.xz
+TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_TARBALL = gcc-linaro-arm-linux-gnueabihf-4.9-2014.09_src.tar.bz2
 TOOLCHAIN_EXTERNAL_POST_INSTALL_STAGING_HOOKS += TOOLCHAIN_EXTERNAL_LINARO_ARMHF_SYMLINK
 else
 TOOLCHAIN_EXTERNAL_SITE = http://releases.linaro.org/components/toolchain/binaries/5.2-2015.11-2/arm-linux-gnueabihf

@@ -306,6 +307,7 @@ else ifeq ($(BR2_TOOLCHAIN_EXTERNAL_LINARO_ARMEB),y)
 ifeq ($(HOSTARCH),x86)
 TOOLCHAIN_EXTERNAL_SITE = http://releases.linaro.org/14.09/components/toolchain/binaries
 TOOLCHAIN_EXTERNAL_SOURCE = gcc-linaro-armeb-linux-gnueabihf-4.9-2014.09_linux.tar.xz
+TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_TARBALL = gcc-linaro-armeb-linux-gnueabihf-4.9-2014.09_src.tar.bz2
 TOOLCHAIN_EXTERNAL_POST_INSTALL_STAGING_HOOKS += TOOLCHAIN_EXTERNAL_LINARO_ARMEBHF_SYMLINK
 else
 TOOLCHAIN_EXTERNAL_SITE = http://releases.linaro.org/components/toolchain/binaries/5.2-2015.11-2/armeb-linux-gnueabihf

@@ -350,6 +352,7 @@ else ifeq ($(BR2_TOOLCHAIN_EXTERNAL_LINARO_AARCH64),y)
 ifeq ($(HOSTARCH),x86)
 TOOLCHAIN_EXTERNAL_SITE = http://releases.linaro.org/14.09/components/toolchain/binaries
 TOOLCHAIN_EXTERNAL_SOURCE = gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux.tar.xz
+TOOLCHAIN_EXTERNAL_ACTUAL_SOURCE_TARBALL = gcc-linaro-aarch64-linux-gnu-4.9-2014.09_src.tar.bz2
 TOOLCHAIN_EXTERNAL_POST_INSTALL_STAGING_HOOKS += TOOLCHAIN_EXTERNAL_LINARO_AARCH64_SYMLINK
 else
 TOOLCHAIN_EXTERNAL_SITE = http://releases.linaro.org/components/toolchain/binaries/5.2-2015.11-2/aarch64-linux-gnu

@@ -406,9 +409,6 @@ endif
 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

1.9.1

Dear Yann E. MORIN,

Applied, thanks.

Thomas

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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.

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

index df6d7e9..944c98f 100644
@@ -12,30 +12,43 @@ 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
 # NiosII
 sha256 cc47745dc1264fcb8fb98fb1315ab772ab98691396021c455229b58abaf887f5  sourceryg++-2015.11-27-nios2-linux-gnu-i686-pc-linux-gnu.tar.bz2
+sha256 8b5710e41685a316d577b503c068c69ba1eaf228fe0a056cac36945323a0dd23  sourceryg++-2015.11-27-nios2-linux-gnu.src.tar.bz2
 # PowerPC
 sha256 525e1f53abbf65c2974ae9af762c45bb38520fe5fc50e968a23fe6a18e9eec04  freescale-2011.03-38-powerpc-linux-gnu-i686-pc-linux-gnu.tar.bz2
+sha256 fe45a1d2725e1b9f0ec7b8041c923e01f461da01b298caad537854ec1c61e211  freescale-2011.03-38-powerpc-linux-gnu.src.tar.bz2
 sha256 d6c94587d546197836e7e1a6909f6aabfa5879e91f501ab03088a6887cc242fc  mentor-2012.03-71-powerpc-mentor-linux-gnu-i686-pc-linux-gnu.tar.bz2
+sha256 4964938da0108a9a5bcc295b0f504a3804021e6e6e596509ee085d9143711b50  mentor-2012.03-71-powerpc-mentor-linux-gnu.src.tar.bz2
 # SuperH
 sha256 59d6766fde244931aa52db01433d5acd051998762a931121c5fc109536a1a802  renesas-2012.09-61-sh-linux-gnu-i686-pc-linux-gnu.tar.bz2
+sha256 e2e58c10e52395d5d35157e35f85233f713c6f9223a652dfc56194cfd2eed004  renesas-2012.09-61-sh-linux-gnu.src.tar.bz2
 # x86
 sha256 ea804cf02014369da52abc4f64e91e96bde2dd2230aca96109459013d4545458  ia32-2012.09-62-i686-pc-linux-gnu-i386-linux.tar.bz2
+sha256 1a9519e415a1e6892c760bf21f7e98f3a633a9d1c5bb8781a96d338e4dd62717  ia32-2012.09-62-i686-pc-linux-gnu.src.tar.bz2
 # AMD64
 sha256 cb4d071db8aefb8005fe72824b96568d93a50f5acd85bacf505a34fe2f265f70  amd-2015.11-36-x86_64-amd-linux-gnu-i686-pc-linux-gnu.tar.bz2
+sha256 a4de11669cddd4594f228db7bd38b0fed3c8692d1bac021739390b4c766bbd4f  amd-2015.11-36-x86_64-amd-linux-gnu.src.tar.bz2
 # Aarch64
 sha256 8ea78c5988b2bb507534f1ad46aa46659f66b39d55f2fc40e163a90b4195e70f  aarch64-2014.05-30-aarch64-linux-gnu-i686-pc-linux-gnu.tar.bz2
+sha256 fef1ba94b43f2aefe5634815745c1cb5442a12d12d98e653601392cb65517b7f  aarch64-2014.05-30-aarch64-linux-gnu.src.tar.bz2
 
 # ARM toolchains from Texas Instrument's Arago project
+# There is one source file that covers both binary distributions.
 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 ff2d231749e59968cb5e7032b4f4e4ae82ff9f11c23967863e627a6c59cb3bc0  gcc-linaro-5.2-2015.11-2-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 e3df0c31d0bd8d1f6d45d585203c0f601e50b1ff7225da1423a8ca36e8caf58f  gcc-linaro-5.2-2015.11-2-x86_64_armeb-linux-gnueabihf.tar.xz
 sha256 3954f496ab01de67241109e82abfaa9b7625fdab4f05e79e7902e9814a07b832  gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux.tar.xz
+sha256 3954f496ab01de67241109e82abfaa9b7625fdab4f05e79e7902e9814a07b832  gcc-linaro-aarch64-linux-gnu-4.9-2014.09_src.tar.bz2
 sha256 b318a1837a54146b0120a13852386576e38355513b4e2cd5e2125f9c26913777  gcc-linaro-5.2-2015.11-2-x86_64_aarch64-linux-gnu.tar.xz
 
 # Codescape toolchains from Imagination Technologies

1.9.1

This macro will try to copy a source file into a destination directory, by first attempting to 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.

---
Changes v3 -> v4:
- forcibly remove destination file first (Arnout, Luca)
- typoes (Luca)
- drop trailing slash in destination directory name

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 | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

index 44bd2c9..ed05964 100644
@@ -113,6 +113,35 @@ $$(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 we 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.
+#
+# Note-2: we do not introduce any intermediate shell variables because we can't
+# guarantee the number of expansions (as $-signs) we'll need.
+################################################################################
+define hardlink-copy
+	{ mkdir -p $(2) && \
+	  rm -f        $(strip $(2))/$(if $(3),$(strip $(3)),$(notdir $(1))) && \
+	  { ln -f $(1) $(strip $(2))/$(if $(3),$(strip $(3)),$(notdir $(1))) 2>/dev/null || \
+	    cp -f $(1) $(strip $(2))/$(if $(3),$(strip $(3)),$(notdir $(1))); \
+	  } \
+	}
+endef
+
 #
 # legal-info helper functions
 #

1.9.1

Hi Yann,

$ cat defconfig
BR2_arm=y
BR2_TOOLCHAIN_EXTERNAL=y
BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0"
BR2_SYSTEM_DHCP="eth0"
BR2_TARGET_ROOTFS_EXT2=y
# BR2_TARGET_ROOTFS_TAR is not set
$ make clean
[...]
$ make legal-info
[...]

Applying 0001-networking-libiproute-use-linux-if_packet.h-instead-.patch using patch:
patching file networking/libiproute/iplink.c

Applying 0002-unzip.patch using patch:
patching file archival/libarchive/decompress_gunzip.c patching file testsuite/unzip.tests

Applying 0003-g-unzip-fix-recent-breakage.patch using patch:
patching file archival/libarchive/decompress_gunzip.c patching file testsuite/unzip.tests

Applying 0004-truncate-open-mode.patch using patch:
patching file coreutils/truncate.c

Applying 0008-Makefile.flags-strip-non-l-arguments-returned-by-pkg.patch
using patch:
patching file Makefile.flags
Hunk #1 succeeded at 148 (offset 7 lines).
cp: cannot create regular file '/home/murray/devel/buildroot/output/legal-info/sources/busybox-1.24.1//home/murray/devel/buildroot/package/busybox/0001-networking-libiproute-use-linux-if_packet.h-instead-.patch':
No such file or directory
make[1]: *** [busybox-legal-info] Error 1
make: *** [_all] Error 2
$

Notice the pwd/package/busybox/ prefix has not been removed from the name of the destination file. The issue materializes in hardlink-copy, so I instrumented it this way:

define hardlink-copy

	{ mkdir -p $(2) && \
+	  echo "__1_$(1)_" && \
+	  echo "_n1_$(notdir $(1))_" && \
+	  echo "__2_$(2)_" && \
+	  echo "__3_$(3)_" && \
	  rm -f $(strip $(2))/$(if $(3),$(strip $(3)),$(notdir $(1))) && \
	  { ln -f $(1) $(strip $(2))/$(if $(3),$(strip $(3)),$(notdir $(1)))
2>/dev/null || \
	  cp -f $(1) $(strip $(2))/$(if $(3),$(strip $(3)),$(notdir $(1))); \

And I get the following debug messages:

__1_ /home/murray/src/busybox-1.24.1.tar.bz2_
_n1_busybox-1.24.1.tar.bz2_
__2_ /home/murray/devel/buildroot/output/legal-info/sources/busybox-1.24.1_
__3__ __1_/home/murray/devel/buildroot/package/busybox/0001-networking-libiproute-use-linux-if_packet.h-instead-.patch_ _n1_/home/murray/devel/buildroot/package/busybox/0001-networking-libiproute-use-linux-if_packet.h-instead-.patch_ __2_/home/murray/devel/buildroot/output/legal-info/sources/busybox-1.24.1_
__3__

The first 4 lines are output when saving the tarball. See the difference between the line prefixed with __1_ and the one prefixed with _n1_. Clearly $(notdir) has removed the directory prefix, which is correct.

On the next 4 lines we have the failing file, which is a patch. Here it looks like $(notdir) has done nothing.

This explains why the issue disappears applying up to patch 12: the patch renumbering added by patch 12 uses the third parameter to hardlink-copy, which bypasses the call to $(notdir).

I noticed $1 and $2 have a leading space in the first call, but it is not related to the problem. I tried adding/removing the space, without any change. This is correct according to the documentation for $(notdir): "...everything through the last slash is removed".

This is the best I could understand at the moment... :-(

Luca

Luca, All,

On 2016-02-12 23:28 +0100, Luca Ceresoli spake thusly:

[--SNIP--]

Damned, that's right. It's broke... :-/

[--SNIP--]

Thanks for the detailed analysis! :-)
I'll investigate this further.

Regards,
Yann E. MORIN.

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

When in doubt, add a dollar.

Regards,
Arnout

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:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

Luca, All,

On 2016-02-12 23:46 +0100, Yann E. MORIN spake thusly:

And now I looked at the code, it is pretty obvious what's going on.

hardlink-or-copy exclusively uses make functions, like $(notdir), on its parameters, but it is called with parameters that are set from the shell.

I.e. we test the parameters of hardlink-or-copy in make, while they are not yet valid, as they will only be valid by the time we actually execute the shell fragment, by which time it is too late.

That's what happens (removing a $-level for the sake of simplicity):

while read f; do
$(call hardlink-copy,$${f},$($(2)_REDIST_SOURCES_DIR)) || exit 1;
done </path/to/.applied_patches_list

So@the time we evaluate hardlink-or-copy, here are the values of the parameters:

    $(1) == ${f}
    $(2) == /path/to/redist/dir
    $(3) ==         (empty)

Since $(3) is empty, we'd fall in the else-clause of the $(if...), which evaluates to $(notdir ${f}) which is just plain ${f}.

Bummer.

It happens to work when we renumber the patches, because the third argument is not empty, being a shell variable evaluation construct.

I'll try to fix that.

Thank you! ;-)

Regards,
Yann E. MORIN.

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

.. and use $(Q) instead of a hard-coded @.

---
Change v2 -> v3;
- comment the @ -> $(Q) change (Arnout)

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

index 1e024d3..893ee07 100644
@@ -804,9 +804,10 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 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)_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)))
+# Copy the source tarball
+	$$(Q)$$(call hardlink-copy,\
+		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
+		     $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))))
 endif # redistribute
 
 endif # other packages

1.9.1

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.

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

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

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.

1.9.1

It's identical to v4, so:

Luca

TBC: explain the part about the stamp file

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

---
Changes v3 -> v4:
- handle it with a stamp file (Luca)

Changes v2 -> v3:
- re-order the PHONY targets (Arnout)
- don't reorder setting _ACTUAL_SOURCE/_SITE, it's done in its own patch now (Arnout)
- adapt the commit log accordingly (Arnout)
- typo

Changes v1 -> v2:
- drop the 'redistribute == ignore', it does not exist yet

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

index 5958029..95dbffd 100644
@@ -123,6 +123,12 @@ $(BUILD_DIR)/%/.stamp_downloaded:
 	$(Q)mkdir -p $(@D)
 	$(Q)touch $@
 
+# Retrieve actual source archive, e.g. for prebuilt external toolchains
+$(BUILD_DIR)/%/.stamp_actual_downloaded:
+	$(call DOWNLOAD,$($(PKG)_ACTUAL_SOURCE_SITE)/$($(PKG)_ACTUAL_SOURCE_TARBALL)); \
+	$(Q)mkdir -p $(@D)
+	$(Q)touch $@
+
 # Unpack the archive
 $(BUILD_DIR)/%/.stamp_extracted:
 	@$(call step_start,extract)

@@ -527,6 +533,7 @@ $(2)_TARGET_RSYNC =	        $$($(2)_DIR)/.stamp_rsynced
 $(2)_TARGET_PATCH =		$$($(2)_DIR)/.stamp_patched
 $(2)_TARGET_EXTRACT =		$$($(2)_DIR)/.stamp_extracted
 $(2)_TARGET_SOURCE =		$$($(2)_DIR)/.stamp_downloaded
+$(2)_TARGET_ACTUAL_SOURCE =	$$($(2)_DIR)/.stamp_actual_downloaded
 $(2)_TARGET_DIRCLEAN =		$$($(2)_DIR)/.stamp_dircleaned
 
 # default extract command

@@ -634,6 +641,17 @@ $(1)-depends:		$$($(2)_FINAL_DEPENDENCIES)
 
 $(1)-source:		$$($(2)_TARGET_SOURCE)
 
+$(1)-all-source:	$(1)-legal-source
+$(1)-legal-info:	$(1)-legal-source
+$(1)-legal-source:	$(1)-source
+
+# Only downlaod the actual source if it differs from the 'main' archive
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),)
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
+$(1)-legal-source:	$$($(2)_TARGET_ACTUAL_SOURCE)
+endif # actual sources != sources
+endif # actual sources != ""
+
 $(1)-source-check:
 	$$(foreach p,$$($(2)_ALL_DOWNLOADS),$$(call SOURCE_CHECK,$$(p))$$(sep))
 

@@ -659,6 +677,7 @@ $(1)-extract:		$(1)-rsync
 $(1)-rsync:		$$($(2)_TARGET_RSYNC)
 
 $(1)-source:
+$(1)-legal-source:
 
 $(1)-source-check:
 	test -d $$($(2)_OVERRIDE_SRCDIR)

@@ -731,6 +750,8 @@ $$($(2)_TARGET_PATCH):			PKGDIR=$(pkgdir)
 $$($(2)_TARGET_EXTRACT):		PKG=$(2)
 $$($(2)_TARGET_SOURCE):			PKG=$(2)
 $$($(2)_TARGET_SOURCE):			PKGDIR=$(pkgdir)
+$$($(2)_TARGET_ACTUAL_SOURCE):		PKG=$(2)
+$$($(2)_TARGET_ACTUAL_SOURCE):		PKGDIR=$(pkgdir)
 $$($(2)_TARGET_DIRCLEAN):		PKG=$(2)
 
 # Compute the name of the Kconfig option that correspond to the

@@ -801,9 +822,6 @@ else
 # Other packages
 
 ifeq ($$($(2)_REDISTRIBUTE),YES)
-ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
-	$$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))
-endif
 # Copy the source tarball
 	$$(Q)$$(call hardlink-copy,\
 		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\

@@ -898,6 +916,7 @@ endif
 	$(1)-install-staging \
 	$(1)-install-target \
 	$(1)-legal-info \
+	$(1)-legal-source \
 	$(1)-patch \
 	$(1)-rebuild \
 	$(1)-reconfigure \

1.9.1

All,

On 2016-02-03 23:21 +0100, Yann E. MORIN spake thusly:

Damn, I borked my rebase and lost that part of the commit log... :-(

I'll rewrite it tomorrow (too late now).

Sorry for the mess... :-/

Regards,
Yann E. MORIN.

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Hi Yann,

With the stamp file the patch is a lot cleaner and coherent with the rest of the infra. Thanks.

I'll add my Reviewed-by tag as soon as you fix the commit message. But in the meantime:

Luca

Introduce a new per-package variable to store the 'rawname-version' tuple, instead of computing it every time we need it.

Currently, it's only a single location, but follow-up patches will introduce more use of it.

---
 package/pkg-generic.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

index 95dbffd..ac8cc26 100644
@@ -393,6 +393,7 @@ else
  $(2)_DL_VERSION := $$(strip $$($(2)_VERSION))
 endif
 $(2)_VERSION := $$(call sanitize,$$($(2)_DL_VERSION))
+$(2)_RAWNAME_VERSION = $$($(2)_RAWNAME)-$$($(2)_VERSION)
 
 ifdef $(3)_OVERRIDE_SRCDIR
   $(2)_OVERRIDE_SRCDIR ?= $$($(3)_OVERRIDE_SRCDIR)

@@ -429,7 +430,7 @@ ifndef $(2)_SOURCE
  ifdef $(3)_SOURCE
   $(2)_SOURCE = $$($(3)_SOURCE)
  else
-  $(2)_SOURCE			?= $$($(2)_RAWNAME)-$$($(2)_VERSION).tar.gz
+  $(2)_SOURCE			?= $$($(2)_RAWNAME_VERSION).tar.gz
  endif
 endif
 

1.9.1

Currently, we put all source archives side-by-side in the same directory.

Since we're about to also save individual patches that were applied on those sources, we don't want to make that directory a complete mess of unassorted files.

So, we install each source archive in its own sub-directory, where we'll later store the patches too. Store that location in a variable, so it can be re-used later on (to install patches in a future commit).

---
Changes v1 -> v2:
- perl no longer has a post-legal-info hook (Thomas, Luca)

---
 package/pkg-generic.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index ac8cc26..906cf41 100644
@@ -496,6 +496,8 @@ endif
 
 $(2)_REDISTRIBUTE		?= YES
 
+$(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
 # 'NO' so the 'toolchain' dependency is not added to prevent a circular
 # dependency

@@ -826,7 +828,7 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 # Copy the source tarball
 	$$(Q)$$(call hardlink-copy,\
 		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
-		     $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))))
+		     $$($(2)_REDIST_SOURCES_DIR))
 endif # redistribute
 
 endif # other packages

1.9.1

Now that we save the source archives in a directory named after the package and its version, do the same for the license files, for consistency.

It has a not-so-bad side-effect of also saving the version string in the all-licenses list.

The only (small) side-effect, is that the warnings about undefined _LICENSE_FILES now contains the version string, too. That's unavoidable, since that's what is stored in the legal report.

---
Changes v1 -> v2:
- s/drawback/side-effect/ (Luca)

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

index 906cf41..432a153 100644
@@ -808,10 +808,10 @@ ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
 # is that the license still applies to the files distributed as part
 # of the rootfs, even if the sources are not themselves redistributed.
 ifeq ($$(call qstrip,$$($(2)_LICENSE_FILES)),)
-	@$$(call legal-license-nofiles,$$($(2)_RAWNAME),$$(call UPPERCASE,$(4)))
-	@$$(call legal-warning-pkg,$$($(2)_RAWNAME),cannot save license ($(2)_LICENSE_FILES not defined))
+	@$$(call legal-license-nofiles,$$($(2)_RAWNAME_VERSION),$$(call UPPERCASE,$(4)))
+	@$$(call legal-warning-pkg,$$($(2)_RAWNAME_VERSION),cannot save license ($(2)_LICENSE_FILES not defined))
 else
-	@$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME),$$(F),$$($(2)_DIR)/$$(F),$$(call UPPERCASE,$(4)))$$(sep))
+	@$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME_VERSION),$$(F),$$($(2)_DIR)/$$(F),$$(call UPPERCASE,$(4)))$$(sep))
 endif # license files
 
 ifeq ($$($(2)_SITE_METHOD),local)

1.9.1

Currently, we only store the filename of the applied patches.

However, we are soon to want to install those patches in the legal-info directory, so we'll have to know where those patches come from.

Instead of duplicating the logic to find the patches (bundled, downloaded, from a global patch dir...), just store the full path to each of those patches so we can retrieve them more easily later on.

Also always create the list-file, even if empty, so that we need not test for its existence before reading it.

---
Changes v1 -> v2:
- do not duplicate '/' in paths (Luca)

---
 support/scripts/apply-patches.sh | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

index 201278d..20a1552 100755
@@ -63,8 +63,12 @@ find ${builddir}/ '(' -name '*.rej' -o -name '.*.rej' ')' -print0 | \
     xargs -0 -r rm -f
 
 function apply_patch {
-    path=$1
-    patch=$2
+    path="${1%%/}"
+    patch="${2}"
+    case "${path}" in
+        /*) ;;
+        *)  path="$(pwd)/${path}";;
+    esac
     if [ "$3" ]; then
         type="series"; uncomp="cat"
     else

@@ -99,7 +103,7 @@ function apply_patch {
         echo "Error: missing patch file ${path}/$patch"
         exit 1
     fi
-    echo $patch >> ${builddir}/.applied_patches_list
+    echo "${path}/${patch}" >> ${builddir}/.applied_patches_list
     ${uncomp} "${path}/$patch" | patch -g0 -p1 -E -d "${builddir}" -t -N $silent
     if [ $? != 0 ] ; then
         echo "Patch failed!  Please fix ${patch}!"

@@ -141,6 +145,7 @@ function scan_patchdir {
     fi
 }
 
+touch ${builddir}/.applied_patches_list
 scan_patchdir "$patchdir" "$patchpattern"
 
 # Check for rejects...

1.9.1

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.

---
Changes v3 -> v4:
- typo (Luca)

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(-)

index 432a153..055dbdb 100644
@@ -776,12 +776,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

@@ -829,6 +827,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

index d07c45d..418de14 100644
@@ -14,10 +14,11 @@ This material is composed of the following items.
    compiled programs.
    Note: this may have not been saved due to technical limitations, you may
    need to collect it manually.
- * The source code for all packages; this has been saved in the sources/
-   subdirectory (except for the non-redistributable packages, which have not
-   been saved); patches applied to some packages by Buildroot are included in
-   the Buildroot sources and were not duplicated in the sources/ subdirectory.
+ * The original source code for all packages; this has been saved in the
+   sources/ subdirectory (except for the non-redistributable packages, which
+   have not been saved). Patches that were applied are also saved, along
+   with a file named 'series' that lists the patches in the order they were
+   applied.
  * A manifest file listing the configured packages and related information.
  * The license text of the packages; they have been saved in the licenses/
    subdirectory.

1.9.1

Dear Yann,

I have a late remark, see below.

Luca

Luca, All,

On 2016-03-05 00:45 +0100, Luca Ceresoli spake thusly:

Eh! ;-)

[--SNIP--]

Well, we do need $(1)-patch, because a patch may change the licensing info (i.e. add a new license, add a LICENSE file or some such), so depending on -source is not enough.

Hmmm... I'm not sure I am following you here... Are yoiu saying that we need not depend on -source because we already depend on -patch, above?

Eh! ;-)

Regards,
Yann E. MORIN.

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

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

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

Note: an alternative solution was suggested by Luca (and Arnout?), which would be to check that no two patches have the same basename or bail-out otherwise. I choose to keep the renumbering, because it follows the path of least resistance (i.e. it does not break existing setups).

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

index 055dbdb..fc6ee16 100644
@@ -828,9 +828,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; \
+# 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)patch_num=1; while read f; do \
+		prefix_num=$$$$(printf "%04d" $$$${patch_num}); \
+		$$(call hardlink-copy,$$$${f},$$($(2)_REDIST_SOURCES_DIR),\
+			$$$${prefix_num}-$$$${f##*/}) || exit 1; \
+		printf "%s\n" "$$$${prefix_num}-$$$${f##*/}" >>$$($(2)_REDIST_SOURCES_DIR)/series || exit 1; \
+		: $$$$((patch_num++)); \
 	done <$$($(2)_DIR)/.applied_patches_list
 endif # redistribute
 

1.9.1

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

---
Changes v2 -> v3:
- typo (Luca)
- incorporate the post-commit log message (the part about the side-effect) into the commit log itself, it makes sense to not forget about that

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

index fc6ee16..ec93416 100644
@@ -823,10 +823,12 @@ else
 # Other packages
 
 ifeq ($$($(2)_REDISTRIBUTE),YES)
-# Copy the source tarball
-	$$(Q)$$(call hardlink-copy,\
-		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
-		     $$($(2)_REDIST_SOURCES_DIR))
+# Copy the source tarball and any extra downloads, but not
+# patches, as they are handled specially afterwards.
+	$$(foreach e,$$($(2)_ACTUAL_SOURCE_TARBALL) $$(notdir $$($(2)_EXTRA_DOWNLOADS)),\
+			$$(Q)$$(call hardlink-copy,\
+				$$(DL_DIR)/$$(e),\
+				$$($(2)_REDIST_SOURCES_DIR))$$(sep))
 # Copy patches and generate the series file
 # Because patches may come from various places (bundled in Buildroot,
 # from one or more global-patch-dir), there might be collisions on the

1.9.1

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.

---
Changes v1 -> v2:
- simplify getting rid of the ..../legal-info/ prefix (Luca)
- always sort with the C locale

---
 Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

index 68259fd..1acc2b2 100644
@@ -696,8 +696,14 @@ legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p
 		cat support/legal-info/README.warnings-header \
 			$(LEGAL_WARNINGS) >>$(LEGAL_REPORT); \
 		cat $(LEGAL_WARNINGS); fi
-	@echo "Legal info produced in $(LEGAL_INFO_DIR)"
 	@rm -f $(LEGAL_WARNINGS)
+	@(cd $(LEGAL_INFO_DIR); \
+	  find * -type f -exec sha256sum {} + \
+	  |LC_ALL=C sort -k2 \
+	  >.legal-info.sha256; \
+	  mv .legal-info.sha256 legal-info.sha256 \
+	)
+	@echo "Legal info produced in $(LEGAL_INFO_DIR)"
 
 show-targets:
 	@echo $(PACKAGES) $(TARGETS_ROOTFS)

1.9.1

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

- virtual packages 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 mentioned in the maniufest, its source archive, patches and license files are not saved into legal-info/ .

---
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(+)

index b7817e3..26a7b6c 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 when collecting the +legal-info+.
 
+* +LIBFOO_LEGAL_IGNORE+ can be set to +YES+ or +NO+ (the default) to indicate
+  that this package should be completely ignored when saving the licensing
+  information. If set to +YES+, then the package is not listed in the manifest,
+  its source archive and its license files are not saved. You probably do not
+  want to set it to +YES+, unless under very specific conditions (e.g. when
+  you use the `legal-info/` output directory as-is to provide a compliance
+  delivery, and do not want your proprietary, non-redistributable packages to
+  even be mentioned in the manifest).
+
 * +LIBFOO_FLAT_STACKSIZE+ defines the stack size of an application built into
   the FLAT binary format. The application stack size on the NOMMU architecture
   processors can't be enlarged at run time. The default stack size for the

index ec93416..9acc869 100644
@@ -496,6 +496,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

@@ -779,7 +787,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

@@ -795,6 +806,8 @@ endif
 
 # 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)),)

@@ -848,6 +861,8 @@ endif # other packages
 endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
 	$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))
 
+endif # $(2)_LEGAL_IGNORE != YES
+
 # add package to the general list of targets if requested by the buildroot
 # configuration
 ifeq ($$($$($(2)_KCONFIG_VAR)),y)

1.9.1

Virtual packages really have nothing to save for legal-info, so we just ignore them, to avoid spurious "extracting" and "patching" messages while running legal-info.

---
Changes v3 -> v4:
- rephrase the .Note (Luca)
- drop now-incorrect blurb from the commit log (Luca)

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

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

index a5f17a2..3857927 100644
@@ -56,6 +56,11 @@ The +.mk+ for the virtual package should just evaluate the +virtual-package+ mac
 The ability to have target and host packages is also available, with the
 +host-virtual-package+ macro.
 
+.Note:
+The +virtual-package+ infrastructure automatically marks virtual packages to be
+excluded from the +legal-info+ output (by internally setting +FOO_LEGAL_IGNORE+
+to 'YES').
+
 ==== Provider's +Config.in+ file
 
 When adding a package as a provider, only the +Config.in+ file requires some

index 244c1d0..445f09a 100644
@@ -59,6 +59,8 @@ endif
 # Add dependency against the provider
 $(2)_DEPENDENCIES += $$(call qstrip,$$(BR2_PACKAGE_PROVIDES_$(2)))
 
+$(2)_LEGAL_IGNORE = YES
+
 # Call the generic package infrastructure to generate the necessary
 # make targets
 $(call inner-generic-package,$(1),$(2),$(3),$(4))

1.9.1

Hi Yann,

Luca