core/legal-info: Add package dependencies with licenses to the manifest

PATCH Superseded sojkam1 at fel.cvut.cz 3 participants 9 messages Aug 10, 2018 – Aug 20, 2018 Kbuild

lore

Share & tools

lore

sojkam1 at fel.cvut.cz

author

2018-08-10 14:03 UTC #0 lore raw

PATCH v2 1/1 core/legal-info: Add package dependencies with licenses to the manifest

From: Michal Sojka <[email protected]>

This adds one column to the legal-info manifest table. It contains the dependencies of the given package and their licenses. This information is useful when assessing license compatibility of the packages and their libraries.

An example of the content of the new column for the MPD package is shown below:

    "alsa-lib [LGPL-2.1+ (library), GPL-2.0+ (aserver)], boost
    [BSL-1.0], libid3tag [GPL-2.0+], libmad [GPL-2.0+], libogg
    [BSD-3-Clause], libvorbis [BSD-3-Clause], libzlib [Zlib],
    skeleton-init-common [unknown], skeleton-init-sysv [unknown],
    sqlite [Public domain], toolchain-external-linaro-arm [unknown], "

Signed-off-by Michal Sojka <[email protected]>

---
Changes against v1:

* switched parameters of legal-manifest (added one is the last)
* producing some output even for host packages
* documented legal-deps macro

 Makefile               |  6 +++---
 package/pkg-generic.mk |  2 +-
 package/pkg-utils.mk   | 23 +++++++++++++++++++++--
 3 files changed, 25 insertions(+), 6 deletions(-)

Makefile +3 -3

index f79d39fd26..cadf43146b 100644
@@ -781,9 +781,9 @@ legal-info-clean:
 legal-info-prepare: $(LEGAL_INFO_DIR)
 	@$(call MESSAGE,"Buildroot $(BR2_VERSION_FULL) Collecting legal info")
 	@$(call legal-license-file,buildroot,buildroot,support/legal-info,COPYING,COPYING,HOST)
-	@$(call legal-manifest,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE,TARGET)
-	@$(call legal-manifest,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE,HOST)
-	@$(call legal-manifest,buildroot,$(BR2_VERSION_FULL),GPL-2.0+,COPYING,not saved,not saved,HOST)
+	@$(call legal-manifest,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE,TARGET,DEPENDENCIES WITH LICENSES)
+	@$(call legal-manifest,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE,HOST,DEPENDENCIES WITH LICENSES)
+	@$(call legal-manifest,buildroot,$(BR2_VERSION_FULL),GPL-2.0+,COPYING,not saved,not saved,HOST,)
 	@$(call legal-warning,the Buildroot source code has not been saved)
 	@cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config

package/pkg-generic.mk +1 -1

index 91b61c6de0..a539483ae4 100644
@@ -936,7 +936,7 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 endif # redistribute
 
 endif # other packages
-	@$$(call legal-manifest,$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call UPPERCASE,$(4)))
+	@$$(call legal-manifest,$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call UPPERCASE,$(4)),$$(call legal-deps,$(1)))
 endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
 	$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))

package/pkg-utils.mk +21 -2

index c3acc22b17..f7a3609443 100644
@@ -79,8 +79,8 @@ define legal-warning-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, url, {HOST|TARGET}
-	echo '"$(1)","$(2)","$(3)","$(4)","$(5)","$(6)"' >>$(LEGAL_MANIFEST_CSV_$(7))
+define legal-manifest # pkg, version, license, license-files, source, url, {HOST|TARGET}, dependencies
+	echo '"$(1)","$(2)","$(3)","$(4)","$(5)","$(6)","$(8)"' >>$(LEGAL_MANIFEST_CSV_$(7))
 endef
 
 define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-fullpath, {HOST|TARGET}

@@ -95,3 +95,22 @@ define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full
 	} && \
 	cp $(5) $(LICENSE_FILES_DIR_$(6))/$(2)/$(4)
 endef
