[Buildroot] [PATCH 0/2] Tainting support

[Buildroot] [PATCH 0/2] Tainting support

[Angelo Compagnucci]

Packages that need to resolve dependencies internally
and use a package manager would harm the reproducibility
of a build, moreover they escape the legal infrastructure
not giving enough informations on licensing.

This patch adds a tainting mechanism in the form of a
variable FOO_TAINTS that can be used to signal that
a package harms the reproducibility under certain
conditions.

This variable is later used to check if the build is
tainted or not. The build then aborts with an error
when the build is marked tainted and BR2_REPRODUCIBLE is
enabled.
 
This opens the door to include per language dependency
managers in buildroot.

Angelo Compagnucci (2):
  Makefile: add tainting support
  package/nodejs: taint the build on external modules

 Makefile                 | 14 +++++++++++++-
 package/nodejs/nodejs.mk |  1 +
 package/pkg-generic.mk   |  9 +++++++++
 3 files changed, 23 insertions(+), 1 deletion(-)

-- 
2.7.4

[Buildroot] [PATCH 1/2] Makefile: add tainting support

[Angelo Compagnucci]

From: Angelo Compagnucci <angelo.compagnucci@gmail.com>

Packages who harms the build reproducibility can declare
FOO_TAINTS variable.
If a package taints the build it will be added to a list
of tainting packages.
The build ends with an error when the BR2_REPRODUCIBLE
is enabled and the tainting packages list is not empty.
Moreover, legal info will show a warning in presence
of a tainting package.

Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
---
 Makefile               | 14 +++++++++++++-
 package/pkg-generic.mk |  9 +++++++++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 1b34818..afa5f29 100644
--- a/Makefile
+++ b/Makefile
@@ -761,8 +761,19 @@ endif
 		$(call MESSAGE,"Executing post-build script $(s)"); \
 		$(EXTRA_ENV) $(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
 
+# Check here if there are packages declaring they harm
+# the reproducibility of the build
+.PHONY: check-tainted
+check-tainted:
+ifneq ($(BR2_REPRODUCIBLE),)
+ifneq ($(BR2_TAINTED_BY),)
+	$(error Buildroot is tainted (by: $(BR2_TAINTED_BY)). Cannot do a reproducible build.)
+	@exit 1
+endif
+endif
+
 .PHONY: target-post-image
-target-post-image: $(TARGETS_ROOTFS) target-finalize
+target-post-image: check-tainted $(TARGETS_ROOTFS) target-finalize
 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
 		$(call MESSAGE,"Executing post-image script $(s)"); \
 		$(EXTRA_ENV) $(s) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
@@ -1063,6 +1074,7 @@ help:
 	@echo '  source                 - download all sources needed for offline-build'
 	@echo '  external-deps          - list external packages used'
 	@echo '  legal-info             - generate info about license compliance'
+	@echo '  check-tainted          - check if any selected package harms build reproducibility'
 	@echo '  printvars              - dump all the internal variables'
 	@echo
 	@echo '  make V=0|1             - 0 => quiet build (default), 1 => verbose build'
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index a2a12e7..e643ecf 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -556,6 +556,10 @@ ifndef $(2)_REDISTRIBUTE
  endif
 endif
 
+ifdef $(2)_TAINTS
+ BR2_TAINTED_BY+=$$($(2)_RAWNAME)
+endif
+
 $(2)_REDISTRIBUTE		?= YES
 
 $(2)_REDIST_SOURCES_DIR = $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))/$$($(2)_RAW_BASE_NAME)
@@ -869,6 +873,11 @@ else
 	$(Q)$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME),$$($(2)_RAW_BASE_NAME),$$($(2)_PKGDIR),$$(F),$$($(2)_DIR)/$$(F),$$(call UPPERCASE,$(4)))$$(sep))
 endif # license files
 
+# Save a legal warning if tainted
+ifneq ($$(call qstrip,$$($(2)_TAINTS)),)
+	$(Q)$$(call legal-warning-pkg,$$($(2)_RAWNAME),unknown license for additional modules or dependencies)
+endif
+
 ifeq ($$($(2)_SITE_METHOD),local)
 # Packages without a tarball: don't save and warn
 	@$$(call legal-warning-nosource,$$($(2)_RAWNAME),local)
-- 
2.7.4

[Buildroot] [PATCH 1/2] Makefile: add tainting support

[Thomas Petazzoni]

Hello,

On Tue,  6 Mar 2018 09:48:04 +0100, Angelo Compagnucci wrote:

