[Buildroot] [PATCH 01/13 v2] core/legal-info: update the legal-info report header

[Buildroot] [PATCH 01/13 v2] core/legal-info: update the legal-info report header

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

Update the header acoordingly.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Acked-by: Luca Ceresoli <luca@lucaceresoli.net>

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

diff --git a/support/legal-info/README.header b/support/legal-info/README.header
index 3321adb..d07c45d 100644
--- a/support/legal-info/README.header
+++ b/support/legal-info/README.header
@@ -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.
-- 
1.9.1

[Buildroot] [PATCH 02/13 v2] core/pkg-utils: add macro to hardlink-or-copy

[Yann E. MORIN]

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

---
Changes RFC -> v1:
  - move to pkg-utils  (Luca)
---
 package/pkg-utils.mk | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index 44bd2c9..41799b0 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -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
 #
-- 
1.9.1

[Buildroot] [PATCH 03/13 v2] core/legal-info: use the macro to install source archives

[Yann E. MORIN]

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>
---
 package/pkg-generic.mk | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 357a5c7..e69c970 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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
-- 
1.9.1

[Buildroot] [PATCH 04/13 v2] core/legal-info: ensure legal-info works in off-line mode

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

[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" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <jacmet@uclibc.org>

---
Changes v1 -> v2:
  - drop the 'redistribute == ignore', it does not exist yet
---
 package/pkg-generic.mk | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index e69c970..832045e 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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)
-- 
1.9.1

[Buildroot] [PATCH 05/13 v2] core/pkg-generic: add variable to store the package rawname-version

[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
introduce more use of it.

Reported-by: Luca Ceresoli <luca@lucaceresoli.net>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
---
 package/pkg-generic.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 832045e..784c39c 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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
 
-- 
1.9.1

[Buildroot] [PATCH 06/13 v2] core/legal-info: install source archives in their own sub-dir

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

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" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 784c39c..3b6240a 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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
-- 
1.9.1

[Buildroot] [PATCH 07/13 v2] core/legal-info: add package version to license directory

[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
_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" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>

---
Changes v1 -> v2:
  - s/drawback/side-effect/  (Luca)
---
 package/pkg-generic.mk | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 3b6240a..9b9a27f 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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)
-- 
1.9.1

[Buildroot] [PATCH 08/13 v2] core/apply-patches: store full path of applied patches

[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" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>

---
Changes v1 -> v2:
  - do not duplicate '/' in paths  (Luca)
---
 support/scripts/apply-patches.sh | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
index aa13e88..5110458 100755
--- a/support/scripts/apply-patches.sh
+++ b/support/scripts/apply-patches.sh
@@ -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...
-- 
1.9.1

[Buildroot] [PATCH 0/13 v2] legal-info improvements and completeness (branch yem/legal-2)

[Yann E. MORIN]

Hello All!

This series brings improvements to the legal-info infrastructure, so
that we provide the most complete and correct content in the output of
legal-info.

Currently, our legal-info ouput is missing two types of files that might
be important to have:
  - patches
  - extra downloads


The first 9 patches in the series take care of saving patches we apply
to packages:

So far, we've shuffled the patches under the rag, assumong the user
would provide the Buildroot source tree with the compliance delivery, so
that our bundled patches would automatically be included.

However, that's not enough, as not all patches may be in the Buildroot
source tree. That's the case for at least two types of patches:
  - patches that are downloaded,
  - patches from a global patch directory.

In either case, those patches must be provided in the output of
legal-info, because they are not part of Buildroot, so distributing
Buidlroot would not be enough.

Patches that are referenced from Buidlroot woudl probably be OK-ish,
even if not to the letter of the compliance requirements.

That's not much so the case for patches from a global patch dir, since
those would be completely ignored and usually unreachable from a
recipient of the complaince delivery.

So we must save those two types of patches in the output of legal-info.
Because it would be a bit silly to only save those patches, we just save
all of them, whether bundled in buildroot, downloaded or from a global
patch dir alike.


Second, a single patch (#10) takes care of saving extra downloads:

Second, some packages are using extra-downloads to complement the
content of the main archive. That's the case for Perl, for which the
cross-compilation "enabler" is downloaded as a secondary archive via
extra downloads. The Blackfin external toolchains also use extra
downloads to download a secondary archive with the sysroot.

Even though the Blackfin sysroot archive is not really a source, we
still need to provide it along with the main archive, otherwise it's
completely impossible to compile code.

As for the Perl case, however, we're "only" downloading a buildsystem
infrastructure (AFAIU), but without it, it is completely impossible to
cross-compile Perl at all.

So, in both cases, we also need to save the extra downloads.


We then have a patch (#11) that generates a sha256 hash of all the fines
in the legal-info output. It's handy when delivering legal-info/
directly as-is so that the recipient can check the integrity of the
files  her received.


Finally, the last two patches add the possibility to indicate that a
package should be completely ignored from the legal-info output, not
even be mentionned in the manifest. This is useful when the content of
legal-info is used as-is to provide a compliance delivery.


Changes v1 -> v2:
  - keep only the core legal-info patches, drop the gcc/binutils/gdb
    changes (they'll be reworked later, let's focus on the important and
    easier parts first)
  - drop the tristate REDISTRIBUTE, introduce another boolean
    _LEGAL_IGNORE  (Thomas, Peter, Luca)
  - drop the post-legal-info Perl hook, it's no longer needed thanks to
    saving extra downloads  (Thomas, Luca)
  - compute the rwaname-version tuple only once, instead of five times
    (Luca)
  - reorder patches  (Luca)
  - slight commit log rephrasing and corrections  (Luca)


Regards,
Yann E. MORIN.


The following changes since commit 27d6deb449fdb3b7662059a83212787ae568c76e:

  python-pyudev: new package (2015-12-13 16:03:26 +0100)

are available in the git repository at:

  git://git.busybox.net/~ymorin/git/buildroot yem/legal-2

for you to fetch changes up to 25dd1d1f85baf34f5b4d8cfda7a4c3bd17687ccc:

  core/pkg-virtual: ignore from legal-info output (2015-12-13 19:26:53 +0100)

----------------------------------------------------------------
Yann E. MORIN (13):
      core/legal-info: update the legal-info report header
      core/pkg-utils: add macro to hardlink-or-copy
      core/legal-info: use the macro to install source archives
      core/legal-info: ensure legal-info works in off-line mode
      core/pkg-generic: add variable to store the package rawname-version
      core/legal-info: install source archives in their own sub-dir
      core/legal-info: add package version to license directory
      core/apply-patches: store full path of applied patches
      core/legal-info: also save patches
      core/legal-info: also save extra downloads
      core/legal-info: generate a hash of all saved files
      core/legal-info: allow ignoring packages from the legal-info
      core/pkg-virtual: ignore from legal-info output

 Makefile                                |  8 +++-
 docs/manual/adding-packages-generic.txt |  9 ++++
 docs/manual/adding-packages-virtual.txt |  3 ++
 package/pkg-generic.mk                  | 74 +++++++++++++++++++++++----------
 package/pkg-utils.mk                    | 18 ++++++++
 package/pkg-virtual.mk                  |  2 +
 support/legal-info/README.header        |  9 ++--
 support/scripts/apply-patches.sh        | 11 +++--
 8 files changed, 104 insertions(+), 30 deletions(-)

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

[Buildroot] [PATCH 09/13 v2] core/legal-info: also save patches

[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" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 9b9a27f..f873e9c 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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
diff --git a/support/legal-info/README.header b/support/legal-info/README.header
index d07c45d..4d7fd7c 100644
--- a/support/legal-info/README.header
+++ b/support/legal-info/README.header
@@ -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.
-- 
1.9.1

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

[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
saved in the legal-info output.

Add those files to the legal-info output.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index f873e9c..7468e54 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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; \
-- 
1.9.1

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

[Yann E. MORIN]

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.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>

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

diff --git a/Makefile b/Makefile
index 8e8514e..cc31211 100644
--- a/Makefile
+++ b/Makefile
@@ -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)
-- 
1.9.1

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

[Yann E. MORIN]

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

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <jacmet@uclibc.org>

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

diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
index 1c25c4e..1219292 100644
--- a/docs/manual/adding-packages-generic.txt
+++ b/docs/manual/adding-packages-generic.txt
@@ -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
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 7468e54..6ede09d 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -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)
-- 
1.9.1

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

[Yann E. MORIN]

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.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <jacmet@uclibc.org>

---
Changes v1 -> v2:
  - use the new _LEGAL_IGNORE variable  (Thomas, Luca, Peter)
---
 docs/manual/adding-packages-virtual.txt | 3 +++
 package/pkg-virtual.mk                  | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/docs/manual/adding-packages-virtual.txt b/docs/manual/adding-packages-virtual.txt
index a5f17a2..11a497d 100644
--- a/docs/manual/adding-packages-virtual.txt
+++ b/docs/manual/adding-packages-virtual.txt
@@ -56,6 +56,9 @@ The +.mk+ for the virtual package should just evaluate the +virtual-package+ mac
 The ability to have target and host packages is also available, with the
 +host-virtual-package+ macro.
 
+.Note:
+Virtual packages are automatically ignored when generating the +legal-info+ output.
+
 ==== Provider's +Config.in+ file
 
 When adding a package as a provider, only the +Config.in+ file requires some
diff --git a/package/pkg-virtual.mk b/package/pkg-virtual.mk
index 9c68b51..c347ba3 100644
--- a/package/pkg-virtual.mk
+++ b/package/pkg-virtual.mk
@@ -57,6 +57,8 @@ endif
 # Add dependency against the provider
 $(2)_DEPENDENCIES += $$(call qstrip,$$(BR2_PACKAGE_PROVIDES_$(2)))
 
+$(2)_LEGAL_IGNORE = YES
+
 # Call the generic package infrastructure to generate the necessary
 # make targets
 $(call inner-generic-package,$(1),$(2),$(3),$(4))
-- 
1.9.1

[Buildroot] [PATCH 04/13 v2] core/legal-info: ensure legal-info works in off-line mode

[Luca Ceresoli]

Dear Yann,

Yann E. MORIN wrote:
> 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...
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Peter Korsgaard <jacmet@uclibc.org>

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

Just a couple of minor notes below.

[...]
> +# 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

> +endif # actuall source != ""

actuall -> actual

With these fixed:
  Acked-by: Luca Ceresoli <luca@lucaceresoli.net>

[Tested with a minimal Sourcery ARM 2014.05 config]
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

-- 
Luca

[Buildroot] [PATCH 05/13 v2] core/pkg-generic: add variable to store the package rawname-version

[Luca Ceresoli]

Dear Yann,

Yann E. MORIN wrote:
> 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

> introduce more use of it.
>
> Reported-by: Luca Ceresoli <luca@lucaceresoli.net>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>

With the typo fixed:
  Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>

-- 
Luca

[Buildroot] [PATCH 07/13 v2] core/legal-info: add package version to license directory

[Luca Ceresoli]

Dear Yann,

Yann E. MORIN wrote:
> 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

> _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" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
>
> ---
> Changes v1 -> v2:
>    - s/drawback/side-effect/  (Luca)
> ---
>   package/pkg-generic.mk | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 3b6240a..9b9a27f 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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))

Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

-- 
Luca

[Buildroot] [PATCH 01/13 v2] core/legal-info: update the legal-info report header

[Arnout Vandecappelle]

On 13-12-15 19:35, Yann E. MORIN wrote:
> 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

> 
> Update the header acoordingly.

 accordingly

> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Acked-by: Luca Ceresoli <luca@lucaceresoli.net>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

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

 Regards,
 Arnout

> 
> ---
> 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(-)
> 
> diff --git a/support/legal-info/README.header b/support/legal-info/README.header
> index 3321adb..d07c45d 100644
> --- a/support/legal-info/README.header
> +++ b/support/legal-info/README.header
> @@ -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.
> 


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

[Buildroot] [PATCH 02/13 v2] core/pkg-utils: add macro to hardlink-or-copy

[Arnout Vandecappelle]

On 13-12-15 19:35, Yann E. MORIN wrote:
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> 
> ---
> Changes RFC -> v1:
>   - move to pkg-utils  (Luca)
> ---
>  package/pkg-utils.mk | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> index 44bd2c9..41799b0 100644
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -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
>  #
> 


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

[Buildroot] [PATCH 03/13 v2] core/legal-info: use the macro to install source archives

[Arnout Vandecappelle]

On 13-12-15 19:35, Yann E. MORIN wrote:
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

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

 Regards,
 Arnout

> ---
>  package/pkg-generic.mk | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 357a5c7..e69c970 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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
> 


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

[Buildroot] [PATCH 04/13 v2] core/legal-info: ensure legal-info works in off-line mode

[Luca Ceresoli]

Dear Yann,

Luca Ceresoli wrote:
[...]
>> +# 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
>
>> +endif # actuall source != ""
>
> actuall -> actual
>
> With these fixed:
>   Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
>
> [Tested with a minimal Sourcery ARM 2014.05 config]
> Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

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.

-- 
Luca

[Buildroot] [PATCH 04/13 v2] core/legal-info: ensure legal-info works in off-line mode

[Yann E. MORIN]

Luca, All,

On 2015-12-16 23:47 +0100, Luca Ceresoli spake thusly:
> Luca Ceresoli wrote:
> [...]
> >>+# 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
> >
> >>+endif # actuall source != ""
> >
> >actuall -> actual
> >
> >With these fixed:
> >  Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
> >
> >[Tested with a minimal Sourcery ARM 2014.05 config]
> >Tested-by: Luca Ceresoli <luca@lucaceresoli.net>
> 
> Hey, NO! This patch is wrong.
[--SNIP--]
> 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

Damit... :-(

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

> Sorry for having missed this during my first review.

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

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH 04/13 v2] core/legal-info: ensure legal-info works in off-line mode

[Arnout Vandecappelle]

On 17-12-15 00:05, Yann E. MORIN wrote:
> Luca, All,
> 
> On 2015-12-16 23:47 +0100, Luca Ceresoli spake thusly:
>> Luca Ceresoli wrote:
>> [...]
>>>> +# 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
>>>
>>>> +endif # actuall source != ""
>>>
>>> actuall -> actual
>>>
>>> With these fixed:
>>>  Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
>>>
>>> [Tested with a minimal Sourcery ARM 2014.05 config]
>>> Tested-by: Luca Ceresoli <luca@lucaceresoli.net>
>>
>> Hey, NO! This patch is wrong.
> [--SNIP--]
>> 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
> 
> Damit... :-(
> 
>> 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.

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

 Regards,
 Arnout

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


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

[Buildroot] [PATCH 04/13 v2] core/legal-info: ensure legal-info works in off-line mode

[Yann E. MORIN]

Arnout, All,

On 2015-12-17 00:09 +0100, Arnout Vandecappelle spake thusly:
> On 17-12-15 00:05, Yann E. MORIN wrote:
> > On 2015-12-16 23:47 +0100, Luca Ceresoli spake thusly:
> >> Luca Ceresoli wrote:
> >> [...]
> >>>> +# 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
> >>>
> >>>> +endif # actuall source != ""
> >>>
> >>> actuall -> actual
> >>>
> >>> With these fixed:
> >>>  Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
> >>>
> >>> [Tested with a minimal Sourcery ARM 2014.05 config]
> >>> Tested-by: Luca Ceresoli <luca@lucaceresoli.net>
> >>
> >> Hey, NO! This patch is wrong.
> > [--SNIP--]
> >> 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
> > 
> > Damit... :-(
> > 
> >> 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.
> 
>  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.

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

[Buildroot] [PATCH 04/13 v2] core/legal-info: ensure legal-info works in off-line mode

[Arnout Vandecappelle]

On 13-12-15 19:35, Yann E. MORIN wrote:
> 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;

 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.

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

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.


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

> 
>   - 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" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Peter Korsgaard <jacmet@uclibc.org>
> 
> ---
> Changes v1 -> v2:
>   - drop the 'redistribute == ignore', it does not exist yet
> ---
>  package/pkg-generic.mk | 36 ++++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index e69c970..832045e 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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?

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

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

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

> +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 \

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


 Regards,
 Arnout

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


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

[Buildroot] [PATCH 05/13 v2] core/pkg-generic: add variable to store the package rawname-version

[Arnout Vandecappelle]

On 13-12-15 19:35, Yann E. MORIN wrote:
> 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.

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

 follow-up

> introduce more use of it.
> 
> Reported-by: Luca Ceresoli <luca@lucaceresoli.net>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> ---
>  package/pkg-generic.mk | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 832045e..784c39c 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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) <arnout@mind.be>

 Regards,
 Arnout

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


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

[Buildroot] [PATCH 06/13 v2] core/legal-info: install source archives in their own sub-dir

[Arnout Vandecappelle]

On 13-12-15 19:35, Yann E. MORIN wrote:
> 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).
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
> Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

> 
> ---
> 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(-)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 784c39c..3b6240a 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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

> +
>  # 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
> 


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

[Buildroot] [PATCH 08/13 v2] core/apply-patches: store full path of applied patches

[Luca Ceresoli]

Dear Yann,

Yann E. MORIN wrote:
> 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" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
>
> ---
> Changes v1 -> v2:
>    - do not duplicate '/' in paths  (Luca)
> ---
>   support/scripts/apply-patches.sh | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
> index aa13e88..5110458 100755
> --- a/support/scripts/apply-patches.sh
> +++ b/support/scripts/apply-patches.sh
> @@ -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! :)

Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>

[Tested only with patches in the Buildroot sources]
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

-- 
Luca

[Buildroot] [PATCH 09/13 v2] core/legal-info: also save patches

[Luca Ceresoli]

Dear Yann,

Yann E. MORIN wrote:
> 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" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
>
> ---
> 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(-)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 9b9a27f..f873e9c 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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
> diff --git a/support/legal-info/README.header b/support/legal-info/README.header
> index d07c45d..4d7fd7c 100644
> --- a/support/legal-info/README.header
> +++ b/support/legal-info/README.header
> @@ -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:

Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

-- 
Luca

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

[Luca Ceresoli]

Dear Yann,

Yann E. MORIN wrote:
> 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

> saved in the legal-info output.
>
> Add those files to the legal-info output.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>
> ---
> 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.

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

Agreed.

> ---
>   package/pkg-generic.mk | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index f873e9c..7468e54 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -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

> +# 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.

> +# 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:
Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>

-- 
Luca

[Buildroot] [PATCH 04/13 v2] core/legal-info: ensure legal-info works in off-line mode

[Yann E. MORIN]

Arnout, All,

On 2015-12-17 00:19 +0100, Arnout Vandecappelle spake thusly:
> On 13-12-15 19:35, Yann E. MORIN wrote:
> > 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;
> 
>  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.

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

> >   - 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:
> 
> 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'

> > 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--]
> > +$(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.

> > @@ -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! :-)

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

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

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

[Buildroot] [PATCH 04/13 v2] core/legal-info: ensure legal-info works in off-line mode

[Yann E. MORIN]

Arnout, All,

On 2015-12-18 23:11 +0100, Yann E. MORIN spake thusly:
> On 2015-12-17 00:19 +0100, Arnout Vandecappelle spake thusly:
> > On 13-12-15 19:35, Yann E. MORIN wrote:
[--SNIP--]
> > >   - 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:
> > 
> > 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.

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

[Buildroot] [PATCH 06/13 v2] core/legal-info: install source archives in their own sub-dir

[Yann E. MORIN]

Arnout, All,

On 2015-12-17 00:56 +0100, Arnout Vandecappelle spake thusly:
> On 13-12-15 19:35, Yann E. MORIN wrote:
> > 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).
> > 
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Luca Ceresoli <luca@lucaceresoli.net>
> > Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
> > Tested-by: Luca Ceresoli <luca@lucaceresoli.net>
> 
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
> > 
> > ---
> > 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(-)
> > 
> > diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> > index 784c39c..3b6240a 100644
> > --- a/package/pkg-generic.mk
> > +++ b/package/pkg-generic.mk
> > @@ -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.

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.

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

[Buildroot] [PATCH 09/13 v2] core/legal-info: also save patches

[Yann E. MORIN]

Luca, All,

On 2015-12-17 11:45 +0100, Luca Ceresoli spake thusly:
> Yann E. MORIN wrote:
> >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--]
> >diff --git a/support/legal-info/README.header b/support/legal-info/README.header
> >index d07c45d..4d7fd7c 100644
> >--- a/support/legal-info/README.header
> >+++ b/support/legal-info/README.header
> >@@ -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.

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.

> Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

Do they still stand with the above wording?

Thanks! :-)

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH 06/13 v2] core/legal-info: install source archives in their own sub-dir

[Arnout Vandecappelle]

On 18-12-15 23:53, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2015-12-17 00:56 +0100, Arnout Vandecappelle spake thusly:
>> On 13-12-15 19:35, Yann E. MORIN wrote:
[snip]
>>> +$(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

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