legal-info improvements and completeness (branch yem/legal-2)

Cc Luca Ceresoli <[email protected]>

---
Changes v1 -> v2:
- slight rephrasing (Luca)
- move early in the series (Luca)

---
 support/legal-info/README.header | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

support/legal-info/README.header +5 -5

index 3321adb..d07c45d 100644
@@ -12,12 +12,12 @@ This material is composed of the following items.
  * The Buildroot configuration file; this has been saved in buildroot.config.
  * The toolchain (cross-compiler and related tools) used to generate all the
    compiled programs.
-   Note: this has not been saved due to technical limitations, you must
-   collect it manually.
+   Note: this may have not been saved due to technical limitations, you may
+   need to collect it manually.
  * The source code for all packages; this has been saved in the sources/
-   subdirectory (except for the proprietary packages, which have not been
-   saved); patches applied to some packages by Buildroot are included in the
-   Buildroot sources and were not duplicated in the sources/ subdirectory.
+   subdirectory (except for the non-redistributable packages, which have not
+   been saved); patches applied to some packages by Buildroot are included in
+   the Buildroot sources and were not duplicated in the sources/ subdirectory.
  * A manifest file listing the configured packages and related information.
  * The license text of the packages; they have been saved in the licenses/
    subdirectory.

Signature · 1 line

1.9.1

Arnout Vandecappelle

2015-12-16 22:20 UTC #2 lore raw

PATCH v2 1/13 core/legal-info: update the legal-info report header

Quoted 7 lines · Yann E. MORIN

In some cases, the toolchain sources are now recovered and available in the legal-info output. So, adapt the header to use conditional instead of an definitive negation.
Also update the part about saving the sources: it's not the license list that defines whether sources are installed, but rather whether the package is reditributable or not.

redistributable

Quoted 2 lines

Update the header acoordingly.

accordingly

Quoted 4 lines

Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Acked-by Luca Ceresoli <[email protected]>

Acked-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

This can go in regardless of the rest of the series.

Regards,
Arnout

Quoted 32 lines

---
Changes v1 -> v2:
- slight rephrasing (Luca)
- move early in the series (Luca)

---
 support/legal-info/README.header | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

support/legal-info/README.header +5 -5

index 3321adb..d07c45d 100644
@@ -12,12 +12,12 @@ This material is composed of the following items.
  * The Buildroot configuration file; this has been saved in buildroot.config.
  * The toolchain (cross-compiler and related tools) used to generate all the
    compiled programs.
-   Note: this has not been saved due to technical limitations, you must
-   collect it manually.
+   Note: this may have not been saved due to technical limitations, you may
+   need to collect it manually.
  * The source code for all packages; this has been saved in the sources/
-   subdirectory (except for the proprietary packages, which have not been
-   saved); patches applied to some packages by Buildroot are included in the
-   Buildroot sources and were not duplicated in the sources/ subdirectory.
+   subdirectory (except for the non-redistributable packages, which have not
+   been saved); patches applied to some packages by Buildroot are included in
+   the Buildroot sources and were not duplicated in the sources/ subdirectory.
  * A manifest file listing the configured packages and related information.
  * The license text of the packages; they have been saved in the licenses/
    subdirectory.

Signature · 6 lines

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

Yann E. MORIN

author

2015-12-13 18:35 UTC #3 lore raw

PATCH v2 2/13 core/pkg-utils: add macro to hardlink-or-copy

Cc Luca Ceresoli <[email protected]>

---
Changes RFC -> v1:
- move to pkg-utils (Luca)

---
 package/pkg-utils.mk | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

package/pkg-utils.mk +18 -0

index 44bd2c9..41799b0 100644
@@ -113,6 +113,24 @@ $$(error Package error: use $(2) instead of $(1). Please fix your .mk file)
 endif
 endef
 
+################################################################################
+# hardlink-copy -- hardlink source and destination if possible, otherwise
+# do a simple copy
+#
+# argument 1 is the source *file*
+# argument 2 is the destination *directory*
+#
+# example:
+#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir/)
+#
+# Note: we make that a single command, so se can:
+#  - use '$(Q)' in front of it and properly silence the whole macro,
+#  - use '|| exit 1' after it, so we can exit on error in compound commands.
+################################################################################
+define hardlink-copy
+	{ mkdir -p $(2) && { cp -l $(1) $(2) 2>/dev/null || cp $(1) $(2); } }
+endef
+
 #
 # legal-info helper functions
 #

Signature · 1 line

1.9.1

Arnout Vandecappelle

2015-12-16 22:21 UTC #4 lore raw

PATCH v2 2/13 core/pkg-utils: add macro to hardlink-or-copy

Quoted 4 lines · Yann E. MORIN

Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Reviewed-by Luca Ceresoli <[email protected]>
Tested-by Luca Ceresoli <[email protected]>

Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

Regards,
Arnout

Quoted 38 lines

---
Changes RFC -> v1:
- move to pkg-utils (Luca)

---
 package/pkg-utils.mk | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

package/pkg-utils.mk +18 -0

index 44bd2c9..41799b0 100644
@@ -113,6 +113,24 @@ $$(error Package error: use $(2) instead of $(1). Please fix your .mk file)
 endif
 endef
 
+################################################################################
+# hardlink-copy -- hardlink source and destination if possible, otherwise
+# do a simple copy
+#
+# argument 1 is the source *file*
+# argument 2 is the destination *directory*
+#
+# example:
+#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir/)
+#
+# Note: we make that a single command, so se can:
+#  - use '$(Q)' in front of it and properly silence the whole macro,
+#  - use '|| exit 1' after it, so we can exit on error in compound commands.
+################################################################################
+define hardlink-copy
+	{ mkdir -p $(2) && { cp -l $(1) $(2) 2>/dev/null || cp $(1) $(2); } }
+endef
+
 #
 # legal-info helper functions
 #

Signature · 6 lines

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

Yann E. MORIN

author

2015-12-13 18:35 UTC #5 lore raw

PATCH v2 3/13 core/legal-info: use the macro to install source archives

Cc Luca Ceresoli <[email protected]>

---
 package/pkg-generic.mk | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

package/pkg-generic.mk +4 -3

index 357a5c7..e69c970 100644
@@ -805,9 +805,10 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
 	$$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))
 endif
-# Copy the source tarball (just hardlink if possible)
-	@cp -l $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))) 2>/dev/null || \
-	    cp $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
+# Copy the source tarball
+	$$(Q)$$(call hardlink-copy,\
+		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
+		     $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))))
 endif # redistribute
 
 endif # other packages

Signature · 1 line

1.9.1

Arnout Vandecappelle

2015-12-16 22:23 UTC #6 lore raw

PATCH v2 3/13 core/legal-info: use the macro to install source archives

Quoted 4 lines · Yann E. MORIN

Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Reviewed-by Luca Ceresoli <[email protected]>
Tested-by Luca Ceresoli <[email protected]>

Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

There's a small functional change here as well: @ -> $Q

Regards,
Arnout

Quoted 23 lines

---
 package/pkg-generic.mk | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

package/pkg-generic.mk +4 -3

index 357a5c7..e69c970 100644
@@ -805,9 +805,10 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
 	$$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))
 endif
-# Copy the source tarball (just hardlink if possible)
-	@cp -l $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))) 2>/dev/null || \
-	    cp $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL) $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
+# Copy the source tarball
+	$$(Q)$$(call hardlink-copy,\
+		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
+		     $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))))
 endif # redistribute
 
 endif # other packages

Signature · 6 lines

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

Yann E. MORIN

author

2015-12-13 18:35 UTC #7 lore raw

PATCH v2 4/13 core/legal-info: ensure legal-info works in off-line mode

Almost all packages which are saved for legal-info have their source archives downloaded as part of 'make source', which makes an off-line build completely possible [0].

However, for the pre-configured external toolchains, the source tarball is different, as the main tarball is a binary package. And that source tarball is only downloaded during the legal-info phase, which makes it inconvenient for full off-line builds.

We fix that in two steps:

- first, we move the declaration of _ACTUAL_SOURCE_TARBALL and _SITE earlier in the pkg-generic.mk file, so we get those definitions at the time we define the -source and -all-source rules;

- second, we add a new rule, $(1)-legal-source which only $(1)-all-source depends on, so that we only download it for a top-level 'make source', not as part of the standard download mechanism (i.e. only what is really needed to build).

This way, we can do a complete [0] off-line build and are still able to generate legal-info, while at the same time we do not incur any download overhead during a simple build.

Also, we previously downloaded the _ACTUAL_SOURCE_TARBALL when it was not empty. However, since _ACTUAL_SOURCE_TARBALL default to the value of _SOURCE, it can not be empty when _SOURCE is not. Thus, we'd get a spurious report of a missing hash for the tarball, since it was not in a standard package rule (configure, build, install..) and thus would miss the PKG and PKGDIR variables to find the .hash file.

We fix that in this commit as well, by:

- setting PKG and PKGDIR just for the -legal-source rule;

- only downloading _ACTUAL_SOURCE_TARBALL if it is not empty *and* not the same as _SOURCE (to avoid a second report about the hash).

[0] Save for nodejs which invarriably wants to download stuff at build time. Sigh... :-( Fixing that is work for another time...

Cc Luca Ceresoli <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

---
Changes v1 -> v2:
- drop the 'redistribute == ignore', it does not exist yet

---
 package/pkg-generic.mk | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

package/pkg-generic.mk +26 ~~-10~~

index e69c970..832045e 100644
@@ -428,6 +428,14 @@ ifndef $(2)_SOURCE
  endif
 endif
 
+# If FOO_ACTUAL_SOURCE_TARBALL is explicitly defined, it means FOO_SOURCE is
+# indeed a binary (e.g. external toolchain) and FOO_ACTUAL_SOURCE_TARBALL/_SITE
+# point to the actual sources tarball. Use the actual sources for legal-info.
+# For most packages the FOO_SITE/FOO_SOURCE pair points to real source code,
+# so these are the defaults for FOO_ACTUAL_*.
+$(2)_ACTUAL_SOURCE_TARBALL ?= $$($(2)_SOURCE)
+$(2)_ACTUAL_SOURCE_SITE    ?= $$(call qstrip,$$($(2)_SITE))
+
 ifndef $(2)_PATCH
  ifdef $(3)_PATCH
   $(2)_PATCH = $$($(3)_PATCH)

@@ -627,6 +635,10 @@ $(1)-depends:		$$($(2)_FINAL_DEPENDENCIES)
 
 $(1)-source:		$$($(2)_TARGET_SOURCE)
 
+$(1)-all-source:	$(1)-legal-source
+
+$(1)-legal-source:	$(1)-source
+
 $(1)-source-check:
 	$$(foreach p,$$($(2)_ALL_DOWNLOADS),$$(call SOURCE_CHECK,$$(p))$$(sep))

@@ -764,13 +776,19 @@ endif
 endif
 endif
 
-# If FOO_ACTUAL_SOURCE_TARBALL is explicitly defined, it means FOO_SOURCE is
-# indeed a binary (e.g. external toolchain) and FOO_ACTUAL_SOURCE_TARBALL/_SITE
-# point to the actual sources tarball. Use the actual sources for legal-info.
-# For most packages the FOO_SITE/FOO_SOURCE pair points to real source code,
-# so these are the defaults for FOO_ACTUAL_*.
-$(2)_ACTUAL_SOURCE_TARBALL ?= $$($(2)_SOURCE)
-$(2)_ACTUAL_SOURCE_SITE    ?= $$(call qstrip,$$($(2)_SITE))
+# Download actual sources if they differ from the extracted sources
+# (e.g. for external toolchains)
+#
+# We need to provide PKG and PKGDIR, because there's no .stamp file for
+# the legal-info step.
+$(1)-legal-source:	PKG=$(2)
+$(1)-legal-source:	PKGDIR=$(pkgdir)
+$(1)-legal-source:
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),)
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
+	$$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))
+endif # actuall sources != sources
+endif # actuall source != ""
 
 # legal-info: produce legally relevant info.
 $(1)-legal-info:

@@ -802,9 +820,6 @@ else
 # Other packages
 
 ifeq ($$($(2)_REDISTRIBUTE),YES)
-ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
-	$$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))
-endif
 # Copy the source tarball
 	$$(Q)$$(call hardlink-copy,\
 		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\

@@ -907,6 +922,7 @@ endif
 	$(1)-show-depends \
 	$(1)-show-version \
 	$(1)-source \
+	$(1)-legal-source \
 	$(1)-source-check
 
 ifeq ($$(patsubst %/,ERROR,$$($(2)_SITE)),ERROR)

Signature · 1 line

1.9.1

Luca Ceresoli

2015-12-16 11:30 UTC #8 lore raw

PATCH v2 4/13 core/legal-info: ensure legal-info works in off-line mode

Dear Yann,

Quoted 38 lines · Yann E. MORIN

Almost all packages which are saved for legal-info have their source archives downloaded as part of 'make source', which makes an off-line build completely possible [0].
However, for the pre-configured external toolchains, the source tarball is different, as the main tarball is a binary package. And that source tarball is only downloaded during the legal-info phase, which makes it inconvenient for full off-line builds.
We fix that in two steps:
- first, we move the declaration of _ACTUAL_SOURCE_TARBALL and _SITE earlier in the pkg-generic.mk file, so we get those definitions at the time we define the -source and -all-source rules;
- second, we add a new rule, $(1)-legal-source which only $(1)-all-source depends on, so that we only download it for a top-level 'make source', not as part of the standard download mechanism (i.e. only what is really needed to build).
This way, we can do a complete [0] off-line build and are still able to generate legal-info, while at the same time we do not incur any download overhead during a simple build.
Also, we previously downloaded the _ACTUAL_SOURCE_TARBALL when it was not empty. However, since _ACTUAL_SOURCE_TARBALL default to the value of _SOURCE, it can not be empty when _SOURCE is not. Thus, we'd get a spurious report of a missing hash for the tarball, since it was not in a standard package rule (configure, build, install..) and thus would miss the PKG and PKGDIR variables to find the .hash file.
We fix that in this commit as well, by:
- setting PKG and PKGDIR just for the -legal-source rule;
- only downloading _ACTUAL_SOURCE_TARBALL if it is not empty *and* not the same as _SOURCE (to avoid a second report about the hash).

Quoted 7 lines

[0] Save for nodejs which invarriably wants to download stuff at build time. Sigh... :-( Fixing that is work for another time...
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Peter Korsgaard <[email protected]>

Thanks for the very detailed commit log message. It's very useful!

Just a couple of minor notes below.

[...]

package/pkg-generic.mk endif · line 790

Quoted 12 lines

package/pkg-generic.mk +12 -0

+# Download actual sources if they differ from the extracted sources
+# (e.g. for external toolchains)
+#
+# We need to provide PKG and PKGDIR, because there's no .stamp file for
+# the legal-info step.
+$(1)-legal-source:	PKG=$(2)
+$(1)-legal-source:	PKGDIR=$(pkgdir)
+$(1)-legal-source:
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),)
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
+	$$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))
+endif # actuall sources != sources

actuall sources -> actual source

Quoted 1 line

+endif # actuall source != ""

actuall -> actual

With these fixed:

[Tested with a minimal Sourcery ARM 2014.05 config]

Signature · 1 line

Luca

Luca Ceresoli

2015-12-16 22:47 UTC #9 lore raw

PATCH v2 4/13 core/legal-info: ensure legal-info works in off-line mode

Dear Yann,

Luca Ceresoli wrote:
[...]

Quoted 18 lines

Quoted 11 lines
package/pkg-generic.mk +11 -0
+# Download actual sources if they differ from the extracted sources
+# (e.g. for external toolchains)
+#
+# We need to provide PKG and PKGDIR, because there's no .stamp file for
+# the legal-info step.
+$(1)-legal-source:    PKG=$(2)
+$(1)-legal-source:    PKGDIR=$(pkgdir)
+$(1)-legal-source:
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),)
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
+    $$(call
Quoted 2 lines
DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL)) +endif # actuall sources != sources
actuall sources -> actual source
Quoted 1 line
+endif # actuall source != ""

Quoted 7 lines

actuall -> actual
With these fixed:
Acked-by Luca Ceresoli <[email protected]>
[Tested with a minimal Sourcery ARM 2014.05 config]
Tested-by Luca Ceresoli <[email protected]>

Hey, NO! This patch is wrong.

It makes off-line legal-info work, and that's what I tested successfully:

   $ make clean
   $ rm -fr dl/
   $ make source
   $ make legal-info (offline)

with this defconfig:

BR2_arm=y
BR2_TOOLCHAIN_EXTERNAL=y

But it breaks online 'make legal-info' when _ACTUAL_SOURCES have not yet been downloaded, i.e.:

   $ make clean
   $ rm -fr dl/
   $ make legal-info
   [...]
   cp: cannot stat 
'/home/murray/devel/buildroot/dl/arm-2014.05-29-arm-none-linux-gnueabi.src.tar.bz2': 
No such file or directory

I think this is because you moved the downloading of _ACTUAL_SOURCE from the $(1)-legal-info to $(1)-legal-source, and the former does not depend on the latter.

Perhaps we just need this?

   $(1)-legal-info: $(1)-legal-source

Sorry for having missed this during my first review.

Signature · 1 line

Luca

Yann E. MORIN

author

2015-12-16 23:05 UTC #10 lore raw

PATCH v2 4/13 core/legal-info: ensure legal-info works in off-line mode

Luca, All,

On 2015-12-16 23:47 +0100, Luca Ceresoli spake thusly:

Quoted 22 lines

Luca Ceresoli wrote:
[...]
Quoted 20 lines
Quoted 11 lines
package/pkg-generic.mk +11 -0
+# Download actual sources if they differ from the extracted sources
+# (e.g. for external toolchains)
+#
+# We need to provide PKG and PKGDIR, because there's no .stamp file for
+# the legal-info step.
+$(1)-legal-source:    PKG=$(2)
+$(1)-legal-source:    PKGDIR=$(pkgdir)
+$(1)-legal-source:
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),)
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
+    $$(call
Quoted 2 lines
DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL)) +endif # actuall sources != sources
actuall sources -> actual source
Quoted 1 line
+endif # actuall source != ""
actuall -> actual

Quoted 7 lines

Quoted 5 lines
With these fixed:
Acked-by Luca Ceresoli <[email protected]>
[Tested with a minimal Sourcery ARM 2014.05 config]
Tested-by Luca Ceresoli <[email protected]>
Hey, NO! This patch is wrong.

[--SNIP--]

Quoted 8 lines

with this defconfig:
BR2_arm=y
BR2_TOOLCHAIN_EXTERNAL=y
But it breaks online 'make legal-info' when _ACTUAL_SOURCES have not yet been downloaded, i.e.:

Quoted 6 lines

  $ make clean
  $ rm -fr dl/
  $ make legal-info
  [...]
  cp: cannot stat '/home/murray/devel/buildroot/dl/arm-2014.05-29-arm-none-linux-gnueabi.src.tar.bz2':
No such file or directory

Damit... :-(

Quoted 7 lines

I think this is because you moved the downloading of _ACTUAL_SOURCE from the $(1)-legal-info to $(1)-legal-source, and the former does not depend on the latter.
Perhaps we just need this?
  $(1)-legal-info: $(1)-legal-source

Nope, that does not work either. The download indeed occurs, but then it can not find a hash for it. Indeed, we do not have ssuch a hash anywhere.

I'll investigate tomorrow (or this WE).

Quoted 1 line

Sorry for having missed this during my first review.

No problem, thank you very much for the testing! :-)

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

Arnout Vandecappelle

2015-12-16 23:09 UTC #11 lore raw

PATCH v2 4/13 core/legal-info: ensure legal-info works in off-line mode

Quoted 54 lines · Yann E. MORIN

Luca, All,
On 2015-12-16 23:47 +0100, Luca Ceresoli spake thusly:
Quoted 22 lines
Luca Ceresoli wrote:
[...]
Quoted 20 lines
Quoted 11 lines
package/pkg-generic.mk +11 -0
+# Download actual sources if they differ from the extracted sources
+# (e.g. for external toolchains)
+#
+# We need to provide PKG and PKGDIR, because there's no .stamp file for
+# the legal-info step.
+$(1)-legal-source:    PKG=$(2)
+$(1)-legal-source:    PKGDIR=$(pkgdir)
+$(1)-legal-source:
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),)
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
+    $$(call
Quoted 2 lines
DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL)) +endif # actuall sources != sources
actuall sources -> actual source
Quoted 1 line
+endif # actuall source != ""
actuall -> actual
Quoted 7 lines
Quoted 5 lines
With these fixed:
Acked-by Luca Ceresoli <[email protected]>
[Tested with a minimal Sourcery ARM 2014.05 config]
Tested-by Luca Ceresoli <[email protected]>
Hey, NO! This patch is wrong.
[--SNIP--]
Quoted 8 lines
with this defconfig:
BR2_arm=y
BR2_TOOLCHAIN_EXTERNAL=y
But it breaks online 'make legal-info' when _ACTUAL_SOURCES have not yet been downloaded, i.e.:
Quoted 6 lines
  $ make clean
  $ rm -fr dl/
  $ make legal-info
  [...]
  cp: cannot stat '/home/murray/devel/buildroot/dl/arm-2014.05-29-arm-none-linux-gnueabi.src.tar.bz2':
No such file or directory
Damit... :-(
Quoted 4 lines
I think this is because you moved the downloading of _ACTUAL_SOURCE from the $(1)-legal-info to $(1)-legal-source, and the former does not depend on the latter.

Quoted 7 lines

Quoted 3 lines
Perhaps we just need this?
  $(1)-legal-info: $(1)-legal-source
Nope, that does not work either. The download indeed occurs, but then it can not find a hash for it. Indeed, we do not have ssuch a hash anywhere.

Isn't it just that we need to add hashes for the _ACTUAL_SOURCE_TARBALLs that
we have?

Regards,
Arnout

Quoted 10 lines

I'll investigate tomorrow (or this WE).
Quoted 1 line
Sorry for having missed this during my first review.
No problem, thank you very much for the testing! :-)
Regards,
Yann E. MORIN.

Signature · 6 lines

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

Yann E. MORIN

author

2015-12-16 23:13 UTC #12 lore raw

PATCH v2 4/13 core/legal-info: ensure legal-info works in off-line mode

Arnout, All,

On 2015-12-17 00:09 +0100, Arnout Vandecappelle spake thusly:

Quoted 55 lines · Yann E. MORIN

Quoted 54 lines
On 2015-12-16 23:47 +0100, Luca Ceresoli spake thusly:
Quoted 22 lines
Luca Ceresoli wrote:
[...]
Quoted 20 lines
Quoted 11 lines
package/pkg-generic.mk +11 -0
+# Download actual sources if they differ from the extracted sources
+# (e.g. for external toolchains)
+#
+# We need to provide PKG and PKGDIR, because there's no .stamp file for
+# the legal-info step.
+$(1)-legal-source:    PKG=$(2)
+$(1)-legal-source:    PKGDIR=$(pkgdir)
+$(1)-legal-source:
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),)
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
+    $$(call
Quoted 2 lines
DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL)) +endif # actuall sources != sources
actuall sources -> actual source
Quoted 1 line
+endif # actuall source != ""
actuall -> actual
Quoted 7 lines
Quoted 5 lines
With these fixed:
Acked-by Luca Ceresoli <[email protected]>
[Tested with a minimal Sourcery ARM 2014.05 config]
Tested-by Luca Ceresoli <[email protected]>
Hey, NO! This patch is wrong.
[--SNIP--]
Quoted 8 lines
with this defconfig:
BR2_arm=y
BR2_TOOLCHAIN_EXTERNAL=y
But it breaks online 'make legal-info' when _ACTUAL_SOURCES have not yet been downloaded, i.e.:
Quoted 6 lines
  $ make clean
  $ rm -fr dl/
  $ make legal-info
  [...]
  cp: cannot stat '/home/murray/devel/buildroot/dl/arm-2014.05-29-arm-none-linux-gnueabi.src.tar.bz2':
No such file or directory
Damit... :-(
Quoted 6 lines
I think this is because you moved the downloading of _ACTUAL_SOURCE from the $(1)-legal-info to $(1)-legal-source, and the former does not depend on the latter.
Perhaps we just need this?

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
  $(1)-legal-info: $(1)-legal-source
Nope, that does not work either. The download indeed occurs, but then it can not find a hash for it. Indeed, we do not have ssuch a hash anywhere.
Isn't it just that we need to add hashes for the _ACTUAL_SOURCE_TARBALLs that
we have?

It seems that does the trick, indeed. By "investigate", I really meant "adding the missing hashes". ;-)

Anyway, I'll do some more tests later this week...

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

Arnout Vandecappelle

2015-12-16 23:19 UTC #13 lore raw

PATCH v2 4/13 core/legal-info: ensure legal-info works in off-line mode

Quoted 9 lines · Yann E. MORIN

Almost all packages which are saved for legal-info have their source archives downloaded as part of 'make source', which makes an off-line build completely possible [0].
However, for the pre-configured external toolchains, the source tarball is different, as the main tarball is a binary package. And that source tarball is only downloaded during the legal-info phase, which makes it inconvenient for full off-line builds.

Quoted 5 lines

We fix that in two steps:
- first, we move the declaration of _ACTUAL_SOURCE_TARBALL and _SITE earlier in the pkg-generic.mk file, so we get those definitions at the time we define the -source and -all-source rules;

I don't see why this is needed: they are first use at exactly the place where
they were originally defined.

That said, these definitions fit much better where you put them now (just after
the handling of _SOURCE), so keep it this way. Could be a seperate patch but not really required for me.

Quoted 6 lines

- second, we add a new rule, $(1)-legal-source which only $(1)-all-source depends on, so that we only download it for a top-level 'make source', not as part of the standard download mechanism (i.e. only what is really needed to build).

Quoted 6 lines

This way, we can do a complete [0] off-line build and are still able to generate legal-info, while at the same time we do not incur any download overhead during a simple build.
Also, we previously downloaded the _ACTUAL_SOURCE_TARBALL when it was not empty.

I haven't tested, but according to me this code already did the right thing:

ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))

    $$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))

endif

It is true, however, that the original code didn't handle the case where a
package explicitly declares ACTUAL_SOURCE_TARBALL to be empty.

Quoted 9 lines

However, since _ACTUAL_SOURCE_TARBALL default to the value of _SOURCE, it can not be empty when _SOURCE is not. Thus, we'd get a spurious report of a missing hash for the tarball, since it was not in a standard package rule (configure, build, install..) and thus would miss the PKG and PKGDIR variables to find the .hash file.
We fix that in this commit as well, by:
- setting PKG and PKGDIR just for the -legal-source rule;

Have we still not gotten rid of PKGDIR? How silly...

Quoted 24 lines

- only downloading _ACTUAL_SOURCE_TARBALL if it is not empty *and* not the same as _SOURCE (to avoid a second report about the hash).
[0] Save for nodejs which invarriably wants to download stuff at build time. Sigh... :-( Fixing that is work for another time...
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Peter Korsgaard <[email protected]>
---
Changes v1 -> v2:
- drop the 'redistribute == ignore', it does not exist yet
---
 package/pkg-generic.mk | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)
package/pkg-generic.mk +0 -0
index e69c970..832045e 100644

package/pkg-generic.mk ifndef $(2)_SOURCE · line 437

Quoted 10 lines

package/pkg-generic.mk +7 -0

@@ -428,6 +428,14 @@ ifndef $(2)_SOURCE
  endif
 endif
 
+# If FOO_ACTUAL_SOURCE_TARBALL is explicitly defined, it means FOO_SOURCE is
+# indeed a binary (e.g. external toolchain) and FOO_ACTUAL_SOURCE_TARBALL/_SITE
+# point to the actual sources tarball. Use the actual sources for legal-info.
+# For most packages the FOO_SITE/FOO_SOURCE pair points to real source code,
+# so these are the defaults for FOO_ACTUAL_*.
+$(2)_ACTUAL_SOURCE_TARBALL ?= $$($(2)_SOURCE)
+$(2)_ACTUAL_SOURCE_SITE    ?= $$(call qstrip,$$($(2)_SITE))