+
+remove-virtual-pkgs = $(foreach p,$(1),$(if $($(call UPPERCASE,$(p))_IS_VIRTUAL),,$(p)))
+get-direct-deps = $(sort $(foreach p,$(1),$($(call UPPERCASE,$(p))_FINAL_DEPENDENCIES)))
+
+define get-transitive-deps # packages
+	$(if $(filter-out $(1),$(call get-direct-deps,$(1))),\
+	     $(sort $(1) $(call get-transitive-deps,$(filter-out $(1),$(call get-direct-deps,$(1))))),\
+	     $(1))
+endef
+
+non-virtual-deps = $(call remove-virtual-pkgs,$(filter-out $(1),$(call get-transitive-deps,$(1))))
+host-pattern-if-target-pkg = $(if $(1:host-%=),host-%,)
+
+# Produce a comma-separated list of dependent packages and their
+# licenses. Host packages are removed from the list if the argument is
+# not a host package.
+define legal-deps # package
+$(foreach p,$(filter-out $(call host-pattern-if-target-pkg,$(1)),$(call non-virtual-deps,$(1))),$(p) [$($(call UPPERCASE,$(p))_LICENSE)], )
+endef

Signature · 1 line

2.18.0

Yann E. MORIN

2018-08-12 14:22 UTC #1 lore raw

PATCH v2 1/1 core/legal-info: Add package dependencies with licenses to the manifest

Michal, All,

On 2018-08-10 16:03 +0200, sojkam1 at fel.cvut.cz spake thusly:

Quoted 7 lines

From: Michal Sojka <[email protected]>
This adds one column to the legal-info manifest table. It contains the dependencies of the given package and their licenses. This information is useful when assessing license compatibility of the packages and their libraries.

Quoted 8 lines

An example of the content of the new column for the MPD package is shown below:

    "alsa-lib [LGPL-2.1+ (library), GPL-2.0+ (aserver)], boost
    [BSL-1.0], libid3tag [GPL-2.0+], libmad [GPL-2.0+], libogg
    [BSD-3-Clause], libvorbis [BSD-3-Clause], libzlib [Zlib],
    skeleton-init-common [unknown], skeleton-init-sysv [unknown],
    sqlite [Public domain], toolchain-external-linaro-arm [unknown], "

I believe this is a very good addition to the manifest. Good idea! :-)

The trailing comma is ugly, though. I would just drop the coma altogether...

And here, I have two spaces between each packages:

    "alsa-lib [LGPL-2.1+ (library), GPL-2.0+ (aserver)],  boost
    [BSL-1.0],  libid3tag [GPL-2.0+],  libmad [GPL-2.0+],  [...]"

Quoted 4 lines

Signed-off-by Michal Sojka <[email protected]>
---
Changes against v1:
* switched parameters of legal-manifest (added one is the last)

Actually, I disagree with that one: it is OK that new parameters be added before the last, especially since the 'legal-manifest' macro would be easier to review, see below...

Quoted 2 lines

* producing some output even for host packages
* documented legal-deps macro

I have further comments, see below as well...

[--SNIP--]

package/pkg-utils.mk define legal-warning-nosource # pkg, {local|override} · line 84

Quoted 13 lines

package/pkg-utils.mk +2 -2

index c3acc22b17..f7a3609443 100644
@@ -79,8 +79,8 @@ define legal-warning-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, url, {HOST|TARGET}
-	echo '"$(1)","$(2)","$(3)","$(4)","$(5)","$(6)"' >>$(LEGAL_MANIFEST_CSV_$(7))
+define legal-manifest # pkg, version, license, license-files, source, url, {HOST|TARGET}, dependencies
+	echo '"$(1)","$(2)","$(3)","$(4)","$(5)","$(6)","$(8)"' >>$(LEGAL_MANIFEST_CSV_$(7))
 endef

This change would have been (slightly) easier to review if the order of parameters was changed.

In fact, I even believe a beter order would be:

{HOST|TARGET}, pkg, version, license, license-files, source, url, dependencies

(if you decide to go with this change too, then make it a separate patch that comes first.)

package/pkg-utils.mk define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full · line 100

Quoted 8 lines

package/pkg-utils.mk +3 -0

 define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-fullpath, {HOST|TARGET}

@@ -95,3 +95,22 @@ define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full
 	} && \
 	cp $(5) $(LICENSE_FILES_DIR_$(6))/$(2)/$(4)
 endef
+
+remove-virtual-pkgs = $(foreach p,$(1),$(if $($(call UPPERCASE,$(p))_IS_VIRTUAL),,$(p)))
+get-direct-deps = $(sort $(foreach p,$(1),$($(call UPPERCASE,$(p))_FINAL_DEPENDENCIES)))

This should be using $(2)_FINAL_ALL_DEPENDENCIES and not $(2)_FINAL_DEPENDENCIES, because the _EXTRACT_DEPENDENCIES and _PATCH_DEPENDENCIES can also be used to alter the content of the package. For example, we use _PATCH_DEPENDENCIES for the linux extensions, and those are changing the source code of the linux package, so their licensing terms also apply.

package/pkg-utils.mk define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full · line 105

Quoted 5 lines

package/pkg-utils.mk +5 -0

+define get-transitive-deps # packages
+	$(if $(filter-out $(1),$(call get-direct-deps,$(1))),\
+	     $(sort $(1) $(call get-transitive-deps,$(filter-out $(1),$(call get-direct-deps,$(1))))),\
+	     $(1))
+endef

What about this new variable instead:

package/pkg-generic.mk +8 -0

index a539483ae4..ca55ac5e8e 100644
@@ -609,6 +609,14 @@ $(2)_FINAL_ALL_DEPENDENCIES = \
 		$$($(2)_FINAL_DEPENDENCIES) \
 		$$($(2)_FINAL_EXTRACT_DEPENDENCIES) \
 		$$($(2)_FINAL_PATCH_DEPENDENCIES))
+$(2)_FINAL_RECURSIVE_DEPENDENCIES = \
+	$$(sort \
+		$$(foreach p,\
+			$$($(2)_FINAL_ALL_DEPENDENCIES),\
+			$$(p)\
+			$$($$(call UPPERCASE,$$(p))_FINAL_RECURSIVE_DEPENDENCIES)\
+		)\
+	)
 
 $(2)_INSTALL_STAGING?= NO
 $(2)_INSTALL_IMAGES?= NO

package/pkg-utils.mk define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full · line 106

Quoted 1 line

package/pkg-utils.mk +1 -0

+non-virtual-deps = $(call remove-virtual-pkgs,$(filter-out $(1),$(call get-transitive-deps,$(1))))

And this new macro:

non-virtual-deps = $(foreach p,$(1),$(if $($(call UPPERCASE,$(p))_IS_VIRTUAL),,$(p)))

package/pkg-utils.mk define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full · line 107

Quoted 1 line

package/pkg-utils.mk +1 -0

+host-pattern-if-target-pkg = $(if $(1:host-%=),host-%,)

I am not sure I grok that one... Oh, OK I got it. I don't think we need an intermediate variable (partly because its name is longer than the code it replaces...)

package/pkg-utils.mk define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full · line 113

Quoted 6 lines

package/pkg-utils.mk +6 -0

+# Produce a comma-separated list of dependent packages and their
+# licenses. Host packages are removed from the list if the argument is
+# not a host package.
+define legal-deps # package
+$(foreach p,$(filter-out $(call host-pattern-if-target-pkg,$(1)),$(call non-virtual-deps,$(1))),$(p) [$($(call UPPERCASE,$(p))_LICENSE)], )
+endef

... and then:

# $1: package name, lowercase

legal-deps = \
    $(foreach p,\
        $(filter-out $(if $(1:host-%=),host-%),\
            $($(call UPPERCASE,$(1))_FINAL_RECURSIVE_DEPENDENCIES)),\
        $(p) [$($(call UPPERCASE,$(p))_LICENSE)]\
    )

This is totally untested, but owrth investigating I think. ;-) Regards,
Yann E. MORIN.

Quoted 7 lines

Signature · 6 lines

2.18.0

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

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

Matthew Weber

2018-08-13 13:40 UTC #2 lore raw

PATCH v2 1/1 core/legal-info: Add package dependencies with licenses to the manifest

Yann,

Quoted 30 lines · Yann E. MORIN

Michal, All,
On 2018-08-10 16:03 +0200, sojkam1 at fel.cvut.cz spake thusly:
Quoted 7 lines
From: Michal Sojka <[email protected]>
This adds one column to the legal-info manifest table. It contains the dependencies of the given package and their licenses. This information is useful when assessing license compatibility of the packages and their libraries.
Quoted 8 lines
An example of the content of the new column for the MPD package is shown below:
    "alsa-lib [LGPL-2.1+ (library), GPL-2.0+ (aserver)], boost
    [BSL-1.0], libid3tag [GPL-2.0+], libmad [GPL-2.0+], libogg
    [BSD-3-Clause], libvorbis [BSD-3-Clause], libzlib [Zlib],
    skeleton-init-common [unknown], skeleton-init-sysv [unknown],
    sqlite [Public domain], toolchain-external-linaro-arm [unknown], "
I believe this is a very good addition to the manifest. Good idea! :-)
The trailing comma is ugly, though. I would just drop the coma altogether...
And here, I have two spaces between each packages:
    "alsa-lib [LGPL-2.1+ (library), GPL-2.0+ (aserver)],  boost
    [BSL-1.0],  libid3tag [GPL-2.0+],  libmad [GPL-2.0+],  [...]"