> +# Check here if there are packages declaring they harm
> +# the reproducibility of the build
> +.PHONY: check-tainted
> +check-tainted:
> +ifneq ($(BR2_REPRODUCIBLE),)
> +ifneq ($(BR2_TAINTED_BY),)

I don't think BR2_REPRODUCIBLE has anything to do with this 'tainting'
thing.

BR2_REPRODUCIBLE is about getting binary identical results given a
Buildroot configuration. But even with BR2_REPRODUCIBLE disabled,
Buildroot aims at building the same source code, which the nodejs npm
stuff does not allow to do. So the 'tainting' happens regardless of
whether BR2_REPRODUCIBLE=y or not.

If you remove the BR2_REPRODUCIBLE condition, then the Buildroot build
would always fail as soon as one package taints the build, so I'm not
sure a hard failure is a good approach here. Perhaps just a warning at
the end of the build ?

>  .PHONY: target-post-image
> -target-post-image: $(TARGETS_ROOTFS) target-finalize
> +target-post-image: check-tainted $(TARGETS_ROOTFS) target-finalize
>  	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
>  		$(call MESSAGE,"Executing post-image script $(s)"); \
>  		$(EXTRA_ENV) $(s) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
> @@ -1063,6 +1074,7 @@ help:
>  	@echo '  source                 - download all sources needed for offline-build'
>  	@echo '  external-deps          - list external packages used'
>  	@echo '  legal-info             - generate info about license compliance'
> +	@echo '  check-tainted          - check if any selected package harms build reproducibility'

Not sure it is really useful to make this a first class target,
documented in the help text. A warning at the end of the build is
probably enough.

Or maybe simply mention it as a Config.in comment ?

comment "WARNING: nodejs modules will make your build non reproducible!"
	depends on BR2_PACKAGE_NODEJS_MODULES_ADDITIONAL != ""

>  	@echo '  printvars              - dump all the internal variables'
>  	@echo
>  	@echo '  make V=0|1             - 0 => quiet build (default), 1 => verbose build'
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index a2a12e7..e643ecf 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -556,6 +556,10 @@ ifndef $(2)_REDISTRIBUTE
>   endif
>  endif
>  
> +ifdef $(2)_TAINTS
> + BR2_TAINTED_BY+=$$($(2)_RAWNAME)
> +endif

I think "TAINTS" is very vague. Unfortunately, I don't have a good
proposal. In addition, it's not only about tainting the build in terms
of reproducibility, but also about the lack of integration with the
download and legal infrastructures.

I'm not sure what a good name for this would be. I've added Arnout in
Cc, he often has interesting (and creative) ideas on such topics.

> +# Save a legal warning if tainted
> +ifneq ($$(call qstrip,$$($(2)_TAINTS)),)

Implementation detail: you always use the construct:

ifneq ($(var),)

which is negative logic. Please use:

ifeq ($(var),YES)

instead. Of course, unless the variable really is empty vs. non-empty,
but that isn't the case here.

Also, your patch series was sent at a time where patchwork was missing
some patches, so your PATCH 2/2 was not recorded. It would be useful if
you could resend this patch series.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/2] Makefile: add tainting support

[Angelo Compagnucci]

Dear Thomas,

2018-04-28 18:53 GMT+02:00 Thomas Petazzoni <thomas.petazzoni@bootlin.com>:
> Hello,
>
> On Tue,  6 Mar 2018 09:48:04 +0100, Angelo Compagnucci wrote:
>
>> +# Check here if there are packages declaring they harm
>> +# the reproducibility of the build
>> +.PHONY: check-tainted
>> +check-tainted:
>> +ifneq ($(BR2_REPRODUCIBLE),)
>> +ifneq ($(BR2_TAINTED_BY),)
>
> I don't think BR2_REPRODUCIBLE has anything to do with this 'tainting'
> thing.
>
> BR2_REPRODUCIBLE is about getting binary identical results given a
> Buildroot configuration. But even with BR2_REPRODUCIBLE disabled,
> Buildroot aims at building the same source code, which the nodejs npm
> stuff does not allow to do. So the 'tainting' happens regardless of
> whether BR2_REPRODUCIBLE=y or not.
>
> If you remove the BR2_REPRODUCIBLE condition, then the Buildroot build
> would always fail as soon as one package taints the build, so I'm not
> sure a hard failure is a good approach here. Perhaps just a warning at
> the end of the build ?

Fair enough, will do.