This is not really related to this patch (it was like that in the original),
but why do we qstrip here?

package/pkg-generic.mk $(1)-depends: $$($(2)_FINAL_DEPENDENCIES) · line 640

Quoted 11 lines

package/pkg-generic.mk +4 -0

+
 ifndef $(2)_PATCH
  ifdef $(3)_PATCH
   $(2)_PATCH = $$($(3)_PATCH)

@@ -627,6 +635,10 @@ $(1)-depends:		$$($(2)_FINAL_DEPENDENCIES)
 
 $(1)-source:		$$($(2)_TARGET_SOURCE)
 
+$(1)-all-source:	$(1)-legal-source
+
+$(1)-legal-source:	$(1)-source

As noted by Luca, we're missing a -legal-info -> legal-source dependency here.

Quoted 16 lines

package/pkg-generic.mk +2 -7

+
 $(1)-source-check:
 	$$(foreach p,$$($(2)_ALL_DOWNLOADS),$$(call SOURCE_CHECK,$$(p))$$(sep))

@@ -764,13 +776,19 @@ endif
 endif
 endif
 
-# If FOO_ACTUAL_SOURCE_TARBALL is explicitly defined, it means FOO_SOURCE is
-# indeed a binary (e.g. external toolchain) and FOO_ACTUAL_SOURCE_TARBALL/_SITE
-# point to the actual sources tarball. Use the actual sources for legal-info.
-# For most packages the FOO_SITE/FOO_SOURCE pair points to real source code,
-# so these are the defaults for FOO_ACTUAL_*.
-$(2)_ACTUAL_SOURCE_TARBALL ?= $$($(2)_SOURCE)
-$(2)_ACTUAL_SOURCE_SITE    ?= $$(call qstrip,$$($(2)_SITE))
+# Download actual sources if they differ from the extracted sources

package/pkg-generic.mk endif · line 789

Quoted 10 lines

package/pkg-generic.mk +10 -0

+# (e.g. for external toolchains)
+#
+# We need to provide PKG and PKGDIR, because there's no .stamp file for
+# the legal-info step.
+$(1)-legal-source:	PKG=$(2)
+$(1)-legal-source:	PKGDIR=$(pkgdir)
+$(1)-legal-source:
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),)
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
+	$$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))

Actually we should have the same kind of logic here as in _ALL_DOWNLOADS, to
allow for multiple ACTUAL_SOURCE_TARBALLs and different SITEs. And that would also remove the need for the ACTUAL_SOURCE_SITE. And if we use a foreach, it would remove the need to check for an empty TARBALL. But of course, all that is not for this patch :-)

Quoted 16 lines

package/pkg-generic.mk +2 -3

+endif # actuall sources != sources
+endif # actuall source != ""
 
 # legal-info: produce legally relevant info.
 $(1)-legal-info:

@@ -802,9 +820,6 @@ else
 # Other packages
 
 ifeq ($$($(2)_REDISTRIBUTE),YES)
-ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
-	$$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))
-endif
 # Copy the source tarball
 	$$(Q)$$(call hardlink-copy,\
 		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\

package/pkg-generic.mk endif · line 925

Quoted 4 lines

package/pkg-generic.mk +1 -0

@@ -907,6 +922,7 @@ endif
 	$(1)-show-depends \
 	$(1)-show-version \
 	$(1)-source \
+	$(1)-legal-source \

The rest of .PHONY is kept alphabetically ordered...

Regards,
Arnout

Quoted 4 lines

package/pkg-generic.mk +0 -0

 	$(1)-source-check
 
 ifeq ($$(patsubst %/,ERROR,$$($(2)_SITE)),ERROR)

Signature · 6 lines

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

Yann E. MORIN

author

2015-12-18 22:11 UTC #14 lore raw

PATCH v2 4/13 core/legal-info: ensure legal-info works in off-line mode

Arnout, All,

On 2015-12-17 00:19 +0100, Arnout Vandecappelle spake thusly:

Quoted 10 lines · Yann E. MORIN

Quoted 9 lines
Almost all packages which are saved for legal-info have their source archives downloaded as part of 'make source', which makes an off-line build completely possible [0].
However, for the pre-configured external toolchains, the source tarball is different, as the main tarball is a binary package. And that source tarball is only downloaded during the legal-info phase, which makes it inconvenient for full off-line builds.

Quoted 8 lines

Quoted 5 lines
We fix that in two steps:
- first, we move the declaration of _ACTUAL_SOURCE_TARBALL and _SITE earlier in the pkg-generic.mk file, so we get those definitions at the time we define the -source and -all-source rules;
I don't see why this is needed: they are first use at exactly the place where
they were originally defined.

Yep. That was nto the case in a previous iteration (that I did not post), and the code has changed since then, but I overlooked that part of the commit log. Will fix it.

Quoted 3 lines

That said, these definitions fit much better where you put them now (just after
the handling of _SOURCE), so keep it this way. Could be a seperate patch but not really required for me.

Yep, I'll split and keep the move, so the code is "nicer". ;-)

Quoted 12 lines

Quoted 11 lines
- second, we add a new rule, $(1)-legal-source which only $(1)-all-source depends on, so that we only download it for a top-level 'make source', not as part of the standard download mechanism (i.e. only what is really needed to build).
This way, we can do a complete [0] off-line build and are still able to generate legal-info, while at the same time we do not incur any download overhead during a simple build.
Also, we previously downloaded the _ACTUAL_SOURCE_TARBALL when it was not empty.

Quoted 8 lines

I haven't tested, but according to me this code already did the right thing:
ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
    $$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))
endif
It is true, however, that the original code didn't handle the case where a
package explicitly declares ACTUAL_SOURCE_TARBALL to be empty.

Yes, everything was downloaded, even missing pieces.

However, this patch is about getting the missing pieces when running 'make source' for completely-offline builds.

What was previously broken was;

make menuconfig
[enable whatever, like an external toolchain with an actual source] make source
[unplug your ehternet cable, disable WiFi, get under a tunnel to
avoid 3g...]
make legal-info

-> broken, it tries to download the actual sources.

What this patch does is;

- 'make source' will also download the actual sources

- but since it can take some time to do so, we do not want to do it for a simple build, like just runnint 'make'

Quoted 11 lines

Quoted 9 lines
However, since _ACTUAL_SOURCE_TARBALL default to the value of _SOURCE, it can not be empty when _SOURCE is not. Thus, we'd get a spurious report of a missing hash for the tarball, since it was not in a standard package rule (configure, build, install..) and thus would miss the PKG and PKGDIR variables to find the .hash file.
We fix that in this commit as well, by:
- setting PKG and PKGDIR just for the -legal-source rule;
Have we still not gotten rid of PKGDIR? How silly...

Hmm... It;s used elsewhere in the code, so I stick to it.

Removing can be another series.

[--SNIP--]

Quoted 5 lines

Quoted 2 lines
+$(2)_ACTUAL_SOURCE_TARBALL ?= $$($(2)_SOURCE)
+$(2)_ACTUAL_SOURCE_SITE ?= $$(call qstrip,$$($(2)_SITE))
This is not really related to this patch (it was like that in the original),
but why do we qstrip here?

Good question. I just moved the code around.

Quoted 9 lines

package/pkg-generic.mk $(1)-depends: $$($(2)_FINAL_DEPENDENCIES) · line 640
Quoted 7 lines
package/pkg-generic.mk +3 -0
@@ -627,6 +635,10 @@ $(1)-depends:		$$($(2)_FINAL_DEPENDENCIES)
 
 $(1)-source:		$$($(2)_TARGET_SOURCE)
 
+$(1)-all-source:	$(1)-legal-source
+
+$(1)-legal-source:	$(1)-source
As noted by Luca, we're missing a -legal-info -> legal-source dependency here.

Yup, already fixed here. Thanks! :-)

Quoted 23 lines

package/pkg-generic.mk +9 -7

 $(1)-source-check:
 	$$(foreach p,$$($(2)_ALL_DOWNLOADS),$$(call SOURCE_CHECK,$$(p))$$(sep))

@@ -764,13 +776,19 @@ endif
 endif
 endif
 
-# If FOO_ACTUAL_SOURCE_TARBALL is explicitly defined, it means FOO_SOURCE is
-# indeed a binary (e.g. external toolchain) and FOO_ACTUAL_SOURCE_TARBALL/_SITE
-# point to the actual sources tarball. Use the actual sources for legal-info.
-# For most packages the FOO_SITE/FOO_SOURCE pair points to real source code,
-# so these are the defaults for FOO_ACTUAL_*.
-$(2)_ACTUAL_SOURCE_TARBALL ?= $$($(2)_SOURCE)
-$(2)_ACTUAL_SOURCE_SITE    ?= $$(call qstrip,$$($(2)_SITE))
+# Download actual sources if they differ from the extracted sources
+# (e.g. for external toolchains)
+#
+# We need to provide PKG and PKGDIR, because there's no .stamp file for
+# the legal-info step.
+$(1)-legal-source:	PKG=$(2)
+$(1)-legal-source:	PKGDIR=$(pkgdir)
+$(1)-legal-source:
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),)

Quoted 8 lines

package/pkg-generic.mk endif · line 789
Quoted 2 lines
package/pkg-generic.mk +2 -0
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
+	$$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))
Actually we should have the same kind of logic here as in _ALL_DOWNLOADS, to
allow for multiple ACTUAL_SOURCE_TARBALLs and different SITEs. And that would also remove the need for the ACTUAL_SOURCE_SITE. And if we use a foreach, it would remove the need to check for an empty TARBALL. But of course, all that is not for this patch :-)

Yup, can be done later.

Quoted 7 lines

package/pkg-generic.mk endif · line 925
Quoted 5 lines
package/pkg-generic.mk +1 -0
@@ -907,6 +922,7 @@ endif
 	$(1)-show-depends \
 	$(1)-show-version \
 	$(1)-source \
+	$(1)-legal-source \
The rest of .PHONY is kept alphabetically ordered...

OK, fixed.

Thanks! :-)

Regards,
Yann E. MORIN.

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Yann E. MORIN

author

2015-12-18 22:30 UTC #15 lore raw

PATCH v2 4/13 core/legal-info: ensure legal-info works in off-line mode

Arnout, All,

On 2015-12-18 23:11 +0100, Yann E. MORIN spake thusly:

Quoted 2 lines

On 2015-12-17 00:19 +0100, Arnout Vandecappelle spake thusly:
Quoted 1 line · Yann E. MORIN

[--SNIP--]

Quoted 14 lines

Quoted 14 lines
Quoted 11 lines
- second, we add a new rule, $(1)-legal-source which only $(1)-all-source depends on, so that we only download it for a top-level 'make source', not as part of the standard download mechanism (i.e. only what is really needed to build).
This way, we can do a complete [0] off-line build and are still able to generate legal-info, while at the same time we do not incur any download overhead during a simple build.
Also, we previously downloaded the _ACTUAL_SOURCE_TARBALL when it was not empty.
I haven't tested, but according to me this code already did the right thing:

Quoted 8 lines

Quoted 6 lines
ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
    $$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))
endif
It is true, however, that the original code didn't handle the case where a
package explicitly declares ACTUAL_SOURCE_TARBALL to be empty.
Yes, everything was downloaded, even missing pieces.

Hmmm... I did not answer your question. I'm off-topic... OK, so now for the real answer to your real question:

s/not empty/not equal to the _SOURCE/

But I'll improve the commit log in this respect.

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

Yann E. MORIN

author

2015-12-13 18:35 UTC #16 lore raw

PATCH v2 5/13 core/pkg-generic: add variable to store the package rawname-version

Introduce a new per-package variable to store the 'rawname-version' tuple, instead of computing it every time we need it.

Currently, it's only a single location, but follw-up patches will introduce more use of it.

Reported-by Luca Ceresoli <[email protected]>

Cc Luca Ceresoli <[email protected]>

---
 package/pkg-generic.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

package/pkg-generic.mk +2 -1

index 832045e..784c39c 100644
@@ -388,6 +388,7 @@ else
  $(2)_DL_VERSION := $$(strip $$($(2)_VERSION))
 endif
 $(2)_VERSION := $$(call sanitize,$$($(2)_DL_VERSION))
+$(2)_RAWNAME_VERSION = $$($(2)_RAWNAME)-$$($(2)_VERSION)
 
 ifdef $(3)_OVERRIDE_SRCDIR
   $(2)_OVERRIDE_SRCDIR ?= $$($(3)_OVERRIDE_SRCDIR)

@@ -424,7 +425,7 @@ ifndef $(2)_SOURCE
  ifdef $(3)_SOURCE
   $(2)_SOURCE = $$($(3)_SOURCE)
  else
-  $(2)_SOURCE			?= $$($(2)_RAWNAME)-$$($(2)_VERSION).tar.gz
+  $(2)_SOURCE			?= $$($(2)_RAWNAME_VERSION).tar.gz
  endif
 endif

Signature · 1 line

1.9.1

Luca Ceresoli

2015-12-16 12:07 UTC #17 lore raw

PATCH v2 5/13 core/pkg-generic: add variable to store the package rawname-version

Dear Yann,

Quoted 4 lines · Yann E. MORIN

Introduce a new per-package variable to store the 'rawname-version' tuple, instead of computing it every time we need it.
Currently, it's only a single location, but follw-up patches will

follw-up -> follow-up

Quoted 5 lines

introduce more use of it.
Reported-by Luca Ceresoli <[email protected]>
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>

With the typo fixed:

Signature · 1 line

Luca

Arnout Vandecappelle

2015-12-16 23:44 UTC #18 lore raw

PATCH v2 5/13 core/pkg-generic: add variable to store the package rawname-version

Quoted 2 lines · Yann E. MORIN

Introduce a new per-package variable to store the 'rawname-version' tuple, instead of computing it every time we need it.

It's of course still computed every time we need it because it's a
recursively-expanded variable. So this is just an abbreviation.

Quoted 2 lines

Currently, it's only a single location, but follw-up patches will

follow-up

Quoted 14 lines

introduce more use of it.
Reported-by Luca Ceresoli <[email protected]>
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
---
 package/pkg-generic.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
package/pkg-generic.mk +0 -0
index 832045e..784c39c 100644

package/pkg-generic.mk else · line 391

Quoted 4 lines

package/pkg-generic.mk +1 -0

@@ -388,6 +388,7 @@ else
  $(2)_DL_VERSION := $$(strip $$($(2)_VERSION))
 endif
 $(2)_VERSION := $$(call sanitize,$$($(2)_DL_VERSION))
+$(2)_RAWNAME_VERSION = $$($(2)_RAWNAME)-$$($(2)_VERSION)

There used to be a variable 'NAMEVER'. I think that that's a more appropriate name.

Regardless of this, you can add my

Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

Regards,
Arnout

Quoted 13 lines

package/pkg-generic.mk +1 -1

 ifdef $(3)_OVERRIDE_SRCDIR
   $(2)_OVERRIDE_SRCDIR ?= $$($(3)_OVERRIDE_SRCDIR)

@@ -424,7 +425,7 @@ ifndef $(2)_SOURCE
  ifdef $(3)_SOURCE
   $(2)_SOURCE = $$($(3)_SOURCE)
  else
-  $(2)_SOURCE			?= $$($(2)_RAWNAME)-$$($(2)_VERSION).tar.gz
+  $(2)_SOURCE			?= $$($(2)_RAWNAME_VERSION).tar.gz
  endif
 endif

Signature · 6 lines

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

Yann E. MORIN

author

2015-12-13 18:35 UTC #19 lore raw

PATCH v2 6/13 core/legal-info: install source archives in their own sub-dir

Currently, we put all source archives side-by-side in the same directory.

Since we're about to also save individual patches that were applied on those sources, we don't want to make that directory a complete mess of unassorted files.

So, we install each source archive in its own sub-directory, where we'll later store the patches too. Store that location in a variable, so it can be re-used later on (to install patches in a future commit).

Cc Luca Ceresoli <[email protected]>

---
Changes v1 -> v2:
- perl no longer has a post-legal-info hook (Thoams, Luca)

---
 package/pkg-generic.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

package/pkg-generic.mk +3 -1

index 784c39c..3b6240a 100644
@@ -491,6 +491,8 @@ endif
 
 $(2)_REDISTRIBUTE		?= YES
 
+$(2)_REDIST_SOURCES_DIR = $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))/$$($(2)_RAWNAME_VERSION)
+
 # When a target package is a toolchain dependency set this variable to
 # 'NO' so the 'toolchain' dependency is not added to prevent a circular
 # dependency

@@ -824,7 +826,7 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 # Copy the source tarball
 	$$(Q)$$(call hardlink-copy,\
 		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
-		     $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))))
+		     $$($(2)_REDIST_SOURCES_DIR))
 endif # redistribute
 
 endif # other packages

Signature · 1 line

1.9.1

Arnout Vandecappelle

2015-12-16 23:56 UTC #20 lore raw

PATCH v2 6/13 core/legal-info: install source archives in their own sub-dir

Quoted 7 lines · Yann E. MORIN

Currently, we put all source archives side-by-side in the same directory.
Since we're about to also save individual patches that were applied on those sources, we don't want to make that directory a complete mess of unassorted files.

Quoted 8 lines

So, we install each source archive in its own sub-directory, where we'll later store the patches too. Store that location in a variable, so it can be re-used later on (to install patches in a future commit).
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Acked-by Luca Ceresoli <[email protected]>
Tested-by Luca Ceresoli <[email protected]>

Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

Quoted 13 lines

---
Changes v1 -> v2:
- perl no longer has a post-legal-info hook (Thoams, Luca)
---
 package/pkg-generic.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
package/pkg-generic.mk +0 -0
index 784c39c..3b6240a 100644

package/pkg-generic.mk endif · line 494

Quoted 4 lines

package/pkg-generic.mk +1 -0

@@ -491,6 +491,8 @@ endif
 
 $(2)_REDISTRIBUTE		?= YES
 
+$(2)_REDIST_SOURCES_DIR = $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))/$$($(2)_RAWNAME_VERSION)

Not directly related to this patch, but I find this
$$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))) bit extremely ugly. $(LEGAL_INFO_DIR)/$(4)-sources is a lot nicer. Of course, that means we put things in target-sources instead of plain sources, but I think that's an improvement as well.

Regards,
Arnout

Quoted 14 lines

package/pkg-generic.mk +2 -1

+
 # When a target package is a toolchain dependency set this variable to
 # 'NO' so the 'toolchain' dependency is not added to prevent a circular
 # dependency