Quoted 8 lines

Quoted 4 lines
Signed-off-by Michal Sojka <[email protected]>
---
Changes against v1:
* switched parameters of legal-manifest (added one is the last)
Actually, I disagree with that one: it is OK that new parameters be added before the last, especially since the 'legal-manifest' macro would be easier to review, see below...

If we change the format of the legal info csv, is there someway we could determine version of that file's syntax? I assume worst case we can parse out the first line and see the additional dependencies entry?

I'm concerned about external tools impact to changing this file's
format. I'm sure there are others that use this file for CVE
analysis and legal reporting.

Matt

Yann E. MORIN

2018-08-13 15:18 UTC #3 lore raw

PATCH v2 1/1 core/legal-info: Add package dependencies with licenses to the manifest

Matthew, All,

On 2018-08-13 08:40 -0500, Matthew Weber spake thusly:

Quoted 2 lines · Yann E. MORIN

Quoted 1 line
On 2018-08-10 16:03 +0200, sojkam1 at fel.cvut.cz spake thusly:

[--SNIP--]

Quoted 10 lines

Quoted 5 lines
Quoted 2 lines
Changes against v1:
* switched parameters of legal-manifest (added one is the last)
Actually, I disagree with that one: it is OK that new parameters be added before the last, especially since the 'legal-manifest' macro would be easier to review, see below...
If we change the format of the legal info csv, is there someway we could determine version of that file's syntax? I assume worst case we can parse out the first line and see the additional dependencies entry?

So, I am not arguing that we should change the output at all.

What I am saying is that the _list_of_parameters_ can be reorganised, while still keeping the output as before.

I.e. the 'legal-manifest' macro could be rewrittent from the current:

    define legal-manifest # pkg, version, license, license-files, source, url, {HOST|TARGET}
        echo '"$(1)","$(2)","$(3)","$(4)","$(5)","$(6)"' >>$(LEGAL_MANIFEST_CSV_$(7))
    endef

to this new one:

define legal-manifest # {HOST|TARGET}, pkg, version, license, license-files, source, url, dependencies
echo '"$(2)","$(3)","$(4)","$(5)","$(6)","$(7)","$(8)"' >>$(LEGAL_MANIFEST_CSV_$(1))
endef

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

Michal Sojka

author

2018-08-20 13:57 UTC #4 lore raw

PATCH v2 1/1 core/legal-info: Add package dependencies with licenses to the manifest

In reply to Yann E. MORIN

Dear Yann and others,

Quoted 26 lines · Yann E. MORIN

Michal, All,
On 2018-08-10 16:03 +0200, sojkam1 at fel.cvut.cz spake thusly:
Quoted 7 lines
From: Michal Sojka <[email protected]>
This adds one column to the legal-info manifest table. It contains the dependencies of the given package and their licenses. This information is useful when assessing license compatibility of the packages and their libraries.
Quoted 8 lines
An example of the content of the new column for the MPD package is shown below:
    "alsa-lib [LGPL-2.1+ (library), GPL-2.0+ (aserver)], boost
    [BSL-1.0], libid3tag [GPL-2.0+], libmad [GPL-2.0+], libogg
    [BSD-3-Clause], libvorbis [BSD-3-Clause], libzlib [Zlib],
    skeleton-init-common [unknown], skeleton-init-sysv [unknown],
    sqlite [Public domain], toolchain-external-linaro-arm [unknown], "
I believe this is a very good addition to the manifest. Good idea! :-)
The trailing comma is ugly, though. I would just drop the coma altogether...
And here, I have two spaces between each packages:

[...]

Quoted 17 lines

package/pkg-utils.mk define legal-warning-nosource # pkg, {local|override} · line 84

Quoted 13 lines

package/pkg-utils.mk +2 -2

index c3acc22b17..f7a3609443 100644
@@ -79,8 +79,8 @@ define legal-warning-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, url, {HOST|TARGET}
-	echo '"$(1)","$(2)","$(3)","$(4)","$(5)","$(6)"' >>$(LEGAL_MANIFEST_CSV_$(7))
+define legal-manifest # pkg, version, license, license-files, source, url, {HOST|TARGET}, dependencies
+	echo '"$(1)","$(2)","$(3)","$(4)","$(5)","$(6)","$(8)"' >>$(LEGAL_MANIFEST_CSV_$(7))
 endef

This change would have been (slightly) easier to review if the order of parameters was changed.

Quoted 6 lines

In fact, I even believe a beter order would be:
{HOST|TARGET}, pkg, version, license, license-files, source, url, dependencies
(if you decide to go with this change too, then make it a separate patch that comes first.)

Yes, I think that this order is more natural.

Quoted 6 lines

package/pkg-utils.mk +1 -0

 define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-fullpath, {HOST|TARGET}

@@ -95,3 +95,22 @@ define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full
 	} && \
 	cp $(5) $(LICENSE_FILES_DIR_$(6))/$(2)/$(4)
 endef
+

Quoted 8 lines

package/pkg-utils.mk define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full · line 100
Quoted 2 lines
package/pkg-utils.mk +2 -0
+remove-virtual-pkgs = $(foreach p,$(1),$(if $($(call UPPERCASE,$(p))_IS_VIRTUAL),,$(p)))
+get-direct-deps = $(sort $(foreach p,$(1),$($(call UPPERCASE,$(p))_FINAL_DEPENDENCIES)))
This should be using $(2)_FINAL_ALL_DEPENDENCIES and not $(2)_FINAL_DEPENDENCIES, because the _EXTRACT_DEPENDENCIES and _PATCH_DEPENDENCIES can also be used to alter the content of the package. For example, we use _PATCH_DEPENDENCIES for the linux extensions, and those are changing the source code of the linux package, so their licensing terms also apply.

Quoted 14 lines

package/pkg-utils.mk define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full · line 105
Quoted 5 lines
package/pkg-utils.mk +5 -0
+define get-transitive-deps # packages
+	$(if $(filter-out $(1),$(call get-direct-deps,$(1))),\
+	     $(sort $(1) $(call get-transitive-deps,$(filter-out $(1),$(call get-direct-deps,$(1))))),\
+	     $(1))
+endef
What about this new variable instead:
package/pkg-generic.mk +0 -0
index a539483ae4..ca55ac5e8e 100644
@@ -609,6 +609,14 @@ $(2)_FINAL_ALL_DEPENDENCIES = \
 		$$($(2)_FINAL_DEPENDENCIES) \

package/pkg-generic.mk $(2)_FINAL_ALL_DEPENDENCIES = \ · line 619

Quoted 10 lines

package/pkg-generic.mk +8 -0

 		$$($(2)_FINAL_EXTRACT_DEPENDENCIES) \
 		$$($(2)_FINAL_PATCH_DEPENDENCIES))
+$(2)_FINAL_RECURSIVE_DEPENDENCIES = \
+	$$(sort \
+		$$(foreach p,\
+			$$($(2)_FINAL_ALL_DEPENDENCIES),\
+			$$(p)\
+			$$($$(call UPPERCASE,$$(p))_FINAL_RECURSIVE_DEPENDENCIES)\
+		)\
+	)

This looks nice. When I tried it, make printvars was about 4 times slower. I don't know whether it's a problem or not, but I use printvars quite often during debugging of various stuff. Of course, if I limit the variables with VARS=..., it is fast again.

Quoted 26 lines

package/pkg-generic.mk +0 -0
 $(2)_INSTALL_STAGING?= NO
 $(2)_INSTALL_IMAGES?= NO
 
package/pkg-utils.mk define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full · line 106
Quoted 1 line
package/pkg-utils.mk +1 -0
+non-virtual-deps = $(call remove-virtual-pkgs,$(filter-out $(1),$(call get-transitive-deps,$(1))))
And this new macro:
non-virtual-deps = $(foreach p,$(1),$(if $($(call UPPERCASE,$(p))_IS_VIRTUAL),,$(p)))
package/pkg-utils.mk define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full · line 107
Quoted 1 line
package/pkg-utils.mk +1 -0
+host-pattern-if-target-pkg = $(if $(1:host-%=),host-%,)
I am not sure I grok that one... Oh, OK I got it. I don't think we need an intermediate variable (partly because its name is longer than the code it replaces...)
package/pkg-utils.mk define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full · line 113
Quoted 6 lines
package/pkg-utils.mk +6 -0
+# Produce a comma-separated list of dependent packages and their
+# licenses. Host packages are removed from the list if the argument is
+# not a host package.
+define legal-deps # package
+$(foreach p,$(filter-out $(call host-pattern-if-target-pkg,$(1)),$(call non-virtual-deps,$(1))),$(p) [$($(call UPPERCASE,$(p))_LICENSE)], )
+endef
... and then:
# $1: package name, lowercase

Quoted 8 lines

legal-deps = \
    $(foreach p,\
        $(filter-out $(if $(1:host-%=),host-%),\
            $($(call UPPERCASE,$(1))_FINAL_RECURSIVE_DEPENDENCIES)),\
        $(p) [$($(call UPPERCASE,$(p))_LICENSE)]\
    )

This is totally untested, but owrth investigating I think. ;-)

Yes. It also produces double spaces. I managed to fix that only by putting the last three lines on a single line.

I'll send v3 with the current state.

-Michal

Michal Sojka

author

2018-08-20 14:12 UTC #5 lore raw

PATCH v3 1/2 core/legal-info: Change order of legal-manifest parameters

From: Michal Sojka <[email protected]>

The last parameter {HOST|TARGET} is now first. With this change, adding new columns to the legal manifest file (as in the next commit) will be slightly easier to review.

Signed-off-by Michal Sojka <[email protected]>

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

Makefile +3 -3

index f79d39fd26..3d8106e201 100644
@@ -781,9 +781,9 @@ legal-info-clean:
 legal-info-prepare: $(LEGAL_INFO_DIR)
 	@$(call MESSAGE,"Buildroot $(BR2_VERSION_FULL) Collecting legal info")
 	@$(call legal-license-file,buildroot,buildroot,support/legal-info,COPYING,COPYING,HOST)
-	@$(call legal-manifest,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE,TARGET)
-	@$(call legal-manifest,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE,HOST)
-	@$(call legal-manifest,buildroot,$(BR2_VERSION_FULL),GPL-2.0+,COPYING,not saved,not saved,HOST)
+	@$(call legal-manifest,TARGET,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE)
+	@$(call legal-manifest,HOST,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE)
+	@$(call legal-manifest,HOST,buildroot,$(BR2_VERSION_FULL),GPL-2.0+,COPYING,not saved,not saved)
 	@$(call legal-warning,the Buildroot source code has not been saved)
 	@cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config

package/pkg-generic.mk +1 -1

index 91b61c6de0..6c9592123d 100644
@@ -936,7 +936,7 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 endif # redistribute
 
 endif # other packages
-	@$$(call legal-manifest,$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call UPPERCASE,$(4)))
+	@$$(call legal-manifest,$$(call UPPERCASE,$(4)),$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE))
 endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
 	$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))

package/pkg-utils.mk +2 -2

index c3acc22b17..8d5ea234e4 100644
@@ -79,8 +79,8 @@ define legal-warning-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, url, {HOST|TARGET}
-	echo '"$(1)","$(2)","$(3)","$(4)","$(5)","$(6)"' >>$(LEGAL_MANIFEST_CSV_$(7))
+define legal-manifest # {HOST|TARGET}, pkg, version, license, license-files, source, url
+	echo '"$(2)","$(3)","$(4)","$(5)","$(6)","$(7)"' >>$(LEGAL_MANIFEST_CSV_$(1))
 endef
 
 define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-fullpath, {HOST|TARGET}

Signature · 1 line

2.18.0

Michal Sojka

author

2018-08-20 14:12 UTC #6 lore raw

PATCH v3 2/2 core/legal-info: Add package dependencies with licenses to the manifest

From: Michal Sojka <[email protected]>

An example of the content of the new column for the MPD package is shown below:

"alsa-lib [LGPL-2.1+ (library), GPL-2.0+ (aserver)] boost [BSL-1.0] libid3tag [GPL-2.0+] libmad [GPL-2.0+] libogg [BSD-3-Clause] libvorbis [BSD-3-Clause] libzlib [Zlib] skeleton-init-common [unknown] skeleton-init-sysv [unknown] sqlite [Public domain] toolchain-external-linaro-arm [unknown]"

[Credits to Yann E. MORIN <[email protected]> for suggesting a few simplifications.]

Signed-off-by Michal Sojka <[email protected]>

---
 Makefile               |  4 ++--
 package/pkg-generic.mk | 10 +++++++++-
 package/pkg-utils.mk   | 13 +++++++++++--
 3 files changed, 22 insertions(+), 5 deletions(-)

Makefile +2 -2

index 3d8106e201..d6809642bd 100644
@@ -781,8 +781,8 @@ legal-info-clean:
 legal-info-prepare: $(LEGAL_INFO_DIR)
 	@$(call MESSAGE,"Buildroot $(BR2_VERSION_FULL) Collecting legal info")
 	@$(call legal-license-file,buildroot,buildroot,support/legal-info,COPYING,COPYING,HOST)