>
>>  .PHONY: target-post-image
>> -target-post-image: $(TARGETS_ROOTFS) target-finalize
>> +target-post-image: check-tainted $(TARGETS_ROOTFS) target-finalize
>>       @$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
>>               $(call MESSAGE,"Executing post-image script $(s)"); \
>>               $(EXTRA_ENV) $(s) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
>> @@ -1063,6 +1074,7 @@ help:
>>       @echo '  source                 - download all sources needed for offline-build'
>>       @echo '  external-deps          - list external packages used'
>>       @echo '  legal-info             - generate info about license compliance'
>> +     @echo '  check-tainted          - check if any selected package harms build reproducibility'
>
> Not sure it is really useful to make this a first class target,
> documented in the help text. A warning at the end of the build is
> probably enough.

I think this should be kept especially for CI systems in which it's
useful to know if a user introduced something that harms the build.

> Or maybe simply mention it as a Config.in comment ?
>
> comment "WARNING: nodejs modules will make your build non reproducible!"
>         depends on BR2_PACKAGE_NODEJS_MODULES_ADDITIONAL != ""
>
>>       @echo '  printvars              - dump all the internal variables'
>>       @echo
>>       @echo '  make V=0|1             - 0 => quiet build (default), 1 => verbose build'
>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>> index a2a12e7..e643ecf 100644
>> --- a/package/pkg-generic.mk
>> +++ b/package/pkg-generic.mk
>> @@ -556,6 +556,10 @@ ifndef $(2)_REDISTRIBUTE
>>   endif
>>  endif
>>
>> +ifdef $(2)_TAINTS
>> + BR2_TAINTED_BY+=$$($(2)_RAWNAME)
>> +endif
>
> I think "TAINTS" is very vague. Unfortunately, I don't have a good
> proposal. In addition, it's not only about tainting the build in terms
> of reproducibility, but also about the lack of integration with the
> download and legal infrastructures.

Yes, it's not the best name probably, but the concept is here.

>
> I'm not sure what a good name for this would be. I've added Arnout in
> Cc, he often has interesting (and creative) ideas on such topics.
>
>> +# Save a legal warning if tainted
>> +ifneq ($$(call qstrip,$$($(2)_TAINTS)),)
>
> Implementation detail: you always use the construct:
>
> ifneq ($(var),)
>
> which is negative logic. Please use:
>
> ifeq ($(var),YES)
>
> instead. Of course, unless the variable really is empty vs. non-empty,
> but that isn't the case here.
>
> Also, your patch series was sent at a time where patchwork was missing
> some patches, so your PATCH 2/2 was not recorded. It would be useful if
> you could resend this patch series.
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot



-- 
Profile: http://it.linkedin.com/in/compagnucciangelo

[Buildroot] [PATCH 1/2] Makefile: add tainting support

[Arnout Vandecappelle]

On 03-05-18 23:34, Angelo Compagnucci wrote:
> Dear Thomas,
> 
> 2018-04-28 18:53 GMT+02:00 Thomas Petazzoni <thomas.petazzoni@bootlin.com>:
>> Hello,
>>
>> On Tue,  6 Mar 2018 09:48:04 +0100, Angelo Compagnucci wrote:
[snip]
>>> @@ -1063,6 +1074,7 @@ help:
>>>       @echo '  source                 - download all sources needed for offline-build'
>>>       @echo '  external-deps          - list external packages used'
>>>       @echo '  legal-info             - generate info about license compliance'
>>> +     @echo '  check-tainted          - check if any selected package harms build reproducibility'
>>
>> Not sure it is really useful to make this a first class target,
>> documented in the help text. A warning at the end of the build is
>> probably enough.
> 
> I think this should be kept especially for CI systems in which it's
> useful to know if a user introduced something that harms the build.

 Yes, makes sense. Especially the way you did it in v2: non-fatal in the
target-finalize step, but fatal if check-tainted is called explicitly.

>> Or maybe simply mention it as a Config.in comment ?
>>
>> comment "WARNING: nodejs modules will make your build non reproducible!"
>>         depends on BR2_PACKAGE_NODEJS_MODULES_ADDITIONAL != ""
>>
>>>       @echo '  printvars              - dump all the internal variables'
>>>       @echo
>>>       @echo '  make V=0|1             - 0 => quiet build (default), 1 => verbose build'
>>> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
>>> index a2a12e7..e643ecf 100644
>>> --- a/package/pkg-generic.mk
>>> +++ b/package/pkg-generic.mk
>>> @@ -556,6 +556,10 @@ ifndef $(2)_REDISTRIBUTE
>>>   endif
>>>  endif
>>>
>>> +ifdef $(2)_TAINTS
>>> + BR2_TAINTED_BY+=$$($(2)_RAWNAME)
>>> +endif
>>
>> I think "TAINTS" is very vague. Unfortunately, I don't have a good
>> proposal. In addition, it's not only about tainting the build in terms
>> of reproducibility, but also about the lack of integration with the
>> download and legal infrastructures.
> 
> Yes, it's not the best name probably, but the concept is here.

 In the kernel the 'taint' name is also used for things that are not really
literally tainting, so why not.

 Regards,
 Arnout


>> I'm not sure what a good name for this would be. I've added Arnout in
>> Cc, he often has interesting (and creative) ideas on such topics.
[snip]

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH 2/2] package/nodejs: taint the build on external modules

[Angelo Compagnucci]

From: Angelo Compagnucci <angelo.compagnucci@gmail.com>

This patch enables the tainting of the build if when an
external module is added.

Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
---
 package/nodejs/nodejs.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/nodejs/nodejs.mk b/package/nodejs/nodejs.mk
index a72b9a7..9733701 100644
--- a/package/nodejs/nodejs.mk
+++ b/package/nodejs/nodejs.mk
@@ -160,6 +160,7 @@ NPM = $(TARGET_CONFIGURE_OPTS) \
 # We can only call NPM if there's something to install.
 #
 ifneq ($(NODEJS_MODULES_LIST),)
+NODEJS_TAINTS = YES
 define NODEJS_INSTALL_MODULES
 	# If you're having trouble with module installation, adding -d to the
 	# npm install call below and setting npm_config_rollback=false can both
-- 
2.7.4

[Buildroot] [PATCH 1/2] Makefile: add tainting support

[Angelo Compagnucci]

From: Angelo Compagnucci <angelo.compagnucci@gmail.com>

Packages who harms the build reproducibility can declare
FOO_TAINTS variable.
If a package taints the build it will be added to a list
of tainting packages.
The build ends with a warning if the tainting packages
list is not empty.
Moreover, legal info will show a warning in presence
of a tainting package.

Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
---
 Makefile               | 11 +++++++++++
 package/pkg-generic.mk |  9 +++++++++
 2 files changed, 20 insertions(+)

diff --git a/Makefile b/Makefile
index c024c65..1b3d987 100644
--- a/Makefile
+++ b/Makefile
@@ -758,12 +758,22 @@ endif
 
 	touch $(TARGET_DIR)/usr
 
+# Check here if there are packages declaring they harm
+# the reproducibility of the build
+.PHONY: check-tainted
+check-tainted:
+ifneq ($(BR2_TAINTED_BY),)
+	$(error Buildroot is tainted (by: $(BR2_TAINTED_BY)).)
+endif
+
 .PHONY: target-post-image
 target-post-image: $(TARGETS_ROOTFS) target-finalize
 	@rm -f $(ROOTFS_COMMON_TAR)
 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
 		$(call MESSAGE,"Executing post-image script $(s)"); \
 		$(EXTRA_ENV) $(s) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
+	@if [ ! -z "$(BR2_TAINTED_BY)" ]; then \
+		echo "WARNING: Buildroot is tainted (by: $(BR2_TAINTED_BY))"; fi
 
 .PHONY: source
 source: $(foreach p,$(PACKAGES),$(p)-all-source)
@@ -1070,6 +1080,7 @@ help:
 	@echo '  source                 - download all sources needed for offline-build'
 	@echo '  external-deps          - list external packages used'
 	@echo '  legal-info             - generate info about license compliance'
+	@echo '  check-tainted          - check if any selected package harms build reproducibility'
 	@echo '  printvars              - dump all the internal variables'
 	@echo
 	@echo '  make V=0|1             - 0 => quiet build (default), 1 => verbose build'
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index a303dc2..a71ed6a 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -542,6 +542,10 @@ ifndef $(2)_REDISTRIBUTE
  endif
 endif
 
+ifdef $(2)_TAINTS
+ BR2_TAINTED_BY+=$$($(2)_RAWNAME)
+endif
+
 $(2)_REDISTRIBUTE		?= YES
 
 $(2)_REDIST_SOURCES_DIR = $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))/$$($(2)_BASENAME_RAW)
@@ -900,6 +904,11 @@ else
 	$(Q)$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME),$$($(2)_BASENAME_RAW),$$($(2)_PKGDIR),$$(F),$$($(2)_DIR)/$$(F),$$(call UPPERCASE,$(4)))$$(sep))
 endif # license files
 
+# Save a legal warning if tainted
+ifeq ($$(call qstrip,$$($(2)_TAINTS)),YES)
+	$(Q)$$(call legal-warning-pkg,$$($(2)_RAWNAME),unknown license for additional modules or dependencies)
+endif
+
 ifeq ($$($(2)_SITE_METHOD),local)
 # Packages without a tarball: don't save and warn
 	@$$(call legal-warning-nosource,$$($(2)_RAWNAME),local)