@@ -824,7 +826,7 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 # Copy the source tarball
 	$$(Q)$$(call hardlink-copy,\
 		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
-		     $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))))
+		     $$($(2)_REDIST_SOURCES_DIR))
 endif # redistribute
 
 endif # other packages

Signature · 6 lines

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

Yann E. MORIN

author

2015-12-18 22:53 UTC #21 lore raw

PATCH v2 6/13 core/legal-info: install source archives in their own sub-dir

Arnout, All,

On 2015-12-17 00:56 +0100, Arnout Vandecappelle spake thusly:

Quoted 35 lines · Yann E. MORIN

Quoted 7 lines
Currently, we put all source archives side-by-side in the same directory.
Since we're about to also save individual patches that were applied on those sources, we don't want to make that directory a complete mess of unassorted files.
Quoted 8 lines
So, we install each source archive in its own sub-directory, where we'll later store the patches too. Store that location in a variable, so it can be re-used later on (to install patches in a future commit).
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Acked-by Luca Ceresoli <[email protected]>
Tested-by Luca Ceresoli <[email protected]>
Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>
Quoted 16 lines
---
Changes v1 -> v2:
- perl no longer has a post-legal-info hook (Thoams, Luca)
---
 package/pkg-generic.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
package/pkg-generic.mk +0 -0
index 784c39c..3b6240a 100644
@@ -491,6 +491,8 @@ endif
 
 $(2)_REDISTRIBUTE		?= YES
 

Quoted 7 lines

package/pkg-generic.mk endif · line 494
Quoted 1 line
package/pkg-generic.mk +1 -0
+$(2)_REDIST_SOURCES_DIR = $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))/$$($(2)_RAWNAME_VERSION)
Not directly related to this patch, but I find this
$$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))) bit extremely ugly. $(LEGAL_INFO_DIR)/$(4)-sources is a lot nicer. Of course, that means we put things in target-sources instead of plain sources, but I think that's an improvement as well.

I'm not so sure it is better: we do not have the habit of prefixing target stuff with 'target-'. Only the host stuff we do prefix with 'host-'. Why would we derogate from that rule for legal-info?

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

Arnout Vandecappelle

2015-12-19 00:02 UTC #22 lore raw

PATCH v2 6/13 core/legal-info: install source archives in their own sub-dir

Quoted 4 lines · Yann E. MORIN

Arnout, All,
On 2015-12-17 00:56 +0100, Arnout Vandecappelle spake thusly:
Quoted 1 line · Yann E. MORIN

[snip]

Quoted 11 lines

Quoted 7 lines
Quoted 1 line
+$(2)_REDIST_SOURCES_DIR = $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))/$$($(2)_RAWNAME_VERSION)
Not directly related to this patch, but I find this
$$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4))) bit extremely ugly. $(LEGAL_INFO_DIR)/$(4)-sources is a lot nicer. Of course, that means we put things in target-sources instead of plain sources, but I think that's an improvement as well.
I'm not so sure it is better: we do not have the habit of prefixing target stuff with 'target-'. Only the host stuff we do prefix with 'host-'. Why would we derogate from that rule for legal-info?

Hm, good point. Let's keep it like it is then.

Regards,
Arnout

Signature · 6 lines

Arnout Vandecappelle      arnout dot vandecappelle at essensium dot com
Senior Embedded Software Architect . . . . . . +32-478-010353 (mobile)
Essensium, Mind division . . . . . . . . . . . . . . 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

Yann E. MORIN

author

2015-12-13 18:35 UTC #23 lore raw

PATCH v2 7/13 core/legal-info: add package version to license directory

Now that we save the source archives in a directory named after the package and its version, do the same for the license files, for consistency.

It has a not-so-bad side-effect of also saving the version string in the all-licenses list.

The only (small) side-efect, is that the warnings about undefined _LICENSE_FILES now contains the version string, too. That's unavoidable, since that's what is stored in the legal report.

Cc Luca Ceresoli <[email protected]>

---
Changes v1 -> v2:
- s/drawback/side-effect/ (Luca)

---
 package/pkg-generic.mk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

package/pkg-generic.mk +3 -3

index 3b6240a..9b9a27f 100644
@@ -806,10 +806,10 @@ ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
 # 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))
+	@$$(call legal-license-nofiles,$$($(2)_RAWNAME_VERSION),$$(call UPPERCASE,$(4)))
+	@$$(call legal-warning-pkg,$$($(2)_RAWNAME_VERSION),cannot save license ($(2)_LICENSE_FILES not defined))
 else
-	@$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME),$$(F),$$($(2)_DIR)/$$(F),$$(call UPPERCASE,$(4)))$$(sep))
+	@$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME_VERSION),$$(F),$$($(2)_DIR)/$$(F),$$(call UPPERCASE,$(4)))$$(sep))
 endif # license files
 
 ifeq ($$($(2)_SITE_METHOD),local)

Signature · 1 line

1.9.1

Luca Ceresoli

2015-12-16 14:21 UTC #24 lore raw

PATCH v2 7/13 core/legal-info: add package version to license directory

Dear Yann,

Quoted 8 lines · Yann E. MORIN

Now that we save the source archives in a directory named after the package and its version, do the same for the license files, for consistency.
It has a not-so-bad side-effect of also saving the version string in the all-licenses list.
The only (small) side-efect, is that the warnings about undefined

efect -> effect

Quoted 18 lines

_LICENSE_FILES now contains the version string, too. That's unavoidable, since that's what is stored in the legal report.
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
---
Changes v1 -> v2:
- s/drawback/side-effect/ (Luca)
---
  package/pkg-generic.mk | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)
package/pkg-generic.mk +0 -0
index 3b6240a..9b9a27f 100644

package/pkg-generic.mk ifneq ($$(call qstrip,$$($(2)_SOURCE)),) · line 812

Quoted 10 lines

package/pkg-generic.mk +3 -3

@@ -806,10 +806,10 @@ ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
  # 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))
+	@$$(call legal-license-nofiles,$$($(2)_RAWNAME_VERSION),$$(call UPPERCASE,$(4)))
+	@$$(call legal-warning-pkg,$$($(2)_RAWNAME_VERSION),cannot save license ($(2)_LICENSE_FILES not defined))
  else
-	@$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME),$$(F),$$($(2)_DIR)/$$(F),$$(call UPPERCASE,$(4)))$$(sep))
+	@$$(foreach F,$$($(2)_LICENSE_FILES),$$(call legal-license-file,$$($(2)_RAWNAME_VERSION),$$(F),$$($(2)_DIR)/$$(F),$$(call UPPERCASE,$(4)))$$(sep))

Signature · 1 line

Luca

Yann E. MORIN

author

2015-12-13 18:35 UTC #25 lore raw

PATCH v2 8/13 core/apply-patches: store full path of applied patches

Currently, we only store the filename of the applied patches.

However, we are soon to want to install those patches in the legal-info directory, so we'll have to know where those patches come from.

Instead of duplicating the logic to find the patches (bundled, downloaded, from a global patch dir...), just store the full path to each of those patches so we can retrieve them more easily later on.

Also always create the list-file, even if empty, so that we need not test for its existence before reading it.

Cc Luca Ceresoli <[email protected]>

---
Changes v1 -> v2:
- do not duplicate '/' in paths (Luca)

---
 support/scripts/apply-patches.sh | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

support/scripts/apply-patches.sh +8 -3