-	@$(call legal-manifest,TARGET,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE)
-	@$(call legal-manifest,HOST,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE)
+	@$(call legal-manifest,TARGET,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE,DEPENDENCIES WITH LICENSES)
+	@$(call legal-manifest,HOST,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE,DEPENDENCIES WITH LICENSES)
 	@$(call legal-manifest,HOST,buildroot,$(BR2_VERSION_FULL),GPL-2.0+,COPYING,not saved,not saved)
 	@$(call legal-warning,the Buildroot source code has not been saved)
 	@cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config

package/pkg-generic.mk +9 -1

index 6c9592123d..a3bedb660f 100644
@@ -609,6 +609,14 @@ $(2)_FINAL_ALL_DEPENDENCIES = \
 		$$($(2)_FINAL_DEPENDENCIES) \
 		$$($(2)_FINAL_EXTRACT_DEPENDENCIES) \
 		$$($(2)_FINAL_PATCH_DEPENDENCIES))
+$(2)_FINAL_RECURSIVE_DEPENDENCIES = \
+	$$(sort \
+		$$(foreach p,\
+			$$($(2)_FINAL_ALL_DEPENDENCIES),\
+			$$(p)\
+			$$($$(call UPPERCASE,$$(p))_FINAL_RECURSIVE_DEPENDENCIES)\
+		)\
+	)
 
 $(2)_INSTALL_STAGING		?= NO
 $(2)_INSTALL_IMAGES		?= NO

@@ -936,7 +944,7 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 endif # redistribute
 
 endif # other packages
-	@$$(call legal-manifest,$$(call UPPERCASE,$(4)),$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE))
+	@$$(call legal-manifest,$$(call UPPERCASE,$(4)),$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call legal-deps,$(1)))
 endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
 	$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))

package/pkg-utils.mk +11 -2

index 8d5ea234e4..8add57cf32 100644
@@ -79,8 +79,8 @@ define legal-warning-nosource # pkg, {local|override}
 	$(call legal-warning-pkg,$(1),sources not saved ($(2) packages not handled))
 endef
 
-define legal-manifest # {HOST|TARGET}, pkg, version, license, license-files, source, url
-	echo '"$(2)","$(3)","$(4)","$(5)","$(6)","$(7)"' >>$(LEGAL_MANIFEST_CSV_$(1))
+define legal-manifest # {HOST|TARGET}, pkg, version, license, license-files, source, url, dependencies
+	echo '"$(2)","$(3)","$(4)","$(5)","$(6)","$(7)","$(8)"' >>$(LEGAL_MANIFEST_CSV_$(1))
 endef
 
 define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-fullpath, {HOST|TARGET}

@@ -95,3 +95,12 @@ define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full
 	} && \
 	cp $(5) $(LICENSE_FILES_DIR_$(6))/$(2)/$(4)
 endef
+
+non-virtual-deps = $(foreach p,$(1),$(if $($(call UPPERCASE,$(p))_IS_VIRTUAL),,$(p)))
+
+# $1: package name, lowercase
+legal-deps = \
+    $(foreach p,\
+        $(filter-out $(if $(1:host-%=),host-%),\
+            $(call non-virtual-deps,\
+                $($(call UPPERCASE,$(1))_FINAL_RECURSIVE_DEPENDENCIES))),$(p) [$($(call UPPERCASE,$(p))_LICENSE)])

Signature · 1 line

2.18.0

Yann E. MORIN

2018-08-20 14:59 UTC #7 lore raw

PATCH v3 2/2 core/legal-info: Add package dependencies with licenses to the manifest

Michal, All,

On 2018-08-20 16:12 +0200, Michal Sojka spake thusly:

Quoted 16 lines

From: Michal Sojka <[email protected]>
This adds one column to the legal-info manifest table. It contains the dependencies of the given package and their licenses. This information is useful when assessing license compatibility of the packages and their libraries.
An example of the content of the new column for the MPD package is shown below:
"alsa-lib [LGPL-2.1+ (library), GPL-2.0+ (aserver)] boost [BSL-1.0] libid3tag [GPL-2.0+] libmad [GPL-2.0+] libogg [BSD-3-Clause] libvorbis [BSD-3-Clause] libzlib [Zlib] skeleton-init-common [unknown] skeleton-init-sysv [unknown] sqlite [Public domain] toolchain-external-linaro-arm [unknown]"

Quoted 4 lines

[Credits to Yann E. MORIN <[email protected]> for suggesting a few simplifications.]
Signed-off-by Michal Sojka <[email protected]>

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

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

