legal-info: extract packages to get license files (branch yem/legal)

From: "Yann E. MORIN" <[email protected]>

Hello All!

Here is a series that fixes the legal-info infra:
- extracts sources before copying license files
- saves license files even for no-redistribute packages
- cleans up to account for the changes (comment and helper rename)

Changes v3 -> v4:
- save licens files for no-redistribute packages (Luca)
- download sources before copying to legal-info/sources/
- rename a legal-info helper

Changes v2 -> v3:
- fix manifest for no-redistribute, or overriden, or local packages

Regards,
Yann E. MORIN.

The following changes since commit 3749143418a2a735ceaed63667a8b3ef66bba9e5:

package: drop <PKG>_VERSION_MINOR variable (2014-03-17 23:50:08 +0100)

are available in the git repository at:

git://gitorious.org/buildroot/buildroot.git yem/legal

for you to fetch changes up to ff3371fa4250df5d073cbcb772d9e5563cec4348:

legal-info: rename legal-warning-pkg-savednothing helper (2014-03-17 23:51:27 +0100)

----------------------------------------------------------------
Yann E. MORIN (4):
legal-info: extract even no-redistribute packages
legal-info: save license files even for no-redistribute packages
legal-info: add a comment about what packages we save the tarballs of
legal-info: rename legal-warning-pkg-savednothing helper

 package/pkg-generic.mk | 45 ++++++++++++++++++++++++++++++---------------
 package/pkg-utils.mk   |  4 ++--
 2 files changed, 32 insertions(+), 17 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.  |
'------------------------------^-------^------------------^--------------------'

From: "Yann E. MORIN" <[email protected]>

Currently, if a package is marked _REDISTRIBUTE = NO, then legal-info will not try to extract it first.

If that package also declares some _LICENSE_FILES, legal-info fails if it is the only action we're trying to run:

    $ cat defconfig
    BR2_INIT_NONE=y
    BR2_PACKAGE_LIBFSLCODEC=y
    $ make BR2_DEFCONFIG=$(pwd)/defconfig defconfig
    $ make libfslcodec-legal-info
    /bin/sh: /home/ymorin/dev/buildroot/O/legal-info/licenses.txt: No such file or directory
    make[1]: *** [libfslcodec-legal-info] Error 1

Fix this by always having legal-info extract the archives if one or more _LICENSE_FILES are specified.

We do this for all types of packages: overriden, local or 'normal' remote packages. Even though we do not save the sources for the overriden or local packages, we need to save their licensing info, so we need to extract them.

This implies that we now need to explicitly add PKG-source as a dependency of legal-info for pacakges we want to save (ie. redistributable, non-local and non-overriden packages).

---
Changes v3 -> v4:
- legal-info needs to depend on PKG-source when it needs to save a package's tarball

Changes v2 -> v3:
- don't include source URL of no-redistribute, or overriden, or local packages in the manifest

Changes v1 -> v2:
- this is not fixing the autobuilders failure it was written to fix so remove the references to such build failures (Thomas P)
- also extract overriden and local packages (Fabio)

---
 package/pkg-generic.mk | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

index 339c3eb..3d8f0da 100644
@@ -555,11 +555,18 @@ $(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),)
+$(1)-legal-info: $(1)-extract
+endif
+
 ifeq ($$($(2)_REDISTRIBUTE),YES)
 ifneq ($$($(2)_SITE_METHOD),local)
 ifneq ($$($(2)_SITE_METHOD),override)
-# Packages that have a tarball need it downloaded and extracted beforehand
-$(1)-legal-info: $(1)-extract $(REDIST_SOURCES_DIR_$(call UPPERCASE,$(4)))
+# We need to download the package sources if we are to save it
+$(1)-legal-info: $(1)-source $(REDIST_SOURCES_DIR_$(call UPPERCASE,$(4)))
 $(2)_MANIFEST_TARBALL = $$($(2)_SOURCE)
 endif
 endif

1.8.3.2

Hi Yann,

        or: package archive <-> save it     (singular)

Luca

Luca, All,

On 2014-03-18 18:36 +0100, Luca Ceresoli spake thusly:

[--SNIP--]

Yep, thanks! :-)

Yep, no problem. :-)

I really depends. My opinion is that, on complex changes (like the switch to the new site), it is important to get the new functionality in place, and improve gradually, or it would take like forever before it is ready. And since the new website was entirely replacing the old one (rather than be a gradual upgrade), it would have never been possible to update the new website with the incremental changes going into the old one. So, at one point in time we had to switch for good.

In this case, it's not the same: we are gradually improving existing code, so I believe the way you did is OK. If you hadn't caught the typo, it would have been possible to send a follow-up patch fixing it, but since you spotted it, better fix it first. So I'll fix and respin the series.

Thanks for the continued reviews! :-)

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

Dear Luca Ceresoli,

Just say so in your review. Something like:

"Peter, if you fix the minor nit <here description of the nit>, then you can add my Reviewed-by on this patch".

Thomas

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

From: "Yann E. MORIN" <[email protected]>