index aa13e88..5110458 100755
@@ -63,8 +63,12 @@ find ${builddir}/ '(' -name '*.rej' -o -name '.*.rej' ')' -print0 | \
     xargs -0 -r rm -f
 
 function apply_patch {
-    path=$1
-    patch=$2
+    path="${1%%/}"
+    patch="${2}"
+    case "${path}" in
+        /*) ;;
+        *)  path="$(pwd)/${path}";;
+    esac
     if [ "$3" ]; then
         type="series"; uncomp="cat"
     else

@@ -99,7 +103,7 @@ function apply_patch {
 	echo "Error: missing patch file ${path}/$patch"
 	exit 1
     fi
-    echo $patch >> ${builddir}/.applied_patches_list
+    echo "${path}/${patch}" >> ${builddir}/.applied_patches_list
     ${uncomp} "${path}/$patch" | patch -g0 -p1 -E -d "${builddir}" -t -N $silent
     if [ $? != 0 ] ; then
         echo "Patch failed!  Please fix ${patch}!"

@@ -141,6 +145,7 @@ function scan_patchdir {
     fi
 }
 
+touch ${builddir}/.applied_patches_list
 scan_patchdir "$patchdir" "$patchpattern"
 
 # Check for rejects...

Signature · 1 line

1.9.1

Luca Ceresoli

2015-12-17 10:44 UTC #26 lore raw

PATCH v2 8/13 core/apply-patches: store full path of applied patches

Dear Yann,

Quoted 27 lines · Yann E. MORIN

Currently, we only store the filename of the applied patches.
However, we are soon to want to install those patches in the legal-info directory, so we'll have to know where those patches come from.
Instead of duplicating the logic to find the patches (bundled, downloaded, from a global patch dir...), just store the full path to each of those patches so we can retrieve them more easily later on.
Also always create the list-file, even if empty, so that we need not test for its existence before reading it.
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
---
Changes v1 -> v2:
- do not duplicate '/' in paths (Luca)
---
  support/scripts/apply-patches.sh | 11 ++++++++---
  1 file changed, 8 insertions(+), 3 deletions(-)
support/scripts/apply-patches.sh +0 -0
index aa13e88..5110458 100755

support/scripts/apply-patches.sh find ${builddir}/ '(' -name '*.rej' -o -name '.*.rej' ')' -print0 | \ · line 66

Quoted 6 lines

support/scripts/apply-patches.sh +1 -2

@@ -63,8 +63,12 @@ find ${builddir}/ '(' -name '*.rej' -o -name '.*.rej' ')' -print0 | \
      xargs -0 -r rm -f
 
  function apply_patch {
-    path=$1
-    patch=$2
+    path="${1%%/}"

Uuuh, yeeah, thanks! :)

[Tested only with patches in the Buildroot sources]

Signature · 1 line

Luca

Yann E. MORIN

author

2015-12-13 18:35 UTC #27 lore raw

PATCH v2 9/13 core/legal-info: also save patches

Currently, the legal-info infra only saves the source archive of a package. However, that's not enough as we may apply some patches on packages sources.

We do suggest users to also redistribute the Buildroot sources as part of their compliance distribution, so the patches bundled in Buildroot would indeed be included in the compliance distribution.

However, that's still not enough, since we may download some patches, or the user may use a global patch directory. Patches in there might not end up in the compliance distribution, and there are risks of non-conformity.

So, always include patches alongside the source archive.

To ensure reproducibility, we also generate a series file, so patches can be re-applied in the correct order.

We get the list of patches to include from the list of patches that were applied by the package infrastructure (via the apply-patches support script). So, we need to get packages properly extracted and patched before we can save their legal-info, not just in the case they define _LICENSE_FILES.

Update the legal-info header accordingly.

Note: this means that, when a package is not patched and defines no LICENSE_FILES, we will extract and patch it for nothing. There is no easy way to know whether we have to patch a package or not. We can only either duplicate the logic to detect patches (bad) or rely on the infra actually patching the package. Also, a vast majority of packages are either patched, or define _LICENSE_FILES, so it is best and easiest to always extract and patch them prior to legal-info.

Cc Luca Ceresoli <[email protected]>

---
Changes v1 -> v2;
- don't recompute rawname-version needlessly (Luca)

---
 package/pkg-generic.mk           | 12 +++++++-----
 support/legal-info/README.header |  3 +--
 2 files changed, 8 insertions(+), 7 deletions(-)

package/pkg-generic.mk +7 -5

index 9b9a27f..f873e9c 100644
@@ -760,12 +760,10 @@ $(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),)
+# We need to extract and patch a package to be able to retrieve its
+# license files (if any) and the list of patches applied to it (if
+# any).
 $(1)-legal-info: $(1)-patch
-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

@@ -827,6 +825,10 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 	$$(Q)$$(call hardlink-copy,\
 		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
 		     $$($(2)_REDIST_SOURCES_DIR))
+	$$(Q)while read f; do \
+		$$(call hardlink-copy,$$$${f},$$($(2)_REDIST_SOURCES_DIR)) || exit 1; \
+		printf "%s\n" "$$$${f##*/}" >>$$($(2)_REDIST_SOURCES_DIR)/series || exit 1; \
+	done <$$($(2)_DIR)/.applied_patches_list
 endif # redistribute
 
 endif # other packages

support/legal-info/README.header +1 -2

index d07c45d..4d7fd7c 100644
@@ -16,8 +16,7 @@ This material is composed of the following items.
    need to collect it manually.
  * The source code for all packages; this has been saved in the sources/
    subdirectory (except for the non-redistributable packages, which have not
-   been saved); patches applied to some packages by Buildroot are included in
-   the Buildroot sources and were not duplicated in the sources/ subdirectory.
+   been saved).
  * A manifest file listing the configured packages and related information.
  * The license text of the packages; they have been saved in the licenses/
    subdirectory.

Signature · 1 line

1.9.1

Luca Ceresoli

2015-12-17 10:45 UTC #28 lore raw

PATCH v2 9/13 core/legal-info: also save patches

Dear Yann,

Quoted 81 lines · Yann E. MORIN

Currently, the legal-info infra only saves the source archive of a package. However, that's not enough as we may apply some patches on packages sources.
We do suggest users to also redistribute the Buildroot sources as part of their compliance distribution, so the patches bundled in Buildroot would indeed be included in the compliance distribution.
However, that's still not enough, since we may download some patches, or the user may use a global patch directory. Patches in there might not end up in the compliance distribution, and there are risks of non-conformity.
So, always include patches alongside the source archive.
To ensure reproducibility, we also generate a series file, so patches can be re-applied in the correct order.
We get the list of patches to include from the list of patches that were applied by the package infrastructure (via the apply-patches support script). So, we need to get packages properly extracted and patched before we can save their legal-info, not just in the case they define _LICENSE_FILES.
Update the legal-info header accordingly.
Note: this means that, when a package is not patched and defines no LICENSE_FILES, we will extract and patch it for nothing. There is no easy way to know whether we have to patch a package or not. We can only either duplicate the logic to detect patches (bad) or rely on the infra actually patching the package. Also, a vast majority of packages are either patched, or define _LICENSE_FILES, so it is best and easiest to always extract and patch them prior to legal-info.
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
---
Changes v1 -> v2;
- don't recompute rawname-version needlessly (Luca)
---
  package/pkg-generic.mk           | 12 +++++++-----
  support/legal-info/README.header |  3 +--
  2 files changed, 8 insertions(+), 7 deletions(-)
package/pkg-generic.mk +7 -5
index 9b9a27f..f873e9c 100644
@@ -760,12 +760,10 @@ $(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),)
+# We need to extract and patch a package to be able to retrieve its
+# license files (if any) and the list of patches applied to it (if
+# any).
  $(1)-legal-info: $(1)-patch
-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
@@ -827,6 +825,10 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
  	$$(Q)$$(call hardlink-copy,\
  		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
  		     $$($(2)_REDIST_SOURCES_DIR))
+	$$(Q)while read f; do \
+		$$(call hardlink-copy,$$$${f},$$($(2)_REDIST_SOURCES_DIR)) || exit 1; \
+		printf "%s\n" "$$$${f##*/}" >>$$($(2)_REDIST_SOURCES_DIR)/series || exit 1; \
+	done <$$($(2)_DIR)/.applied_patches_list
  endif # redistribute
 
  endif # other packages
support/legal-info/README.header +0 -0
index d07c45d..4d7fd7c 100644

support/legal-info/README.header This material is composed of the following items. · line 19

Quoted 6 lines

support/legal-info/README.header +1 -2

@@ -16,8 +16,7 @@ This material is composed of the following items.
     need to collect it manually.
   * The source code for all packages; this has been saved in the sources/
     subdirectory (except for the non-redistributable packages, which have not
-   been saved); patches applied to some packages by Buildroot are included in
-   the Buildroot sources and were not duplicated in the sources/ subdirectory.
+   been saved).

Now we could explicitly state we also save patches. E.g. changing the above bullet to:

* The original source code for all packages; this has been saved in...

        ^^^^^^^^

And adding:

* The patches applied to the source code before building it, along with a file named 'series' that lists the same patches in the order they have been applied.

But this can be done in a later commit, so:

Signature · 1 line

Luca

Yann E. MORIN

author

2015-12-18 23:07 UTC #29 lore raw

PATCH v2 9/13 core/legal-info: also save patches

Luca, All,

On 2015-12-17 11:45 +0100, Luca Ceresoli spake thusly:

Quoted 4 lines

Quoted 3 lines · Yann E. MORIN
Currently, the legal-info infra only saves the source archive of a package. However, that's not enough as we may apply some patches on packages sources.

[--SNIP--]

Quoted 15 lines

support/legal-info/README.header This material is composed of the following items. · line 19
Quoted 11 lines
support/legal-info/README.header +1 -2
index d07c45d..4d7fd7c 100644
@@ -16,8 +16,7 @@ This material is composed of the following items.
    need to collect it manually.
  * The source code for all packages; this has been saved in the sources/
    subdirectory (except for the non-redistributable packages, which have not
-   been saved); patches applied to some packages by Buildroot are included in
-   the Buildroot sources and were not duplicated in the sources/ subdirectory.
+   been saved).
Now we could explicitly state we also save patches. E.g. changing the above bullet to:

Quoted 8 lines

* The original source code for all packages; this has been saved in...
       ^^^^^^^^
And adding:
* The patches applied to the source code before building it, along with a file named 'series' that lists the same patches in the order they have been applied.

I've used sa slightly different wording:

* The source code for all packages; this has been saved in the sources/ subdirectory (except for the non-redistributable packages, which have not been saved). Patches that were applied are also saved, along with a file named 'series' that lists the patches in the order they were applied.

Quoted 2 lines

Reviewed-by Luca Ceresoli <[email protected]>
Tested-by Luca Ceresoli <[email protected]>

Do they still stand with the above wording?

Thanks! :-)

Regards,
Yann E. MORIN.

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Yann E. MORIN

author

2015-12-13 18:35 UTC #30 lore raw

PATCH v2 10/13 core/legal-info: also save extra downloads

Some packages, like perl, download extra files that end up as part of the source that Biuldroot builds. Up until now, those files were not saved in the legal-info output.

Add those files to the legal-info output.

Cc Luca Ceresoli <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
The unfortnate side-effect is that we will also save the secondary archive for the external blackfin toolchains; however, we already to save the binary release of some external toolchains when they do not provide actual source archives.

This is inherently bad, but solving this is a bigger concern, for another series...

---
 package/pkg-generic.mk | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

package/pkg-generic.mk +7 -4

index f873e9c..7468e54 100644
@@ -821,10 +821,13 @@ else
 # Other packages
 
 ifeq ($$($(2)_REDISTRIBUTE),YES)
-# Copy the source tarball
-	$$(Q)$$(call hardlink-copy,\
-		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
-		     $$($(2)_REDIST_SOURCES_DIR))
+# Copy the source tarball and eny extra downloads, but not
+# patches, as they are handled specially afterwards.
+	$$(foreach e,$$($(2)_ACTUAL_SOURCE_TARBALL) $$(notdir $$($(2)_EXTRA_DOWNLOADS)),\
+			$$(Q)$$(call hardlink-copy,\
+				$$(DL_DIR)/$$(e),\
+				$$($(2)_REDIST_SOURCES_DIR))$$(sep))
+# Copy patches and generate the series file
 	$$(Q)while read f; do \
 		$$(call hardlink-copy,$$$${f},$$($(2)_REDIST_SOURCES_DIR)) || exit 1; \
 		printf "%s\n" "$$$${f##*/}" >>$$($(2)_REDIST_SOURCES_DIR)/series || exit 1; \

Signature · 1 line

1.9.1

Luca Ceresoli

2015-12-17 10:57 UTC #31 lore raw

PATCH v2 10/13 core/legal-info: also save extra downloads

Dear Yann,

Quoted 2 lines · Yann E. MORIN

Some packages, like perl, download extra files that end up as part of the source that Biuldroot builds. Up until now, those files were not

Biuldroot -> Buildroot

Quoted 8 lines

saved in the legal-info output.
Add those files to the legal-info output.
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Luca Ceresoli <[email protected]>
Cc Thomas Petazzoni <[email protected]>

Quoted 5 lines

---
The unfortnate side-effect is that we will also save the secondary archive for the external blackfin toolchains; however, we already to save the binary release of some external toolchains when they do not provide actual source archives.

For legal-info, saving one more file is better than missing one. At least if it's an external toolchain that anybody can anyway download from the Internet. So I agree with your choice.

Quoted 3 lines

This is inherently bad, but solving this is a bigger concern, for another series...

Agreed.

Quoted 9 lines

---
  package/pkg-generic.mk | 11 +++++++----
  1 file changed, 7 insertions(+), 4 deletions(-)

package/pkg-generic.mk +0 -0

index f873e9c..7468e54 100644

package/pkg-generic.mk else · line 824

Quoted 8 lines

package/pkg-generic.mk +1 -4

@@ -821,10 +821,13 @@ else
  # Other packages
 
  ifeq ($$($(2)_REDISTRIBUTE),YES)
-# Copy the source tarball
-	$$(Q)$$(call hardlink-copy,\
-		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
-		     $$($(2)_REDIST_SOURCES_DIR))
+# Copy the source tarball and eny extra downloads, but not

eny -> any

package/pkg-generic.mk else · line 829

Quoted 5 lines

package/pkg-generic.mk +5 -0

+# patches, as they are handled specially afterwards.
+	$$(foreach e,$$($(2)_ACTUAL_SOURCE_TARBALL) $$(notdir $$($(2)_EXTRA_DOWNLOADS)),\
+			$$(Q)$$(call hardlink-copy,\
+				$$(DL_DIR)/$$(e),\
+				$$($(2)_REDIST_SOURCES_DIR))$$(sep))

And -legal-info depends on .stamp_downloaded which fetches the extra_downloads, so here we shouldn't have the problem noticed in patch 4.

package/pkg-generic.mk else · line 830

Quoted 1 line

package/pkg-generic.mk +1 -0
+# Copy patches and generate the series file

Not strictly related to this patch, this line should be in patch 9 ("core/legal-info: also save patches").

Anyway the code looks good, and it's a rather small diff, so:

Signature · 1 line

Luca

Yann E. MORIN

author

2015-12-13 18:35 UTC #32 lore raw

PATCH v2 11/13 core/legal-info: generate a hash of all saved files

Having a hash of the saved files can be interesting for the recipient to verify the integrity of the files.

We remove the warning file earlier, to exclude it from the hash list.

Cc Luca Ceresoli <[email protected]>

---
Changes v1 -> v2:
- simplify getting rid of the ..../legal-info/ prefix (Luca)
- always sort with the C locale

---
 Makefile | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Makefile +7 -1

index 8e8514e..cc31211 100644
@@ -654,8 +654,14 @@ legal-info: dirs legal-info-clean legal-info-prepare $(foreach p,$(PACKAGES),$(p
 		cat support/legal-info/README.warnings-header \
 			$(LEGAL_WARNINGS) >>$(LEGAL_REPORT); \
 		cat $(LEGAL_WARNINGS); fi
-	@echo "Legal info produced in $(LEGAL_INFO_DIR)"
 	@rm -f $(LEGAL_WARNINGS)
+	@(cd $(LEGAL_INFO_DIR); \
+	  find * -type f -exec sha256sum {} + \
+	  |LC_ALL=C sort -k2 \
+	  >.legal-info.sha256; \
+	  mv .legal-info.sha256 legal-info.sha256 \
+	)
+	@echo "Legal info produced in $(LEGAL_INFO_DIR)"
 
 show-targets:
 	@echo $(PACKAGES) $(TARGETS_ROOTFS)

Signature · 1 line

1.9.1

Yann E. MORIN

author

2015-12-13 18:35 UTC #33 lore raw

PATCH v2 12/13 core/legal-info: allow ignoring packages from the legal-info

It might be necessary to not even mention a package in the output of legal-info:

- virtual package have virtually nothing to save in the legal-info

    output;

- for Buildroot itself, host-gcc-initial and host-gcc-final are not real packages, they are just two different steps of the same package, gcc;

- for proprietary packages, it might not even be legal to even mention them, being under NDA or some other such restrictive conditions.

Itroduce the new FOO_LEGAL_INGORE variable that a package can set to 'YES' (default to 'NO') to indicate that the package should be completely ignored from the legal-info output, in which case the package is not metionned in the maniufest, its source archive, patches and license files are not saved into legal-info/ .

Cc Luca Ceresoli <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

---
Changes v1 -> v2:
- introduce a new variable, instead of making _REDISTRIBUTE a tri-state (Thomas, Peter, Luca)

---
 docs/manual/adding-packages-generic.txt | 9 +++++++++
 package/pkg-generic.mk                  | 7 +++++++
 2 files changed, 16 insertions(+)

docs/manual/adding-packages-generic.txt +9 -0

index 1c25c4e..1219292 100644
@@ -416,6 +416,15 @@ information is (assuming the package name is +libfoo+) :
   non-opensource packages: Buildroot will not save the source code for this
   package when collecting the +legal-info+.
 
+* +LIBFOO_LEGAL_IGNORE+ can be set to +YES+ or +NO+ (the default) to indicate
+  that this package should be completely ignored when saving the licensing
+  information. If set to +YES+, then the package is not listed in the manifest,
+  its source archive and its license files are not saved. You probably do not
+  want to set it to +YES+, unless under very specific conditions (e.g. when
+  you use the `legal-info/` output directory as-is to provide a compliance
+  delivery, and do not want your proprietary, non-redistributable packages to
+  even be mentionned in the manifest).
+
 * +LIBFOO_FLAT_STACKSIZE+ defines the stack size of an application built into
   the FLAT binary format. The application stack size on the NOMMU architecture
   processors can't be enlarged at run time. The default stack size for the

package/pkg-generic.mk +7 -0

index 7468e54..6ede09d 100644
@@ -763,7 +763,10 @@ $(2)_MANIFEST_LICENSE_FILES ?= not saved
 # We need to extract and patch a package to be able to retrieve its
 # license files (if any) and the list of patches applied to it (if
 # any).
+# But not if we want to ignore that package completely.
+ifneq ($$($(2)_LEGAL_IGNORE),YES)
 $(1)-legal-info: $(1)-patch
+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

@@ -793,6 +796,8 @@ endif # actuall source != ""
 
 # legal-info: produce legally relevant info.
 $(1)-legal-info:
+ifneq ($$($(2)_LEGAL_IGNORE),YES)
+
 # Packages without a source are assumed to be part of Buildroot, skip them.
 	$$(foreach hook,$$($(2)_PRE_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))
 ifneq ($$(call qstrip,$$($(2)_SOURCE)),)

@@ -839,6 +844,8 @@ endif # other packages
 endif # ifneq ($$(call qstrip,$$($(2)_SOURCE)),)
 	$$(foreach hook,$$($(2)_POST_LEGAL_INFO_HOOKS),$$(call $$(hook))$$(sep))
 
+endif # $(2)_LEGAL_IGNORE != YES
+
 # add package to the general list of targets if requested by the buildroot
 # configuration
 ifeq ($$($$($(2)_KCONFIG_VAR)),y)

Signature · 1 line

1.9.1

Yann E. MORIN

author

2015-12-13 18:35 UTC #34 lore raw

PATCH v2 13/13 core/pkg-virtual: ignore from legal-info output

Virtual packages really have nothing to save for legal-info, so we just ignore them.

Currently, packages are only extracted if they define _LICENSE_FILES. Follow-up patches will always extract and patch non-ignored packages, so we would potentially see spurious "extracting" and "patching" messages for virtual packages on the console, which would be a bit strange. Ignoring virtual packages will get us a cleaner log.