-- 
2.7.4

[Buildroot] [PATCH 1/2] Makefile: add tainting support

[Arnout Vandecappelle]

 Hi Angelo,

 Some nits that were already present in v1 but I didn't review before...

On 03-05-18 23:34, Angelo Compagnucci wrote:
> From: Angelo Compagnucci <angelo.compagnucci@gmail.com>
> 
> Packages who harms the build reproducibility can declare
> FOO_TAINTS variable.
> If a package taints the build it will be added to a list
> of tainting packages.
> The build ends with a warning if the tainting packages
> list is not empty.
> Moreover, legal info will show a warning in presence
> of a tainting package.

 Commit message wrapping is a bit weird. Wrap at 72 columns.

> 
> Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
> ---
>  Makefile               | 11 +++++++++++
>  package/pkg-generic.mk |  9 +++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index c024c65..1b3d987 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -758,12 +758,22 @@ endif
>  
>  	touch $(TARGET_DIR)/usr
>  
> +# Check here if there are packages declaring they harm
> +# the reproducibility of the build

 As Thomas said, reproducibility is not the only issue here.

 Also, wrap at 80 columns.

 But really the comment is not necessary IMO.

> +.PHONY: check-tainted
> +check-tainted:
> +ifneq ($(BR2_TAINTED_BY),)
> +	$(error Buildroot is tainted (by: $(BR2_TAINTED_BY)).)
                ^^^^^^^^^ Your configuration

> +endif
> +
>  .PHONY: target-post-image
>  target-post-image: $(TARGETS_ROOTFS) target-finalize
>  	@rm -f $(ROOTFS_COMMON_TAR)
>  	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
>  		$(call MESSAGE,"Executing post-image script $(s)"); \
>  		$(EXTRA_ENV) $(s) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
> +	@if [ ! -z "$(BR2_TAINTED_BY)" ]; then \

 Personally I'd prefer $(if $(BR2_TAINTED_BY), at echo ...) here. But if you keep
the shell condition:

> +		echo "WARNING: Buildroot is tainted (by: $(BR2_TAINTED_BY))"; fi

 fi should be on a separate line.

>  
>  .PHONY: source
>  source: $(foreach p,$(PACKAGES),$(p)-all-source)
> @@ -1070,6 +1080,7 @@ help:
>  	@echo '  source                 - download all sources needed for offline-build'
>  	@echo '  external-deps          - list external packages used'
>  	@echo '  legal-info             - generate info about license compliance'
> +	@echo '  check-tainted          - check if any selected package harms build reproducibility'

 Not just reproducibility. but I don't really have a suggestion to improve that
isn't very long...

>  	@echo '  printvars              - dump all the internal variables'
>  	@echo
>  	@echo '  make V=0|1             - 0 => quiet build (default), 1 => verbose build'
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index a303dc2..a71ed6a 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -542,6 +542,10 @@ ifndef $(2)_REDISTRIBUTE
>   endif
>  endif
>  
> +ifdef $(2)_TAINTS
> + BR2_TAINTED_BY+=$$($(2)_RAWNAME)

 We normally don't indent inside make conditions.

> +endif
> +
>  $(2)_REDISTRIBUTE		?= YES
>  
>  $(2)_REDIST_SOURCES_DIR = $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))/$$($(2)_BASENAME_RAW)
> @@ -900,6 +904,11 @@ else
>  	$(Q)$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME),$$($(2)_BASENAME_RAW),$$($(2)_PKGDIR),$$(F),$$($(2)_DIR)/$$(F),$$(call UPPERCASE,$(4)))$$(sep))
>  endif # license files
>  
> +# Save a legal warning if tainted

 Comment is useless IMO, the code speaks for itself.

> +ifeq ($$(call qstrip,$$($(2)_TAINTS)),YES)
> +	$(Q)$$(call legal-warning-pkg,$$($(2)_RAWNAME),unknown license for additional modules or dependencies)

 Er, this explanation is very specific... I think we could also add taints if
e.g. a git (or github-helper) download doesn't specify a full sha1, because then
we're also not guaranteed to have reproducibility.

 So for the legal warning, I'd rather think of something separate.

 But actually, that can be changed if/when we actually do add taints for other
situations as well. So this bit is OK as it is for me.

 Regards,
 Arnout

> +endif
> +
>  ifeq ($$($(2)_SITE_METHOD),local)
>  # Packages without a tarball: don't save and warn
>  	@$$(call legal-warning-nosource,$$($(2)_RAWNAME),local)
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF