[Buildroot] [PATCH 0/4 v4] legal-info: extract packages to get license files (branch yem/legal)

[Buildroot] [PATCH 0/4 v4] legal-info: extract packages to get license files (branch yem/legal)

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

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

[Buildroot] [PATCH 1/4] legal-info: extract even no-redistribute packages

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

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

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Fabio Porcedda <fabio.porcedda@gmail.com>

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 339c3eb..3d8f0da 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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

[Buildroot] [PATCH 2/4] legal-info: save license files even for no-redistribute packages

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

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.

Reported-by: Luca Ceresoli <luca@lucaceresoli.net>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 package/pkg-generic.mk | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 3d8f0da..006f862 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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

[Buildroot] [PATCH 3/4] legal-info: add a comment about what packages we save the tarballs of

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

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" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 package/pkg-generic.mk | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 006f862..953d6f9 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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

[Buildroot] [PATCH 4/4] legal-info: rename legal-warning-pkg-savednothing helper

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

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" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Fabio Porcedda <fabio.porcedda@gmail.com>
---
 package/pkg-generic.mk | 4 ++--
 package/pkg-utils.mk   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 953d6f9..58d4ccc 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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
diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index 91a1981..68f1dda 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -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

[Buildroot] [PATCH 1/4] legal-info: extract even no-redistribute packages

[Luca Ceresoli]

Hi Yann,

Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> 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).
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Fabio Porcedda <fabio.porcedda@gmail.com>
>
> ---
> 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(-)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 339c3eb..3d8f0da 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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

Minor nit: package sources <-> save them   (plural)
        or: package archive <-> save it     (singular)

This is so smaller than the overall improvement in the patch, and it's
only in a comment which is understandable anyway, so I would tag this
patch as Reviewed-by me. But the recently established policy is that
"If you reviewed a patch and have comments on it, you should simply
reply to the patch stating these comments, without providing a
Reviewed-by or Acked-by tag.".

So I cannot tag here, sorry, but fix this and I'll do so.

BTW, what if I have comments but would commit as is and improve in a
successive patch? Should I add Acked/Review-by? This is the case here,
actually.

-- 
Luca

[Buildroot] [PATCH 1/4] legal-info: extract even no-redistribute packages

[Yann E. MORIN]

Luca, All,

On 2014-03-18 18:36 +0100, Luca Ceresoli spake thusly:
> Yann E. MORIN wrote:
> >From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >
> >Currently, if a package is marked _REDISTRIBUTE = NO, then legal-info
> >will not try to extract it first.
[--SNIP--]
> >-# 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
> 
> Minor nit: package sources <-> save them   (plural)
>        or: package archive <-> save it     (singular)

Yep, thanks! :-)

> This is so smaller than the overall improvement in the patch, and it's
> only in a comment which is understandable anyway, so I would tag this
> patch as Reviewed-by me. But the recently established policy is that
> "If you reviewed a patch and have comments on it, you should simply
> reply to the patch stating these comments, without providing a
> Reviewed-by or Acked-by tag.".
> 
> So I cannot tag here, sorry, but fix this and I'll do so.

Yep, no problem. :-)

> BTW, what if I have comments but would commit as is and improve in a
> successive patch? Should I add Acked/Review-by? This is the case here,
> actually.

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

[Buildroot] [PATCH 2/4] legal-info: save license files even for no-redistribute packages

[Luca Ceresoli]

Hi Yann,

Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> 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.
>
> Reported-by: Luca Ceresoli <luca@lucaceresoli.net>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Fabio Porcedda <fabio.porcedda@gmail.com>
> ---
>   package/pkg-generic.mk | 27 ++++++++++++++++-----------
>   1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 3d8f0da..006f862 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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)),)

I like this change in general, but I have another minor nit here.

You moved code out of the big
   ifneq ($(call qstrip,$$($(2)_SOURCE)),)
cited above.

So we're now doing stuff also for packages that are part of Buildroot.

Assuming these will never have _LICENSE_FILES defined, which is fair,
we not have one more warning:
   WARNING: toolchain: cannot save license (TOOLCHAIN_LICENSE_FILES \
         not defined)

Probably that ifneq should be moved just before the part of code you're
moving up, but that should be checked carefully.

If you feel like this bunch of lines are an annoying mess to handle, I'm
with you.

So, once more, I cannot give my Reviewed-by... :(

-- 
Luca

[Buildroot] [PATCH 1/4] legal-info: extract even no-redistribute packages

[Thomas Petazzoni]

Dear Luca Ceresoli,

On Tue, 18 Mar 2014 18:36:29 +0100, Luca Ceresoli wrote:

> Minor nit: package sources <-> save them   (plural)
>         or: package archive <-> save it     (singular)
> 
> This is so smaller than the overall improvement in the patch, and it's
> only in a comment which is understandable anyway, so I would tag this
> patch as Reviewed-by me. But the recently established policy is that
> "If you reviewed a patch and have comments on it, you should simply
> reply to the patch stating these comments, without providing a
> Reviewed-by or Acked-by tag.".
> 
> So I cannot tag here, sorry, but fix this and I'll do so.
> 
> BTW, what if I have comments but would commit as is and improve in a
> successive patch? Should I add Acked/Review-by? This is the case here,
> actually.

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

[Buildroot] [PATCH 2/4] legal-info: save license files even for no-redistribute packages

[Yann E. MORIN]

Luca, All,

On 2014-03-18 18:51 +0100, Luca Ceresoli spake thusly:
> Yann E. MORIN wrote:
> >From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >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.
[--SNIP--]
> I like this change in general, but I have another minor nit here.
> 
> You moved code out of the big
>   ifneq ($(call qstrip,$$($(2)_SOURCE)),)
> cited above.
> 
> So we're now doing stuff also for packages that are part of Buildroot.
> 
> Assuming these will never have _LICENSE_FILES defined, which is fair,
> we not have one more warning:
>   WARNING: toolchain: cannot save license (TOOLCHAIN_LICENSE_FILES \
>         not defined)

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.

> Probably that ifneq should be moved just before the part of code you're
> moving up, but that should be checked carefully.
> 
> If you feel like this bunch of lines are an annoying mess to handle, I'm
> with you.

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

[Buildroot] [PATCH 1/4] legal-info: extract even no-redistribute packages

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Sun, 22 Jun 2014 14:41:12 +0200, Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> 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_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
> 
> 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: overridden, local or 'normal'
> remote packages. Even though we do not save the sources for the
> overridden or local packages, we need to save their licensing info,
> so we need to extract them.
> 
> 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 (ie.
> redistributable, non-local and non-overriden packages.)
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Fabio Porcedda <fabio.porcedda@gmail.com>
> Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

Applied, thanks.

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

[Buildroot] [PATCH 1/4] legal-info: extract even no-redistribute packages

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

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

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: overridden, local or 'normal'
remote packages. Even though we do not save the sources for the
overridden or local packages, we need to save their licensing info,
so we need to extract them.

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

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Fabio Porcedda <fabio.porcedda@gmail.com>
Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

---
Chamges v5 -> v6:
  - fix commit log about the actual case that is being solved  (Luca)
  - fix the comment about needing only PKG-source  (Luca)

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 457d873..5f2a27e 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -620,11 +620,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)))
+# Packages that have a tarball need it downloaded beforehand
+$(1)-legal-info: $(1)-source $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
 $(2)_MANIFEST_TARBALL = $$($(2)_SOURCE)
 endif
 endif
-- 
1.8.3.2