* [Buildroot] [git commit] legal info: fix saving of host package licenses
@ 2013-11-12 22:20 Peter Korsgaard
2013-11-13 7:34 ` Thomas De Schampheleire
0 siblings, 1 reply; 6+ messages in thread
From: Peter Korsgaard @ 2013-11-12 22:20 UTC (permalink / raw)
To: buildroot
commit: http://git.buildroot.net/buildroot/commit/?id=003d38da3a716b0a19e980fb69a6cd55b353f4f3
branch: http://git.buildroot.net/buildroot/commit/?id=refs/heads/master
Due to some tricky make behavior, the license texts of host packages that
did not provide an explicit HOST_FOO_LICENSE_FILES definition was not saved.
The problem is that it is not straightforward to use a variable
defined/updated inside an evaluated block as input to a foreach statement.
If you try to use $(FOO) then only the original value of FOO is used for
foreach, any update inside the block is ignored. However, if you use
$$(FOO), the entire contents of FOO (typically a list of items) is passed
as one item to foreach, thus causing just one iteration instead of several.
From Arnout Vandecapelle's explanation:
Any variable referenced with a single $ inside the inner-generic-package
macro is expanded before the resulting contents are eval'ed. Therefore, it
is not possible to refer to variables defined by the inner-generic-package
macro from within a single-$ function call.
To fix the problem, one should defer the evaluation of the entire block
using double dollar signs.
Additionally, a few empty lines have been added to the legal-info-foo block
for clarity.
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
package/pkg-generic.mk | 17 +++++++++++++----
1 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 75fda02..5111f31 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -502,26 +502,35 @@ $(2)_MANIFEST_TARBALL ?= not saved
$(1)-legal-info:
# Packages without a source are assumed to be part of Buildroot, skip them.
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
ifeq ($(call qstrip,$$($(2)_LICENSE_FILES)),)
@$(call legal-license-nofiles,$$($(2)_RAWNAME))
@$(call legal-warning-pkg,$$($(2)_RAWNAME),cannot save license ($(2)_LICENSE_FILES not defined))
else
- @$(foreach F,$($(2)_LICENSE_FILES),$(call legal-license-file,$$($(2)_RAWNAME),$(F),$$($(2)_DIR)/$(F))$$(sep))
-endif
+# 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))$$(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) 2>/dev/null || \
cp $(DL_DIR)/$$($(2)_SOURCE) $(REDIST_SOURCES_DIR)
-endif
-endif
+endif # redistribute
+
+endif # other packages
@$(call legal-manifest,$$($(2)_RAWNAME),$$($(2)_VERSION),$$($(2)_LICENSE),$$($(2)_MANIFEST_LICENSE_FILES),$$($(2)_MANIFEST_TARBALL))
endif # ifneq ($(call qstrip,$$($(2)_SOURCE)),)
$(foreach hook,$($(2)_POST_LEGAL_INFO_HOOKS),$(call $(hook))$(sep))
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [git commit] legal info: fix saving of host package licenses
2013-11-12 22:20 [Buildroot] [git commit] legal info: fix saving of host package licenses Peter Korsgaard
@ 2013-11-13 7:34 ` Thomas De Schampheleire
2013-11-13 7:45 ` Thomas Petazzoni
0 siblings, 1 reply; 6+ messages in thread
From: Thomas De Schampheleire @ 2013-11-13 7:34 UTC (permalink / raw)
To: buildroot
Hi Peter,
On Tue, Nov 12, 2013 at 11:20 PM, Peter Korsgaard <peter@korsgaard.com> wrote:
> commit: http://git.buildroot.net/buildroot/commit/?id=003d38da3a716b0a19e980fb69a6cd55b353f4f3
> branch: http://git.buildroot.net/buildroot/commit/?id=refs/heads/master
>
> Due to some tricky make behavior, the license texts of host packages that
> did not provide an explicit HOST_FOO_LICENSE_FILES definition was not saved.
> The problem is that it is not straightforward to use a variable
> defined/updated inside an evaluated block as input to a foreach statement.
> If you try to use $(FOO) then only the original value of FOO is used for
> foreach, any update inside the block is ignored. However, if you use
> $$(FOO), the entire contents of FOO (typically a list of items) is passed
> as one item to foreach, thus causing just one iteration instead of several.
>
> >From Arnout Vandecapelle's explanation:
> Any variable referenced with a single $ inside the inner-generic-package
> macro is expanded before the resulting contents are eval'ed. Therefore, it
> is not possible to refer to variables defined by the inner-generic-package
> macro from within a single-$ function call.
>
> To fix the problem, one should defer the evaluation of the entire block
> using double dollar signs.
>
> Additionally, a few empty lines have been added to the legal-info-foo block
> for clarity.
>
> Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
> Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
> Tested-by: Luca Ceresoli <luca@lucaceresoli.net>
> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
Is there a particular reason why the 5 other patches in this series
(splitting the legal info output between host/target) have not been
applied? I think they still fit in 2013.11...
Best regards,
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [git commit] legal info: fix saving of host package licenses
2013-11-13 7:34 ` Thomas De Schampheleire
@ 2013-11-13 7:45 ` Thomas Petazzoni
2013-11-13 8:12 ` Peter Korsgaard
2013-11-13 9:16 ` Thomas De Schampheleire
0 siblings, 2 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2013-11-13 7:45 UTC (permalink / raw)
To: buildroot
Dear Thomas De Schampheleire,
On Wed, 13 Nov 2013 08:34:22 +0100, Thomas De Schampheleire wrote:
> Is there a particular reason why the 5 other patches in this series
> (splitting the legal info output between host/target) have not been
> applied? I think they still fit in 2013.11...
I believe PATCH 1 was considered as being a fix, while the other
patches are adding a new feature, and it was too late in the cycle to
add them.
We were already November, 13th, that is 13th days late than the normal
-rc1 date, so I definitely support Peter in not having merged all the
possible pending features in 2013.11.
That's all what time-based releases are for: stopping merging new
features at some point, knowing that they will anyway be merged in
-next pretty soon and be part of the following release 3 months later.
Best regards,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [git commit] legal info: fix saving of host package licenses
2013-11-13 7:45 ` Thomas Petazzoni
@ 2013-11-13 8:12 ` Peter Korsgaard
2013-11-13 9:16 ` Thomas De Schampheleire
1 sibling, 0 replies; 6+ messages in thread
From: Peter Korsgaard @ 2013-11-13 8:12 UTC (permalink / raw)
To: buildroot
>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
> Dear Thomas De Schampheleire,
> On Wed, 13 Nov 2013 08:34:22 +0100, Thomas De Schampheleire wrote:
>> Is there a particular reason why the 5 other patches in this series
>> (splitting the legal info output between host/target) have not been
>> applied? I think they still fit in 2013.11...
> I believe PATCH 1 was considered as being a fix, while the other
> patches are adding a new feature, and it was too late in the cycle to
> add them.
> We were already November, 13th, that is 13th days late than the normal
> -rc1 date, so I definitely support Peter in not having merged all the
> possible pending features in 2013.11.
Yes, you guys keep posting good stuff, but I have to end somewhere, and
-rc1 was very much overdue.
I will soon apply the rest of the series to next.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [git commit] legal info: fix saving of host package licenses
2013-11-13 7:45 ` Thomas Petazzoni
2013-11-13 8:12 ` Peter Korsgaard
@ 2013-11-13 9:16 ` Thomas De Schampheleire
2013-11-13 9:45 ` Thomas Petazzoni
1 sibling, 1 reply; 6+ messages in thread
From: Thomas De Schampheleire @ 2013-11-13 9:16 UTC (permalink / raw)
To: buildroot
Hi Thomas, Peter,
On Wed, Nov 13, 2013 at 8:45 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Dear Thomas De Schampheleire,
>
> On Wed, 13 Nov 2013 08:34:22 +0100, Thomas De Schampheleire wrote:
>
>> Is there a particular reason why the 5 other patches in this series
>> (splitting the legal info output between host/target) have not been
>> applied? I think they still fit in 2013.11...
>
> I believe PATCH 1 was considered as being a fix, while the other
> patches are adding a new feature, and it was too late in the cycle to
> add them.
Well, these patches have been around for much longer than this month,
even long before the buildroot developer days (during which I
highlighted the series on framapad). The core patches had been acked
and tested by Luca. The only changed patch was the first one after
Arnout's helpful comments after my ping this week.
>
> We were already November, 13th, that is 13th days late than the normal
> -rc1 date, so I definitely support Peter in not having merged all the
> possible pending features in 2013.11.
>
> That's all what time-based releases are for: stopping merging new
> features at some point, knowing that they will anyway be merged in
> -next pretty soon and be part of the following release 3 months later.
While I understand and agree with this principle for new patches, I
think that patches that have just been waiting to be merged are a
little different, unless there is an expected risk. Version bumps
clearly add risk, but in my opinion the proposed patches do not.
Anyway, it's not that big of a deal. Let's focus now on stabilization
and autobuild fixes.
Best regards,
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [git commit] legal info: fix saving of host package licenses
2013-11-13 9:16 ` Thomas De Schampheleire
@ 2013-11-13 9:45 ` Thomas Petazzoni
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2013-11-13 9:45 UTC (permalink / raw)
To: buildroot
Dear Thomas De Schampheleire,
On Wed, 13 Nov 2013 10:16:49 +0100, Thomas De Schampheleire wrote:
> > We were already November, 13th, that is 13th days late than the normal
> > -rc1 date, so I definitely support Peter in not having merged all the
> > possible pending features in 2013.11.
> >
> > That's all what time-based releases are for: stopping merging new
> > features at some point, knowing that they will anyway be merged in
> > -next pretty soon and be part of the following release 3 months later.
>
> While I understand and agree with this principle for new patches, I
> think that patches that have just been waiting to be merged are a
> little different, unless there is an expected risk. Version bumps
> clearly add risk, but in my opinion the proposed patches do not.
Well the proposed patches touch the core package infrastructure, which
to me adds more risk than a version bump that may break an isolated
package. I guess it's a matter of perception :)
> Anyway, it's not that big of a deal. Let's focus now on stabilization
> and autobuild fixes.
Agreed :)
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-11-13 9:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-12 22:20 [Buildroot] [git commit] legal info: fix saving of host package licenses Peter Korsgaard
2013-11-13 7:34 ` Thomas De Schampheleire
2013-11-13 7:45 ` Thomas Petazzoni
2013-11-13 8:12 ` Peter Korsgaard
2013-11-13 9:16 ` Thomas De Schampheleire
2013-11-13 9:45 ` Thomas Petazzoni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox