Skip to contentSkip to conversation

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

PATCH Superseded Yann E. MORIN 3 participants 16 messages Jun 14, 2014 – Jun 21, 2014

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore

Yann E. MORIN

author
#0 lore raw

v5 cover 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 v4 -> v5:
- don't warn about missing license fioles for packages bundled with Buildroot (Luca)
- new patch 5/5 to display a proper error message for missing license files
- typoes

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 712414ce2dc6f40ed98a3d8007e7d7ec382f79d4:

gcc: drop redundant explicit version handling for aarch64 (2014-06-13 23:01:16 +0200)

are available in the git repository at:

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

for you to fetch changes up to 8c742a7155e0b24c14ae90f7be00c1043eceb848:

legal-info: properly error out on missing license file (2014-06-14 16:59:40 +0200)

----------------------------------------------------------------
Yann E. MORIN (5):
legal-info: extract even no-redistribute packages
legal-info: save license files even for local or overriden packages
legal-info: add a comment about what packages we save the tarballs of
legal-info: rename legal-warning-pkg-savednothing helper
legal-info: properly error out on missing license file

 package/pkg-generic.mk | 39 +++++++++++++++++++++++++++------------
 package/pkg-utils.mk   | 22 +++++++++++++++-------
 2 files changed, 42 insertions(+), 19 deletions(-)
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
author
#1 lore raw

PATCH v5 1/5 legal-info: extract even no-redistribute packages

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 packages we want to save (ie. redistributable, non-local and non-overriden packages).

Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas De Schampheleire <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Fabio Porcedda <[email protected]>

---
Changes v4 -> v5:
- my usual s/pacakge/package/

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(-)
package/pkg-generic.mk +9 -2
index ccd7f3b..eb3ec9f 100644
@@ -595,11 +595,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 archive if we are to save it
+$(1)-legal-info: $(1)-source $(REDIST_SOURCES_DIR_$(call UPPERCASE,$(4)))
 $(2)_MANIFEST_TARBALL = $$($(2)_SOURCE)
 endif
 endif
Signature · 1 line
1.8.3.2
#2 lore raw

PATCH 1/5 legal-info: extract even no-redistribute packages

Dear Yann,

Quoted 8 lines · Yann E. MORIN

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:

Quoted 7 lines
     $ 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

Note that the present patchset does not solve _this_ error.

The error that your patchset _does_ solve is:

   $ make BR2_DL_DIR=~/src legal-info-prepare libfslcodec-legal-info
Quoted 1 line
Quoted 1 line
Quoted 1 line

Collecting legal info

cat:
/home/murray/devel/buildroot-test/output/build/libfslcodec-3.5.7-1.0.0/EULA:
No such file or directory
make: *** [libfslcodec-legal-info] Error 1
$

The error that you reported due to the fact that output/legal-info has not been created yet.

Workaround:

$ make BR2_DL_DIR=~/src legal-info-prepare libfslcodec-legal-info
                         ^^^^^^^^^^^^^^^^^^

This is not related to your patches, and it's due to the fact that the legal-info stuff is meant to be executed only from its top-level target:

   $ make legal-info
which in turn runs:
  - legal-info-clean, which removes the entire legal-info subdir;
  - legal-info-prepare, which creates dirs and prepares skeleton files
    and the manifest heading;
  - <pkg>-legal-info, which fills the skeleton files.

Maybe this could be fixed by making all $(1)-legal-info targets depend on legal-info-clean and then legal-info-prepare.
If it works it would be a nice improvement, although mostly visible when testing the legal infra. For real cases I don't see a big point in running anything else than make legal-info.

Quoted 5 lines

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'

s/overriden/overridden/

Quoted 2 lines

remote packages. Even though we do not save the sources for the overriden or local packages, we need to save their licensing info,

Ditto.

Quoted 5 lines

so we need to extract them.

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

Said this way, it looks like we're adding a dependency. Instead we are changing the dependence from PKG-extract to PKG-source, which is one step less (-extract implies -source), so bottom line we are removing a dependency.

Better explained IMO:
This implies that we now need only PKG-source, not PKG-extract anymore, as a dependency of legal-info for packages we want to save (.....).

Quoted 32 lines
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas De Schampheleire <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Fabio Porcedda <[email protected]>

---
Changes v4 -> v5:
- my usual s/pacakge/package/

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(-)
package/pkg-generic.mk +0 -0
index ccd7f3b..eb3ec9f 100644

package/pkg-generic.mk $(2)_MANIFEST_LICENSE_FILES = $$($(2)_LICENSE_FILES) · line 599

Quoted 5 lines
package/pkg-generic.mk +2 -0
@@ -595,11 +595,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

Ditto.

Quoted 6 lines
package/pkg-generic.mk +5 -0
+# we want to redistribute it or not.
+ifneq ($$($(2)_LICENSE_FILES),)
+$(1)-legal-info: $(1)-extract
+endif
+

package/pkg-generic.mk $(2)_MANIFEST_LICENSE_FILES = $$($(2)_LICENSE_FILES) · line 612

Quoted 10 lines
package/pkg-generic.mk +2 -2
  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 archive if we are to save it
+$(1)-legal-info: $(1)-source $(REDIST_SOURCES_DIR_$(call UPPERCASE,$(4)))
  $(2)_MANIFEST_TARBALL = $$($(2)_SOURCE)
  endif
  endif

With the above fixed, and once rebased on top of master, and since I'm OK with all of the very few code lines you touched:

Reviewed-by Luca Ceresoli <[email protected]>
[Quick test on top of e00c631ef4aa, will test again once rebased]
Tested-by Luca Ceresoli <[email protected]>
Signature · 1 line
Luca
author
#3 lore raw

PATCH v5 1/5 legal-info: extract even no-redistribute packages

Luca, All,

On 2014-06-18 23:17 +0200, Luca Ceresoli spake thusly:

Quoted 21 lines

Dear Yann,

Quoted 8 lines · Yann E. MORIN

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:

Quoted 7 lines
    $ 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

Note that the present patchset does not solve _this_ error.

Quoted 8 lines

The error that your patchset _does_ solve is:

  $ make BR2_DL_DIR=~/src legal-info-prepare libfslcodec-legal-info
Quoted 1 line
Quoted 1 line
Quoted 1 line

Collecting legal info

cat:
/home/murray/devel/buildroot-test/output/build/libfslcodec-3.5.7-1.0.0/EULA:
No such file or directory
make: *** [libfslcodec-legal-info] Error 1

Well, that's not even the error I'm trying to solve. In fact, I'm trying to solve this:

    $ cat defconfig
    BR2_arm=y
    BR2_TOOLCHAIN_BUILDROOT_EGLIBC=y
    BR2_PACKAGE_LIBFSLCODEC=y
    $ make BR2_DEFCONFIG=$(pwd)/defconfig defconfig
    $ make legal-info
    [--SNIP--]
    cat: /home/ymorin/dev/buildroot/O/build/libfslcodec-3.5.7-1.0.0/EULA: No
    such file or directory
Quoted 2 lines

The error that you reported due to the fact that output/legal-info has not been created yet.

Indeed. I was a bit too fast at writing the commit log (it happens quite often these days, I must be moire careful.) Sorry.

Quoted 2 lines

For real cases I don't see a big point in
running anything else than make legal-info.

So do I.

[--SNIP--]

Quoted 4 lines
Quoted 3 lines

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

Quoted 8 lines

Said this way, it looks like we're adding a dependency. Instead we are changing the dependence from PKG-extract to PKG-source, which is one step less (-extract implies -source), so bottom line we are removing a dependency.

Better explained IMO:
This implies that we now need only PKG-source, not PKG-extract anymore, as a dependency of legal-info for packages we want to save (.....).

OK, will rephrase.

[--SNIP--]

Quoted 4 lines

package/pkg-generic.mk $(2)_MANIFEST_LICENSE_FILES = $$($(2)_LICENSE_FILES) · line 599

Quoted 2 lines
package/pkg-generic.mk +2 -0
+# If the package declares _LICENSE_FILES, we need to extract it,
+# for overriden, local or normal remote packages alike, whether

Ditto.

Will fix that and the others.

[--SNIP--]

Quoted 6 lines

With the above fixed, and once rebased on top of master, and since I'm OK with all of the very few code lines you touched:

Reviewed-by Luca Ceresoli <[email protected]>
[Quick test on top of e00c631ef4aa, will test again once rebased]
Tested-by Luca Ceresoli <[email protected]>

OK, thank you! :-)

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
author
#4 lore raw

PATCH v5 2/5 legal-info: save license files even for local or overriden packages

In reply to Yann E. MORIN

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

Even if we do not save the sources for local or overrident packages because it is too complex, we can still quite easily save the license files.

Also, having the license files is a very important part of complying with the licenses.

Move the copy of license files out of the non-local, non-overriden package case, but still in the case where packages have a _SOURCE defined, to avoid catching packages bundled in Buildroot (eg. mkpasswd et al.)

Reported-by Luca Ceresoli <[email protected]>
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas De Schampheleire <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Fabio Porcedda <[email protected]>

---
Changes v4 -> v5:
- avoid warning for packages bundled in Buildroot (Luca)
- the change is not a bout non-redistributable packages, but about local and overriden packages

---
 package/pkg-generic.mk | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)
package/pkg-generic.mk +15 -10
index eb3ec9f..a978e63 100644
@@ -619,17 +619,12 @@ $(1)-legal-info:
 	$(foreach hook,$($(2)_PRE_LEGAL_INFO_HOOKS),$(call $(hook))$(sep))
 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)
-
-else ifneq ($$($(2)_OVERRIDE_SRCDIR),)
-	@$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),override)
-
-else
-# Other packages
-
 # 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))

@@ -640,6 +635,16 @@ else
 	@$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME),$$(F),$$($(2)_DIR)/$$(F),$(call UPPERCASE,$(4)))$$(sep))
 endif # license files
 
+ifeq ($$($(2)_SITE_METHOD),local)
+# Packages without a tarball: don't save and warn
+	@$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),local)
+
+else ifneq ($$($(2)_OVERRIDE_SRCDIR),)
+	@$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),override)
+
+else
+# Other packages
+
 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 || \
Signature · 1 line
1.8.3.2
#5 lore raw

PATCH 2/5 legal-info: save license files even for local or overriden packages

Dear Yann,

Quoted 3 lines · Yann E. MORIN

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

Even if we do not save the sources for local or overrident packages because

s/overrident/overridden/

Quoted 6 lines

it is too complex, we can still quite easily save the license files.

Also, having the license files is a very important part of complying with the licenses.

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

s/overriden/overridden/

Quoted 30 lines

case, but still in the case where packages have a _SOURCE defined, to avoid catching packages bundled in Buildroot (eg. mkpasswd et al.)

Reported-by Luca Ceresoli <[email protected]>
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas De Schampheleire <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Fabio Porcedda <[email protected]>

---
Changes v4 -> v5:
- avoid warning for packages bundled in Buildroot (Luca)
- the change is not a bout non-redistributable packages, but about local and overriden packages

---
  package/pkg-generic.mk | 25 +++++++++++++++----------
  1 file changed, 15 insertions(+), 10 deletions(-)
package/pkg-generic.mk +0 -3
index eb3ec9f..a978e63 100644
@@ -619,17 +619,12 @@ $(1)-legal-info:
  	$(foreach hook,$($(2)_PRE_LEGAL_INFO_HOOKS),$(call $(hook))$(sep))
  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)

package/pkg-generic.mk $(1)-legal-info: · line 624

Quoted 10 lines
package/pkg-generic.mk +2 -7
-
-else ifneq ($$($(2)_OVERRIDE_SRCDIR),)
-	@$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),override)
-
-else
-# Other packages
-
  # Save license files if defined
+# We save the license files for any kind of package: normal, local,
+# overriden, or non-redistributable alike.

s/overriden/overridden/

Quoted 14 lines
package/pkg-generic.mk +6 -0
+# 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))