Yet, a little comment, see below...

[--SNIP--]

package/pkg-utils.mk

Quoted 4 lines

package/pkg-utils.mk +0 -0
index 8d5ea234e4..8add57cf32 100644

[--SNIP--]

package/pkg-utils.mk define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full · line 102

Quoted 9 lines

package/pkg-utils.mk +5 -0

@@ -95,3 +95,12 @@ define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-full
 	} && \
 	cp $(5) $(LICENSE_FILES_DIR_$(6))/$(2)/$(4)
 endef
+
+non-virtual-deps = $(foreach p,$(1),$(if $($(call UPPERCASE,$(p))_IS_VIRTUAL),,$(p)))
+
+# $1: package name, lowercase
+legal-deps = \

It is not directly obvious what this macro does, so maybe a little bit of comment would be zelcome, something alomng the lines of:

# Returns the recursive list of dependencies and their licensing terms # for the package specified in parameter. If that package is a target # package, remove host packages from the list.

Regards,
Yann E. MORIN.

Quoted 7 lines

package/pkg-utils.mk +4 -0

+    $(foreach p,\
+        $(filter-out $(if $(1:host-%=),host-%),\
+            $(call non-virtual-deps,\
+                $($(call UPPERCASE,$(1))_FINAL_RECURSIVE_DEPENDENCIES))),$(p) [$($(call UPPERCASE,$(p))_LICENSE)])

Signature · 1 line

2.18.0

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

Yann E. MORIN

2018-08-20 14:44 UTC #8 lore raw

PATCH v3 1/2 core/legal-info: Change order of legal-manifest parameters

In reply to Michal Sojka

Michal, All,

On 2018-08-20 16:12 +0200, Michal Sojka spake thusly:

Quoted 7 lines

From: Michal Sojka <[email protected]>
The last parameter {HOST|TARGET} is now first. With this change, adding new columns to the legal manifest file (as in the next commit) will be slightly easier to review.
Signed-off-by Michal Sojka <[email protected]>

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

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

Regards,
Yann E. MORIN.

Quoted 54 lines

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

Makefile +3 -3

index f79d39fd26..3d8106e201 100644
@@ -781,9 +781,9 @@ legal-info-clean:
 legal-info-prepare: $(LEGAL_INFO_DIR)
 	@$(call MESSAGE,"Buildroot $(BR2_VERSION_FULL) Collecting legal info")
 	@$(call legal-license-file,buildroot,buildroot,support/legal-info,COPYING,COPYING,HOST)
-	@$(call legal-manifest,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE,TARGET)
-	@$(call legal-manifest,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE,HOST)
-	@$(call legal-manifest,buildroot,$(BR2_VERSION_FULL),GPL-2.0+,COPYING,not saved,not saved,HOST)
+	@$(call legal-manifest,TARGET,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE)
+	@$(call legal-manifest,HOST,PACKAGE,VERSION,LICENSE,LICENSE FILES,SOURCE ARCHIVE,SOURCE SITE)
+	@$(call legal-manifest,HOST,buildroot,$(BR2_VERSION_FULL),GPL-2.0+,COPYING,not saved,not saved)
 	@$(call legal-warning,the Buildroot source code has not been saved)
 	@cp $(BR2_CONFIG) $(LEGAL_INFO_DIR)/buildroot.config

package/pkg-generic.mk +1 -1

index 91b61c6de0..6c9592123d 100644
@@ -936,7 +936,7 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 endif # redistribute
 
 endif # other packages
-	@$$(call legal-manifest,$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE),$$(call UPPERCASE,$(4)))
+	@$$(call legal-manifest,$$(call UPPERCASE,$(4)),$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_ACTUAL_SOURCE_SITE))
 endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
 	$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))

package/pkg-utils.mk +2 -2

index c3acc22b17..8d5ea234e4 100644
@@ -79,8 +79,8 @@ define legal-warning-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, url, {HOST|TARGET}
-	echo '"$(1)","$(2)","$(3)","$(4)","$(5)","$(6)"' >>$(LEGAL_MANIFEST_CSV_$(7))
+define legal-manifest # {HOST|TARGET}, pkg, version, license, license-files, source, url
+	echo '"$(2)","$(3)","$(4)","$(5)","$(6)","$(7)"' >>$(LEGAL_MANIFEST_CSV_$(1))
 endef
 
 define legal-license-file # pkgname, pkgname-pkgver, pkgdir, filename, file-fullpath, {HOST|TARGET}

Signature · 1 line

2.18.0

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