The reason to save license files even for no-redistribute packages is that the license still applies to the files distributed as part of the rootfs, even if the sources are not themselves redistributed.

Move the copy of license files out of the non-local, non-overriden package case.

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

index 3d8f0da..006f862 100644
@@ -575,6 +575,22 @@ $(2)_MANIFEST_TARBALL ?= not saved
 
 # legal-info: produce legally relevant info.
 $(1)-legal-info:
+# Save license files if defined
+# We save the license files for any kind of package: normal, local,
+# overriden, or non-redistributable alike.
+# The reason to save license files even for no-redistribute packages
+# 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))
+else
+# Double dollar signs are really needed here, to catch host packages
+# without explicit HOST_FOO_LICENSE_FILES assignment, also in case they
+# have multiple license files.
+	@$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME),$$(F),$$($(2)_DIR)/$$(F),$(call UPPERCASE,$(4)))$$(sep))
+endif # license files
+
 # Packages without a source are assumed to be part of Buildroot, skip them.
 ifneq ($(call qstrip,$$($(2)_SOURCE)),)
 

@@ -588,17 +604,6 @@ else ifneq ($$($(2)_OVERRIDE_SRCDIR),)
 else
 # Other packages
 
-# Save license files if defined
-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))
-else
-# Double dollar signs are really needed here, to catch host packages
-# without explicit HOST_FOO_LICENSE_FILES assignment, also in case they
-# have multiple license files.
-	@$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME),$$(F),$$($(2)_DIR)/$$(F),$(call UPPERCASE,$(4)))$$(sep))
-endif # license files
-
 ifeq ($$($(2)_REDISTRIBUTE),YES)
 # Copy the source tarball (just hardlink if possible)
 	@cp -l $(DL_DIR)/$$($(2)_SOURCE) $(REDIST_SOURCES_DIR_$(call UPPERCASE,$(4))) 2>/dev/null || \

1.8.3.2

Hi Yann,

Luca

Luca, All,

On 2014-03-18 18:51 +0100, Luca Ceresoli spake thusly:

[--SNIP--]

Ah, I see. 'toolchain' is a special package, indeed, and will probably never have a _LICENSE_FILES defined.

What I think is we should not fo:

    ifeq ($$($(2)_LICENSE_FILES),)

but probably:

    ifeq ($$(origin $(2)_LICENSE_FILES),undefined)

And then:

TOOLCHAIN_LICENS_FILES =

Which means that TOOLCHAIN_LICENSE_FILES *is* defined. Empty, but defined nonetheless.

In which case, a defined but empty _LICENSE_FILES meanns: no license file to deal with.

OK, I'll see what I can do about that.

Thanks for the review! :-)

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

From: "Yann E. MORIN" <[email protected]>

As the legal-info infra only (rightfully) saves the tarballs of packages that:
- we want to redistribute,
- and are not local,
- and are not overriden,

add a comment stating so.

This should clarify the code-block, which although trivial to read, was not easy to interpret without thinking thouroughly about it.

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

index 006f862..953d6f9 100644
@@ -562,6 +562,9 @@ ifneq ($$($(2)_LICENSE_FILES),)
 $(1)-legal-info: $(1)-extract
 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
+# name of those packages.
 ifeq ($$($(2)_REDISTRIBUTE),YES)
 ifneq ($$($(2)_SITE_METHOD),local)
 ifneq ($$($(2)_SITE_METHOD),override)

1.8.3.2

From: "Yann E. MORIN" <[email protected]>

This helper was called when none of the sources or license files were saved.

Now we handle license files separately from the sources, this is no longer the case: they are only called when the sources are not saved.

Rename the handler and change the warning message accordingly.

---
 package/pkg-generic.mk | 4 ++--
 package/pkg-utils.mk   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

index 953d6f9..58d4ccc 100644
@@ -599,10 +599,10 @@ ifneq ($(call qstrip,$$($(2)_SOURCE)),)
 
 ifeq ($$($(2)_SITE_METHOD),local)
 # Packages without a tarball: don't save and warn
-	@$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),local)
+	@$(call legal-license-nosource,$$($(2)_RAWNAME),local)
 
 else ifneq ($$($(2)_OVERRIDE_SRCDIR),)
-	@$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),override)
+	@$(call legal-license-nosource,$$($(2)_RAWNAME),override)
 
 else
 # Other packages

index 91a1981..68f1dda 100644
@@ -105,8 +105,8 @@ define legal-warning-pkg # pkg, text
 	echo "WARNING: $(1): $(2)" >>$(LEGAL_WARNINGS)
 endef
 
-define legal-warning-pkg-savednothing # pkg, {local|override}
-	$(call legal-warning-pkg,$(1),sources and license files not saved ($(2) packages not handled))
+define legal-license-nosource # pkg, {local|override}
+	$(call legal-warning-pkg,$(1),sources not saved ($(2) packages not handled))
 endef
 
 define legal-manifest # pkg, version, license, license-files, source, {HOST|TARGET}

1.8.3.2