@@ -640,6 +635,16 @@ else
  	@$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME),$$(F),$$($(2)_DIR)/$$(F),$(call UPPERCASE,$(4)))$$(sep))
  endif # license files
 
+ifeq ($$($(2)_SITE_METHOD),local)
+# Packages without a tarball: don't save and warn
+	@$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),local)

package/pkg-generic.mk else · line 650

Quoted 10 lines
package/pkg-generic.mk +7 -0
+
+else ifneq ($$($(2)_OVERRIDE_SRCDIR),)
+	@$(call legal-warning-pkg-savednothing,$$($(2)_RAWNAME),override)
+
+else
+# Other packages
+
  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 || \

With the above fixed, and once rebased on top of master, and since I'm OK with the actual code changes:

Reviewed-by Luca Ceresoli <[email protected]>
[Quick test on top of e00c631ef4aa, will test again once rebased]
Tested-by Luca Ceresoli <[email protected]>
Signature · 1 line
Luca
author
#6 lore raw

PATCH v5 3/5 legal-info: add a comment about what packages we save the tarballs of

In reply to Yann E. MORIN

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.

Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas De Schampheleire <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Fabio Porcedda <[email protected]>
---
 package/pkg-generic.mk | 3 +++
 1 file changed, 3 insertions(+)
package/pkg-generic.mk +3 -0
index a978e63..d16908a 100644
@@ -602,6 +602,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)
Signature · 1 line
1.8.3.2
#7 lore raw

PATCH 3/5 legal-info: add a comment about what packages we save the tarballs of

Dear Yann,

Quoted 28 lines · Yann E. MORIN

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.

Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas De Schampheleire <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Fabio Porcedda <[email protected]>
---
  package/pkg-generic.mk | 3 +++
  1 file changed, 3 insertions(+)
package/pkg-generic.mk +0 -0
index a978e63..d16908a 100644

package/pkg-generic.mk ifneq ($$($(2)_LICENSE_FILES),) · line 610

Quoted 9 lines
package/pkg-generic.mk +3 -0
@@ -602,6 +602,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)
Reviewed-by Luca Ceresoli <[email protected]>
Signature · 1 line
Luca
author
#8 lore raw

PATCH v5 4/5 legal-info: rename legal-warning-pkg-savednothing helper

In reply to Yann E. MORIN

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.

Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas De Schampheleire <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Fabio Porcedda <[email protected]>
---
 package/pkg-generic.mk | 4 ++--
 package/pkg-utils.mk   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
package/pkg-generic.mk +2 -2
index d16908a..8a32f9d 100644
@@ -640,10 +640,10 @@ endif # license files
 
 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
package/pkg-utils.mk +2 -2
index 163ef8a..471f3ed 100644
@@ -101,8 +101,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}
Signature · 1 line
1.8.3.2
#9 lore raw

PATCH 4/5 legal-info: rename legal-warning-pkg-savednothing helper

Dear Yann,

Quoted 43 lines · Yann E. MORIN

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.

Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas De Schampheleire <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Fabio Porcedda <[email protected]>
---
  package/pkg-generic.mk | 4 ++--
  package/pkg-utils.mk   | 4 ++--
  2 files changed, 4 insertions(+), 4 deletions(-)
package/pkg-generic.mk +2 -2
index d16908a..8a32f9d 100644
@@ -640,10 +640,10 @@ endif # license files
 
  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
package/pkg-utils.mk +0 -0
index 163ef8a..471f3ed 100644

package/pkg-utils.mk define legal-warning-pkg # pkg, text · line 104

Quoted 6 lines
package/pkg-utils.mk +1 -2
@@ -101,8 +101,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}

I'm OK with the idea, but I not with the name.
Why "license"? This has to do with the source tarball. Also, I would keep "warning" in the name, as it's what it is.

So, I'd rename it to legal-warning-nosource.

Signature · 1 line
Luca
author
#10 lore raw

PATCH v5 4/5 legal-info: rename legal-warning-pkg-savednothing helper

Luca, All,

On 2014-06-18 17:32 +0200, Luca Ceresoli spake thusly:

Quoted 55 lines
Quoted 43 lines · Yann E. MORIN

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.

Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas De Schampheleire <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Fabio Porcedda <[email protected]>
---
 package/pkg-generic.mk | 4 ++--
 package/pkg-utils.mk   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
package/pkg-generic.mk +2 -2
index d16908a..8a32f9d 100644
@@ -640,10 +640,10 @@ endif # license files
 
 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
package/pkg-utils.mk +0 -0
index 163ef8a..471f3ed 100644

package/pkg-utils.mk define legal-warning-pkg # pkg, text · line 104

Quoted 6 lines
package/pkg-utils.mk +1 -2
@@ -101,8 +101,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}

I'm OK with the idea, but I not with the name.
Why "license"? This has to do with the source tarball. Also, I would keep "warning" in the name, as it's what it is.

Quoted 1 line

So, I'd rename it to legal-warning-nosource.

Indeed. What was on my head when I did that?... :-/

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
author
#11 lore raw

PATCH v5 5/5 legal-info: properly error out on missing license file

In reply to Yann E. MORIN

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

When saving the license files, if a declared license file is missing, print a proper error message, instead of the sometime-criptic error message from 'cp'.

Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas De Schampheleire <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Fabio Porcedda <[email protected]>

---
Note: when I wrote this back in March 2014, I had a concrete example of a criptic error message (the name of the license did not make it obvious we were saving a license file?), but today I can't remember what it was exactly.

Since I believe showing a properly-formatted error message to the user is anyway a good thing, I kept that patch. Feel free to ditch it if you don't like it.

---
 package/pkg-utils.mk | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)
package/pkg-utils.mk +13 -5
index 471f3ed..c3f9142 100644
@@ -101,6 +101,10 @@ define legal-warning-pkg # pkg, text
 	echo "WARNING: $(1): $(2)" >>$(LEGAL_WARNINGS)
 endef
 
+define legal-error-pkg # pkg, text
+	echo "ERROR: $(1): $(2)" && false
+endef
+
 define legal-license-nosource # pkg, {local|override}
 	$(call legal-warning-pkg,$(1),sources not saved ($(2) packages not handled))
 endef

@@ -119,9 +123,13 @@ define legal-license-nofiles # pkg, {HOST|TARGET}
 endef
 
 define legal-license-file # pkg, filename, file-fullpath, {HOST|TARGET}
-	$(call legal-license-header,$(1),$(2) file,$(4)) && \
-	cat $(3) >>$(LEGAL_LICENSES_TXT_$(4)) && \
-	echo >>$(LEGAL_LICENSES_TXT_$(4)) && \
-	mkdir -p $(LICENSE_FILES_DIR_$(4))/$(1)/$(dir $(2)) && \
-	cp $(3) $(LICENSE_FILES_DIR_$(4))/$(1)/$(2)
+	if [ -e $(3) ]; then \
+		$(call legal-license-header,$(1),$(2) file,$(4)) && \
+		cat $(3) >>$(LEGAL_LICENSES_TXT_$(4)) && \
+		echo >>$(LEGAL_LICENSES_TXT_$(4)) && \
+		mkdir -p $(LICENSE_FILES_DIR_$(4))/$(1)/$(dir $(2)) && \
+		cp $(3) $(LICENSE_FILES_DIR_$(4))/$(1)/$(2); \
+	else \
+		$(call legal-error-pkg,$(1),license file $(2) not found); \
+	fi
 endef
Signature · 1 line
1.8.3.2
#12 lore raw

PATCH 5/5 legal-info: properly error out on missing license file

Dear Yann,

Quoted 18 lines · Yann E. MORIN

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

When saving the license files, if a declared license file is missing, print a proper error message, instead of the sometime-criptic error message from 'cp'.

Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas De Schampheleire <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Fabio Porcedda <[email protected]>

---
Note: when I wrote this back in March 2014, I had a concrete example of a criptic error message (the name of the license did not make it obvious we were saving a license file?), but today I can't remember what it was exactly.

Quoted 3 lines

Since I believe showing a properly-formatted error message to the user is anyway a good thing, I kept that patch. Feel free to ditch it if you don't like it.

I may be missing something, but I wonder what cryptic error you had seen. I tried removing a license file and got, before your patch:

cat:
/home/murray/devel/buildroot-test/output/build/busybox-1.22.1/LICENSE:
No such file or directory

Except for the "cat: " prefix, it's quite understandable IMHO.

With your patch applied:

ERROR: busybox: license file LICENSE not found

This is much more concise, but gives a less info about where the LICENSE file should be found. To the inexperienced user, the path might be a useful hint.
OTOH, the "ERROR: busybox: " prefix is very informative.

How about changing your message to:
ERROR: busybox: license file LICENSE not found in /.../busybox-1.22.1/
or
ERROR: busybox: license file /.../busybox-1.22.1/LICENSE not found

Signature · 1 line
Luca
author
#13 lore raw

PATCH v5 5/5 legal-info: properly error out on missing license file

Luca, All,

On 2014-06-18 23:38 +0200, Luca Ceresoli spake thusly:

Quoted 19 lines
Quoted 18 lines · Yann E. MORIN

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

When saving the license files, if a declared license file is missing, print a proper error message, instead of the sometime-criptic error message from 'cp'.

Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas De Schampheleire <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Fabio Porcedda <[email protected]>

---
Note: when I wrote this back in March 2014, I had a concrete example of a criptic error message (the name of the license did not make it obvious we were saving a license file?), but today I can't remember what it was exactly.

Quoted 6 lines
Quoted 3 lines

Since I believe showing a properly-formatted error message to the user is anyway a good thing, I kept that patch. Feel free to ditch it if you don't like it.

I may be missing something, but I wonder what cryptic error you had seen.

Well, now I tried again, that's what I observed:

    $ make legal-info
    [--SNIP--]
Quoted 1 line
Quoted 1 line
Quoted 1 line

busybox 1.22.1 Extracting

    bzcat /home/ymorin/src/busybox-1.22.1.tar.bz2 | tar --strip-components=1
    -C /home/ymorin/dev/buildroot/O/build/busybox-1.22.1  -xf -
    cat: /home/ymorin/dev/buildroot/O/build/libfslcodec-3.5.7-1.0.0/EULA: No
    such file or directory
    make[1]: *** [libfslcodec-legal-info] Error 1

So, OK, the error is explicitly about fslcodec. But remember that the line ">>> busybox 1.22.1 Extracting" is highlighted, and thus what stand out is the 'busybox' package, and it took me a while to understand why on Earth busybox was trying to save an EULA file, when moments later I saw that it was in fact in the libfslcodec package...

But since this series is expressly about this error case (no-redistribute packages failed to save the license files), the initial error should be handled.

I'll redo my tsting here to see if that's still a concern now...

Quoted 9 lines

With your patch applied:

ERROR: busybox: license file LICENSE not found

This is much more concise, but gives a less info about where the LICENSE file should be found. To the inexperienced user, the path might be a useful hint.
OTOH, the "ERROR: busybox: " prefix is very informative.

Quoted 4 lines

How about changing your message to:
ERROR: busybox: license file LICENSE not found in /.../busybox-1.22.1/
or
ERROR: busybox: license file /.../busybox-1.22.1/LICENSE not found

Yes, we can use the full path to the missing license file, indeed.

Thanks! :-)

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
author
#14 lore raw

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

In reply to Yann E. MORIN

All,

On 2014-06-14 17:10 +0200, Yann E. MORIN spake thusly:

Quoted 1 line

Here is a series that fixes the legal-info infra:

Forgt this series for now, it causes a hell of a mess to rebase ontop of the recent $$ changes. I'm fixing locally, and will wait for more comments before respinning.

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#15 lore raw

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

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

Quoted 7 lines

All,

On 2014-06-14 17:10 +0200, Yann E. MORIN spake thusly:

Quoted 1 line

Here is a series that fixes the legal-info infra:

Forgt this series for now, it causes a hell of a mess to rebase ontop of the recent $$ changes.

You wanted dollars, now you'll have to deal with them!

;-)