buildroot.busybox.net archive mirror
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 01/16 v5] toolchain/external: add hashes for actual sources
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-19 15:31   ` Thomas Petazzoni
  2016-03-11 17:49 ` [Buildroot] [PATCH 02/16 v5] core/pkg-utils: add macro to hardlink-or-copy Yann E. MORIN
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

As we currently download the actual sources as part of saving the
legal-info, we do not check the hashes of those downloads.

That's because, during legal-info, there is not package involved, and
thus there's no path to an actual .hash file.

However, this precludes legal-info from working in off-line mode. A
subsequent patch will make it possible to do so, and actual sources will
be downloaded as another classical package download.

This will have two consequences:

  - first, we will be able to add hashes for actual sources, so we can
    ensure their integrity,

  - second, and as a direct consequence of the above, when a .hash file
    is present, it would have to list all the hashes for that package,
    or that would be treated as an error.

Currently, the only package that falls in this case is the external-
toolchain, for which we have means to retrieve the sources for some of
the toolchains.

So we just add hashes for those actual external-toolchain sources we may
have to download.

Those hashes are not used for now, but they'll come into play a few
patches down.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 toolchain/toolchain-external/toolchain-external.hash | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/toolchain/toolchain-external/toolchain-external.hash b/toolchain/toolchain-external/toolchain-external.hash
index ac50281..0ed253d 100644
--- a/toolchain/toolchain-external/toolchain-external.hash
+++ b/toolchain/toolchain-external/toolchain-external.hash
@@ -12,30 +12,43 @@ sha256 c65b1b4b918d5185349d62a3b7bf43b4b21e1175c52598ec047ca56b3f11d857  blackfi
 # Mentor's Sourcery CodeBench Lite toolchains
 # ARM
 sha256 39ee0e789034334ecc89af94e838e3a4815400ac5ff980f808f466b04778532e  arm-2014.05-29-arm-none-linux-gnueabi-i686-pc-linux-gnu.tar.bz2
+sha256 e16a5b1e41d7ff1e74161f9405182001bc8d1360d89564e73911032e6966cc0d  arm-2014.05-29-arm-none-linux-gnueabi.src.tar.bz2
 # NiosII
 sha256 cc47745dc1264fcb8fb98fb1315ab772ab98691396021c455229b58abaf887f5  sourceryg++-2015.11-27-nios2-linux-gnu-i686-pc-linux-gnu.tar.bz2
+sha256 8b5710e41685a316d577b503c068c69ba1eaf228fe0a056cac36945323a0dd23  sourceryg++-2015.11-27-nios2-linux-gnu.src.tar.bz2
 # PowerPC
 sha256 525e1f53abbf65c2974ae9af762c45bb38520fe5fc50e968a23fe6a18e9eec04  freescale-2011.03-38-powerpc-linux-gnu-i686-pc-linux-gnu.tar.bz2
+sha256 fe45a1d2725e1b9f0ec7b8041c923e01f461da01b298caad537854ec1c61e211  freescale-2011.03-38-powerpc-linux-gnu.src.tar.bz2
 sha256 d6c94587d546197836e7e1a6909f6aabfa5879e91f501ab03088a6887cc242fc  mentor-2012.03-71-powerpc-mentor-linux-gnu-i686-pc-linux-gnu.tar.bz2
+sha256 4964938da0108a9a5bcc295b0f504a3804021e6e6e596509ee085d9143711b50  mentor-2012.03-71-powerpc-mentor-linux-gnu.src.tar.bz2
 # SuperH
 sha256 59d6766fde244931aa52db01433d5acd051998762a931121c5fc109536a1a802  renesas-2012.09-61-sh-linux-gnu-i686-pc-linux-gnu.tar.bz2
+sha256 e2e58c10e52395d5d35157e35f85233f713c6f9223a652dfc56194cfd2eed004  renesas-2012.09-61-sh-linux-gnu.src.tar.bz2
 # x86
 sha256 ea804cf02014369da52abc4f64e91e96bde2dd2230aca96109459013d4545458  ia32-2012.09-62-i686-pc-linux-gnu-i386-linux.tar.bz2
+sha256 1a9519e415a1e6892c760bf21f7e98f3a633a9d1c5bb8781a96d338e4dd62717  ia32-2012.09-62-i686-pc-linux-gnu.src.tar.bz2
 # AMD64
 sha256 cb4d071db8aefb8005fe72824b96568d93a50f5acd85bacf505a34fe2f265f70  amd-2015.11-36-x86_64-amd-linux-gnu-i686-pc-linux-gnu.tar.bz2
+sha256 a4de11669cddd4594f228db7bd38b0fed3c8692d1bac021739390b4c766bbd4f  amd-2015.11-36-x86_64-amd-linux-gnu.src.tar.bz2
 # Aarch64
 sha256 8ea78c5988b2bb507534f1ad46aa46659f66b39d55f2fc40e163a90b4195e70f  aarch64-2014.05-30-aarch64-linux-gnu-i686-pc-linux-gnu.tar.bz2
+sha256 fef1ba94b43f2aefe5634815745c1cb5442a12d12d98e653601392cb65517b7f  aarch64-2014.05-30-aarch64-linux-gnu.src.tar.bz2
 
 # ARM toolchains from Texas Instrument's Arago project
+# There is one source file that covers both binary distributions.
 sha256 f2febf3b3c565536461ad4405f1bcb835d75a6afb2a8bec958a1248cb4b81fc7  arago-2011.09-armv7a-linux-gnueabi-sdk.tar.bz2
 sha256 254af7d02eb3bcc8345c78e131700bc995d65b68232caaed21150a5fd1456070  arago-2011.09-armv5te-linux-gnueabi-sdk.tar.bz2
+sha256 25fbf0513ad7322b15cbaae964cafadcbb4c939f2708f57f40b8f9f2d601122b  arago-toolchain-2011.09-sources.tar.bz2
 
 # ARM and Aarch64 toolchains from Linaro
 sha256 0cffac0caea0eb3c8bdddfa14be011ce366680f40aeddbefc7cf23cb6d4f1891  gcc-linaro-arm-linux-gnueabihf-4.9-2014.09_linux.tar.xz
+sha256 eafeb3a5247e9ce31aa35d812e296fba5d5f1443e106d9bef9e38d3ee3ade006  gcc-linaro-arm-linux-gnueabihf-4.9-2014.09_src.tar.bz2
 sha256 ff2d231749e59968cb5e7032b4f4e4ae82ff9f11c23967863e627a6c59cb3bc0  gcc-linaro-5.2-2015.11-2-x86_64_arm-linux-gnueabihf.tar.xz
 sha256 4bc9d86390f8fa67a693ba4768ba5b12faaf7dd37c706c05ccd9321e765226e4  gcc-linaro-armeb-linux-gnueabihf-4.9-2014.09_linux.tar.xz
+sha256 bf5d3111dad5aa9aef0e955875fb7fc9e918cb24519af7014dd67a9e581a49b1  gcc-linaro-armeb-linux-gnueabihf-4.9-2014.09_src.tar.bz2
 sha256 e3df0c31d0bd8d1f6d45d585203c0f601e50b1ff7225da1423a8ca36e8caf58f  gcc-linaro-5.2-2015.11-2-x86_64_armeb-linux-gnueabihf.tar.xz
 sha256 3954f496ab01de67241109e82abfaa9b7625fdab4f05e79e7902e9814a07b832  gcc-linaro-aarch64-linux-gnu-4.9-2014.09_linux.tar.xz
+sha256 3954f496ab01de67241109e82abfaa9b7625fdab4f05e79e7902e9814a07b832  gcc-linaro-aarch64-linux-gnu-4.9-2014.09_src.tar.bz2
 sha256 b318a1837a54146b0120a13852386576e38355513b4e2cd5e2125f9c26913777  gcc-linaro-5.2-2015.11-2-x86_64_aarch64-linux-gnu.tar.xz
 
 # Codescape toolchains from Imagination Technologies
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 02/16 v5] core/pkg-utils: add macro to hardlink-or-copy
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
  2016-03-11 17:49 ` [Buildroot] [PATCH 01/16 v5] toolchain/external: add hashes for actual sources Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-19 14:23   ` Thomas Petazzoni
  2016-03-11 17:49 ` [Buildroot] [PATCH 03/16 v5] core/legal-info: use the macro to install source archives Yann E. MORIN
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

This macro will try to copy a source file into a destination directory,
by first attempting to hard-link, and falling back to a plain copy.

In some situations, it will be necessary that the destination file is
named differently than the source (e.g. due to a re-numbering), so if a
third argument is specified, it is treated as the basename of the
destination file.

In future commits, this macro will be passed shell-level variables, e.g.
in a for-loop, so we can't evaluate those at make-level. Thus, we
delegate the full macro to a shell script. We still keep the make macro,
as it is easier to write, and allows to $(strip) its arguments in a
convenient fashion.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Luca Ceresoli <luca@lucaceresoli.net>
Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
[Ran 'make legal-info' with output dir on {the same,another} fs]
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

---
Changes v4 -> v5:
  - move the body of the macro to a shell script  (Luca)

Changes v3 -> v4:
  - forcibly remove destination file first  (Arnout, Luca)
  - typoes  (Luca)
  - drop trailing slash in destination directory name

Changes v2 -> v3;
  - use "ln" instead of "cp -l"
  - accept third argument, as the basename of the destination file
  - drop reviewed-by and tested-by tags given in v2, due to the above
    two changes

Changes RFC -> v1:
  - move to pkg-utils  (Luca)
---
 package/pkg-utils.mk          | 22 ++++++++++++++++++++++
 support/scripts/hardlink-copy | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)
 create mode 100755 support/scripts/hardlink-copy

diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
index f88313a..a5155e8 100644
--- a/package/pkg-utils.mk
+++ b/package/pkg-utils.mk
@@ -113,6 +113,28 @@ $$(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*
+# argument 3 is the basename of the destination file (optional, defaults to
+#            the basename of the source file.
+#
+# examples:
+#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir)
+#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir,new-name)
+#
+# Notes:
+#  - this is NOT an atomic operation,
+#  - this is only a wrapper to a shell script, so that it can be used with
+#    shell-level variables, like in a for loop.
+################################################################################
+define hardlink-copy
+	support/scripts/hardlink-copy "$(strip $(1))" "$(strip $(2))" "$(strip $(3))"
+endef
+
 #
 # legal-info helper functions
 #
diff --git a/support/scripts/hardlink-copy b/support/scripts/hardlink-copy
new file mode 100755
index 0000000..9c1f883
--- /dev/null
+++ b/support/scripts/hardlink-copy
@@ -0,0 +1,34 @@
+#!/bin/bash
+
+# Try to hardlink a file into a directory, fallback to copy on failure.
+#
+# Hardlink-or-copy the source file in the first argument into the
+# destination directory in the second argument, using the basename in
+# the third argument as basename for the destination file. If the third
+# argument is missing, use the basename of the basename of the source
+# file as basename for the destination file.
+#
+# In either case, remove the destination prior to doing the
+# hardlink-or-copy.
+#
+# Note that this is NOT an atomic operation.
+
+set -e
+
+main() {
+    local src_f="${1}"
+    local dst_d="${2}"
+    local dst_f="${3}"
+
+    if [ -n "${dst_f}" ]; then
+        dst_f="${dst_d}/${dst_f}"
+    else
+        dst_f="${dst_d}/$( basename "${src_f}" )"
+    fi
+
+    mkdir -p "${dst_d}"
+    rm -f "${dst_f}"
+    ln -f "${src_f}" "${dst_f}" 2>/dev/null || cp -f "${src_f}" "${dst_f}"
+}
+
+main "${@}"
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 03/16 v5] core/legal-info: use the macro to install source archives
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
  2016-03-11 17:49 ` [Buildroot] [PATCH 01/16 v5] toolchain/external: add hashes for actual sources Yann E. MORIN
  2016-03-11 17:49 ` [Buildroot] [PATCH 02/16 v5] core/pkg-utils: add macro to hardlink-or-copy Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-11 17:49 ` [Buildroot] [PATCH 04/16 v5] core/pkg-generic: reorder variables definitions for legal-info Yann E. MORIN
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

.. and use $(Q) instead of a hard-coded @.

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>

---
Changes v4 -> v5:
  - s/Copy/Save/ because we're not really copying

Changes v2 -> v3:
  - comment the @ -> $(Q) change  (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 3904c09..f47dc7e 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -803,9 +803,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)))
+# Save 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

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 04/16 v5] core/pkg-generic: reorder variables definitions for legal-info
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
                   ` (2 preceding siblings ...)
  2016-03-11 17:49 ` [Buildroot] [PATCH 03/16 v5] core/legal-info: use the macro to install source archives Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-11 17:49 ` [Buildroot] [PATCH 05/16 v5] core/legal-info: ensure legal-info works in off-line mode Yann E. MORIN
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

Move the declarations of _ACTUAL_SOURCE and _ACTUAL_SITE earlier, so
that they are close to where _SOURCE and _SITE are handled.

This looks so far like a purely cosmetic change, but makes more sense
with the follow-up patch, where we'll need them earlier in the file.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Arnout Vandecappelle <arnout@mind.be>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

---
Changes v2 -> v3:
  - move that to its own patch  (Arnout)
---
 package/pkg-generic.mk | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index f47dc7e..8e70eae 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -427,6 +427,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)
@@ -762,14 +770,6 @@ $(1)-legal-info: $(1)-source $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
 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))
-
 # legal-info: produce legally relevant info.
 $(1)-legal-info:
 # Packages without a source are assumed to be part of Buildroot, skip them.
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 05/16 v5] core/legal-info: ensure legal-info works in off-line mode
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
                   ` (3 preceding siblings ...)
  2016-03-11 17:49 ` [Buildroot] [PATCH 04/16 v5] core/pkg-generic: reorder variables definitions for legal-info Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-19 14:47   ` Thomas Petazzoni
  2016-03-11 17:49 ` [Buildroot] [PATCH 06/16 v5] core/pkg-generic: add variable to store the package rawname-version Yann E. MORIN
                   ` (10 subsequent siblings)
  15 siblings, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

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 by adding 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 new rule depends, like the normal download mechanism, on a stamp
file, so that we do not emit a spurious hash-check message on successive
runs of 'make source'.

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 defaults 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>
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

---
Changes v3 -> v4:
  - handle it with a stamp file  (Luca)

Changes v2 -> v3:
  - re-order the PHONY targets  (Arnout)
  - don't reorder setting _ACTUAL_SOURCE/_SITE, it's done in its own
    patch now  (Arnout)
  - adapt the commit log accordingly  (Arnout)
  - typo

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

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 8e70eae..ce5525c 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -123,6 +123,12 @@ $(BUILD_DIR)/%/.stamp_downloaded:
 	$(Q)mkdir -p $(@D)
 	$(Q)touch $@
 
+# Retrieve actual source archive, e.g. for prebuilt external toolchains
+$(BUILD_DIR)/%/.stamp_actual_downloaded:
+	$(call DOWNLOAD,$($(PKG)_ACTUAL_SOURCE_SITE)/$($(PKG)_ACTUAL_SOURCE_TARBALL)); \
+	$(Q)mkdir -p $(@D)
+	$(Q)touch $@
+
 # Unpack the archive
 $(BUILD_DIR)/%/.stamp_extracted:
 	@$(call step_start,extract)
@@ -527,6 +533,7 @@ $(2)_TARGET_RSYNC =	        $$($(2)_DIR)/.stamp_rsynced
 $(2)_TARGET_PATCH =		$$($(2)_DIR)/.stamp_patched
 $(2)_TARGET_EXTRACT =		$$($(2)_DIR)/.stamp_extracted
 $(2)_TARGET_SOURCE =		$$($(2)_DIR)/.stamp_downloaded
+$(2)_TARGET_ACTUAL_SOURCE =	$$($(2)_DIR)/.stamp_actual_downloaded
 $(2)_TARGET_DIRCLEAN =		$$($(2)_DIR)/.stamp_dircleaned
 
 # default extract command
@@ -634,6 +641,17 @@ $(1)-depends:		$$($(2)_FINAL_DEPENDENCIES)
 
 $(1)-source:		$$($(2)_TARGET_SOURCE)
 
+$(1)-all-source:	$(1)-legal-source
+$(1)-legal-info:	$(1)-legal-source
+$(1)-legal-source:	$(1)-source
+
+# Only download the actual source if it differs from the 'main' archive
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),)
+ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
+$(1)-legal-source:	$$($(2)_TARGET_ACTUAL_SOURCE)
+endif # actual sources != sources
+endif # actual sources != ""
+
 $(1)-source-check:
 	$$(foreach p,$$($(2)_ALL_DOWNLOADS),$$(call SOURCE_CHECK,$$(p))$$(sep))
 
@@ -659,6 +677,7 @@ $(1)-extract:		$(1)-rsync
 $(1)-rsync:		$$($(2)_TARGET_RSYNC)
 
 $(1)-source:
+$(1)-legal-source:
 
 $(1)-source-check:
 	test -d $$($(2)_OVERRIDE_SRCDIR)
@@ -733,6 +752,8 @@ $$($(2)_TARGET_PATCH):			PKGDIR=$(pkgdir)
 $$($(2)_TARGET_EXTRACT):		PKG=$(2)
 $$($(2)_TARGET_SOURCE):			PKG=$(2)
 $$($(2)_TARGET_SOURCE):			PKGDIR=$(pkgdir)
+$$($(2)_TARGET_ACTUAL_SOURCE):		PKG=$(2)
+$$($(2)_TARGET_ACTUAL_SOURCE):		PKGDIR=$(pkgdir)
 $$($(2)_TARGET_DIRCLEAN):		PKG=$(2)
 
 # Compute the name of the Kconfig option that correspond to the
@@ -800,9 +821,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
 # Save the source tarball
 	$$(Q)$$(call hardlink-copy,\
 		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
@@ -897,6 +915,7 @@ endif
 	$(1)-install-staging \
 	$(1)-install-target \
 	$(1)-legal-info \
+	$(1)-legal-source \
 	$(1)-patch \
 	$(1)-rebuild \
 	$(1)-reconfigure \
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 06/16 v5] core/pkg-generic: add variable to store the package rawname-version
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
                   ` (4 preceding siblings ...)
  2016-03-11 17:49 ` [Buildroot] [PATCH 05/16 v5] core/legal-info: ensure legal-info works in off-line mode Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-19 14:48   ` Thomas Petazzoni
  2016-03-11 17:49 ` [Buildroot] [PATCH 07/16 v5] core/legal-info: install source archives in their own sub-dir Yann E. MORIN
                   ` (9 subsequent siblings)
  15 siblings, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

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 follow-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>
Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 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 ce5525c..312290d 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -393,6 +393,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)
@@ -429,7 +430,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

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 07/16 v5] core/legal-info: install source archives in their own sub-dir
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
                   ` (5 preceding siblings ...)
  2016-03-11 17:49 ` [Buildroot] [PATCH 06/16 v5] core/pkg-generic: add variable to store the package rawname-version Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-19 14:51   ` Thomas Petazzoni
  2016-03-11 17:49 ` [Buildroot] [PATCH 08/16 v5] core/legal-info: add package version to license directory Yann E. MORIN
                   ` (8 subsequent siblings)
  15 siblings, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

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  (Thomas, 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 312290d..31f3e17 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -496,6 +496,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
@@ -825,7 +827,7 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 # Save 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

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 08/16 v5] core/legal-info: add package version to license directory
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
                   ` (6 preceding siblings ...)
  2016-03-11 17:49 ` [Buildroot] [PATCH 07/16 v5] core/legal-info: install source archives in their own sub-dir Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-11 17:49 ` [Buildroot] [PATCH 09/16 v5] core/apply-patches: store full path of applied patches Yann E. MORIN
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

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-effect, 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>
Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
Tested-by: 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 31f3e17..4515b40 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -807,10 +807,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

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 09/16 v5] core/apply-patches: store full path of applied patches
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
                   ` (7 preceding siblings ...)
  2016-03-11 17:49 ` [Buildroot] [PATCH 08/16 v5] core/legal-info: add package version to license directory Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-19 15:03   ` Thomas Petazzoni
  2016-03-11 17:49 ` [Buildroot] [PATCH 10/16 v5] core/legal-info: also save patches Yann E. MORIN
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

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>
Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
[Tested only with patches in the Buildroot sources]
Tested-by: 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 201278d..20a1552 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

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 10/16 v5] core/legal-info: also save patches
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
                   ` (8 preceding siblings ...)
  2016-03-11 17:49 ` [Buildroot] [PATCH 09/16 v5] core/apply-patches: store full path of applied patches Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-11 17:49 ` [Buildroot] [PATCH 11/16 v5] core/legal-info: renumber saved patches Yann E. MORIN
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

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

---
Changes v3 -> v4:
  - typo  (Luca)

Changes v2 -> v3:
  - also mention that patches have been saved  (Luca)

Changes v1 -> v2:
  - don't recompute rawname-version needlessly  (Luca)
---
 package/pkg-generic.mk           | 13 ++++++++-----
 support/legal-info/README.header |  9 +++++----
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 4515b40..3a6f62b 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -778,12 +778,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-overriden (local or true override).
@@ -828,6 +826,11 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 	$$(Q)$$(call hardlink-copy,\
 		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
 		     $$($(2)_REDIST_SOURCES_DIR))
+# Save 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; \
+	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..418de14 100644
--- a/support/legal-info/README.header
+++ b/support/legal-info/README.header
@@ -14,10 +14,11 @@ This material is composed of the following items.
    compiled programs.
    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 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.
+ * The original 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.
  * 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

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 11/16 v5] core/legal-info: renumber saved patches
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
                   ` (9 preceding siblings ...)
  2016-03-11 17:49 ` [Buildroot] [PATCH 10/16 v5] core/legal-info: also save patches Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-19 15:05   ` Thomas Petazzoni
  2016-03-11 17:49 ` [Buildroot] [PATCH 12/16 v5] core/legal-info: also save extra downloads Yann E. MORIN
                   ` (4 subsequent siblings)
  15 siblings, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

Patches we save can come from various locations;
  - bundled with Buildroot
  - downloaded
  - from one or more global-patch-dir

It is possible that two patches lying into different locations have the
same basename, like so (first is bundled, second is from an hypothetical
global-patch-dir):
    package/foo/0001-fix-Makefile.patch
    /path/to/my/patches/foo/0001-fix-Makefile.patch

In that case, we'd save only the second patch, overwriting the first.

We fix that by forcibly prefixing saved patches with a new numbering, to
guarantee the unicity of saved files.

The unfortunate side-effects are that:
  - most saved patches will be twice-numbered,
  - the series file is now redundant.

While the former is mostly not nice-looking, we shouldn't try to strip
any leading numbering, as that might not be a numbering (e.g.
42-retries.patch which is a patch add 42 retries).

The latter is not really problematic. A lot of tools (and people) can
work with, and prefer to have a series file.

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

---
Note that I've made this a separate change, not because the patch would
be too complex if squahed with the previous, but rather to have a more
detailed commit log about the reason for the renumbering; squashing the
two changes together would make for a really long commit log, at the
risk of being more difficult to follow.

Note: an alternative solution was suggested by Luca (and Arnout?), which
would be to check that no two patches have the same basename or bail-out
otherwise. I choose to keep the renumbering, because it follows the path
of least resistance (i.e. it does not break existing setups).
---
 package/pkg-generic.mk | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 3a6f62b..ed139fb 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -827,9 +827,16 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
 		     $$($(2)_REDIST_SOURCES_DIR))
 # Save 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; \
+# Because patches may come from various places (bundled in Buildroot,
+# from one or more global-patch-dir), there might be collisions on the
+# basenames of those files.
+# We add a new numbering to each patch to ensure unicity of the filenames.
+	$$(Q)patch_num=1; while read f; do \
+		prefix_num=$$$$(printf "%04d" $$$${patch_num}); \
+		$$(call hardlink-copy,$$$${f},$$($(2)_REDIST_SOURCES_DIR),\
+			$$$${prefix_num}-$$$${f##*/}) || exit 1; \
+		printf "%s\n" "$$$${prefix_num}-$$$${f##*/}" >>$$($(2)_REDIST_SOURCES_DIR)/series || exit 1; \
+		: $$$$((patch_num++)); \
 	done <$$($(2)_DIR)/.applied_patches_list
 endif # redistribute
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3)
@ 2016-03-11 17:49 Yann E. MORIN
  2016-03-11 17:49 ` [Buildroot] [PATCH 01/16 v5] toolchain/external: add hashes for actual sources Yann E. MORIN
                   ` (15 more replies)
  0 siblings, 16 replies; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

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


This series is split in 6 consecutive parts, each depending on the
previous ones:

  - patches 1 adds hashes to all actual sources for pre-built toolchains

  - patches 2-5 make sure legal-info will work in off-line mode;

  - pathces 6-8 reorganise the legal-info directory structure to
    accomodate for the fact that more than one source archive/file may
    be saved for each package;

  - patches 9-12 actually save the patches and extra downloads in the
    legal-info output;

  - patch 13 adds a list of hashes for all files in the legal-info
    output;

  - patches 14-15 add the possibility to ignore packages from the
    legal-info output; this is then used to ignore virtual packages;

  - patch 16 (from Luca) explicits patches licensing.


Why save patches?
-----------------

So far, we've shuffled the patches under the rag, assuming 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 (like patches retrieved at
download time from a http://-or-such scheme to a publicly-reachable
location) would probably be OK-ish, even if not to the letter of the
compliance requirements.

That's not so much the case for patches from a global patch dir, since
those would be completely ignored and usually unreachable from a
recipient of the compliance 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 the non-bundled patches, we
just save all of them, whether bundled in Buildroot, downloaded or from
a global patch dir alike.


Note about saving patches
-------------------------

Buildroot can apply patches from at least three different locations:
  - patches bundled in Buildroot itself,
  - patches download either with FOO_PATCH of FOO_EXTRA_DOWNLOAD
  - patches from one or more BR2_GLOBAL_PATCH_DIR

Since those patches are stored in three different locations, it is
perfectly legit that two patches in two different locations have the
same basename.

However, when saving the patches, the second to be saved would overwrite
the previous one, and would thus cause troubles down the road:

  - the second patch would be applied earlier due to it being referenced
    by the series file, and thus may not apply; even if it applies
    cleanly, it would still be listed a second time in the series, and
    thus would fail to apply that second time,

  - the compliance distribution would not longer be compliant

Hence we have two options at our disposal:

  - rename the patch files so no two patches have the same name; this is
    easily achieved by prefixing them with a monotonically-incremented
    counter, or

  - detect that not two patches have the same basename and fail at
    apply-patch time if that is the case.

This series implements the first solution. The immediate drawback is
that most patches will have two indexes, while the immediate advantage
is that it accepts input patches with the same basename.

The alternative second solution, although tested and implemented (but
not submitted) has an immediate drawback of breaking existing working
setups with patches with the same basename, while there is no real
advantage, except not double-numbering the patches for legal-info.


Why save extra downloads?
-------------------------

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 with just the "main" toolchain.

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.



Changes v4 -> v5:
  - fix the macro by making it  ashell script  (Luca)
  - typoes

Changes v3 -> v4:
  - add hashes for sources to more CodeSourcery pre-built toolchains
    (Arnout)
  - fix hardlink-or-copy when the destination may already exist
    (Arnout, Luca)
  - handle downloading actual sources with a stamp file  (Luca)
  - rephrase manual about ignoring packages  (Luca)
  - typoes  (Luca)
  - add first patch, to fix fetching sources for Linaro pre-built
    toolchains

Changes v2 -> v3:
  - re-order variables in their own patch  (Arnout)
  - update legal-info header about the patches  (Luca)
  - add hashes for external toolchains sources  (Luca, Arnout)
  - misc and typoes  (Arnout, Luca)
  - enhance the hardlink-or-copy macro

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 rawname-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 0b7b84310c3781c346eea4c114c8cb4e97bbd8ea:

  olimex_imx233_olinuxino_defconfig: genimage support (2016-03-11 13:14:54 +0100)

are available in the git repository at:

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

for you to fetch changes up to d6c5a79915775eaa9d1a6d1711a4955cafe221cd:

  legal-info: explicitly state how patches are licensed (2016-03-11 18:11:54 +0100)

----------------------------------------------------------------
Luca Ceresoli (1):
      legal-info: explicitly state how patches are licensed

Yann E. MORIN (15):
      toolchain/external: add hashes for actual sources
      core/pkg-utils: add macro to hardlink-or-copy
      core/legal-info: use the macro to install source archives
      core/pkg-generic: reorder variables definitions for legal-info
      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: renumber saved 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            |  5 ++
 package/pkg-generic.mk                             | 94 +++++++++++++++++-----
 package/pkg-utils.mk                               | 22 +++++
 package/pkg-virtual.mk                             |  2 +
 support/legal-info/README.header                   |  9 ++-
 support/scripts/apply-patches.sh                   | 11 ++-
 support/scripts/hardlink-copy                      | 34 ++++++++
 .../toolchain-external/toolchain-external.hash     | 13 +++
 10 files changed, 177 insertions(+), 30 deletions(-)
 create mode 100755 support/scripts/hardlink-copy

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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 12/16 v5] core/legal-info: also save extra downloads
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
                   ` (10 preceding siblings ...)
  2016-03-11 17:49 ` [Buildroot] [PATCH 11/16 v5] core/legal-info: renumber saved patches Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-19 15:14   ` Thomas Petazzoni
  2016-03-11 17:49 ` [Buildroot] [PATCH 13/16 v5] core/legal-info: generate a hash of all saved files Yann E. MORIN
                   ` (3 subsequent siblings)
  15 siblings, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

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

Add those files to the legal-info output.

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

This is inherently bad, as those are not source archives, but solving
this is a bigger concern, for another series...

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

---
Changes v2 -> v3:
  - typo  (Luca)
  - incorporate the post-commit log message (the part about the
    side-effect) into the commit log itself, it makes sense to not
    forget about that
---
 package/pkg-generic.mk | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index ed139fb..bc39ead 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -822,10 +822,12 @@ else
 # Other packages
 
 ifeq ($$($(2)_REDISTRIBUTE),YES)
-# Save the source tarball
-	$$(Q)$$(call hardlink-copy,\
-		     $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
-		     $$($(2)_REDIST_SOURCES_DIR))
+# Save the source tarball and any 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))
 # Save patches and generate the series file
 # Because patches may come from various places (bundled in Buildroot,
 # from one or more global-patch-dir), there might be collisions on the
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 13/16 v5] core/legal-info: generate a hash of all saved files
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
                   ` (11 preceding siblings ...)
  2016-03-11 17:49 ` [Buildroot] [PATCH 12/16 v5] core/legal-info: also save extra downloads Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-19 15:21   ` Thomas Petazzoni
  2016-03-11 17:49 ` [Buildroot] [PATCH 14/16 v5] core/legal-info: allow ignoring packages from the legal-info Yann E. MORIN
                   ` (2 subsequent siblings)
  15 siblings, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

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>
Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
Tested-by: 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 98c8dc7..4c13a9d 100644
--- a/Makefile
+++ b/Makefile
@@ -696,8 +696,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

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 14/16 v5] core/legal-info: allow ignoring packages from the legal-info
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
                   ` (12 preceding siblings ...)
  2016-03-11 17:49 ` [Buildroot] [PATCH 13/16 v5] core/legal-info: generate a hash of all saved files Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-19 15:29   ` Thomas Petazzoni
  2016-03-11 17:49 ` [Buildroot] [PATCH 15/16 v5] core/pkg-virtual: ignore from legal-info output Yann E. MORIN
  2016-03-11 17:49 ` [Buildroot] [PATCH 16/16 v5] legal-info: explicitly state how patches are licensed Yann E. MORIN
  15 siblings, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

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

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

Introduce 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 mentioned 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>
Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

---
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                  | 15 +++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
index 8ed7fe8..5cf2ae0 100644
--- a/docs/manual/adding-packages-generic.txt
+++ b/docs/manual/adding-packages-generic.txt
@@ -430,6 +430,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 mentioned 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 bc39ead..3a5ad83 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -496,6 +496,14 @@ endif
 
 $(2)_REDISTRIBUTE		?= YES
 
+ifndef $(2)_LEGAL_IGNORE
+ ifdef $(3)_LEGAL_IGNORE
+  $(2)_LEGAL_IGNORE = $$($(3)_LEGAL_IGNORE)
+ endif
+endif
+
+$(2)_LEGAL_IGNORE		?= NO
+
 $(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
@@ -781,7 +789,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-overriden (local or true override).
@@ -794,6 +805,8 @@ endif
 
 # 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)),)
@@ -847,6 +860,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

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 15/16 v5] core/pkg-virtual: ignore from legal-info output
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
                   ` (13 preceding siblings ...)
  2016-03-11 17:49 ` [Buildroot] [PATCH 14/16 v5] core/legal-info: allow ignoring packages from the legal-info Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-27 20:47   ` Arnout Vandecappelle
  2016-03-11 17:49 ` [Buildroot] [PATCH 16/16 v5] legal-info: explicitly state how patches are licensed Yann E. MORIN
  15 siblings, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

Virtual packages really have nothing to save for legal-info, so we just
ignore them, to avoid spurious "extracting" and "patching" messages
while running 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 v3 -> v4:
  - rephrase the .Note  (Luca)
  - drop now-incorrect blurb from the commit log  (Luca)

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

diff --git a/docs/manual/adding-packages-virtual.txt b/docs/manual/adding-packages-virtual.txt
index a5f17a2..3857927 100644
--- a/docs/manual/adding-packages-virtual.txt
+++ b/docs/manual/adding-packages-virtual.txt
@@ -56,6 +56,11 @@ 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:
+The +virtual-package+ infrastructure automatically marks virtual packages to be
+excluded from the +legal-info+ output (by internally setting +FOO_LEGAL_IGNORE+
+to 'YES').
+
 ==== 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 244c1d0..445f09a 100644
--- a/package/pkg-virtual.mk
+++ b/package/pkg-virtual.mk
@@ -59,6 +59,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

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 16/16 v5] legal-info: explicitly state how patches are licensed
  2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
                   ` (14 preceding siblings ...)
  2016-03-11 17:49 ` [Buildroot] [PATCH 15/16 v5] core/pkg-virtual: ignore from legal-info output Yann E. MORIN
@ 2016-03-11 17:49 ` Yann E. MORIN
  2016-03-27 20:49   ` Arnout Vandecappelle
  15 siblings, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-11 17:49 UTC (permalink / raw)
  To: buildroot

From: Luca Ceresoli <luca@lucaceresoli.net>

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
---
 support/legal-info/README.header | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/legal-info/README.header b/support/legal-info/README.header
index 418de14..feee104 100644
--- a/support/legal-info/README.header
+++ b/support/legal-info/README.header
@@ -18,7 +18,7 @@ This material is composed of the following items.
    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.
+   applied. Patches are under the same license as the original package.
  * 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

^ permalink raw reply related	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 02/16 v5] core/pkg-utils: add macro to hardlink-or-copy
  2016-03-11 17:49 ` [Buildroot] [PATCH 02/16 v5] core/pkg-utils: add macro to hardlink-or-copy Yann E. MORIN
@ 2016-03-19 14:23   ` Thomas Petazzoni
  2016-03-19 16:08     ` Arnout Vandecappelle
  2016-03-19 23:11     ` Yann E. MORIN
  0 siblings, 2 replies; 46+ messages in thread
From: Thomas Petazzoni @ 2016-03-19 14:23 UTC (permalink / raw)
  To: buildroot

Dear Yann E. MORIN,

On Fri, 11 Mar 2016 18:49:15 +0100, Yann E. MORIN wrote:

> +################################################################################
> +# 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*
> +# argument 3 is the basename of the destination file (optional, defaults to
> +#            the basename of the source file.
> +#
> +# examples:
> +#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir)
> +#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir,new-name)
> +#
> +# Notes:
> +#  - this is NOT an atomic operation,
> +#  - this is only a wrapper to a shell script, so that it can be used with
> +#    shell-level variables, like in a for loop.
> +################################################################################
> +define hardlink-copy
> +	support/scripts/hardlink-copy "$(strip $(1))" "$(strip $(2))" "$(strip $(3))"
> +endef

While in certain cases, I agree that a shell wrapper is nice and
useful, here I really don't see the point of a shell wrapper. You can
simply do (untested):

define hardlink-or-copy-inner
	rm -f $(2)
	ln -f $(1) $(2) 2>/dev/null || cp -f $(1) $(2)
endef

define hardlink-or-copy
	$(call hardlink-or-copy-inner,$(1),$(if $(3),$(2)/$(3),$(2)/$(basename $(1))))
endef

Note that I think the name of the macro should be hardlink-or-copy and
not hardlink-copy. The name hardlink-copy seems to indicate that you
are copying a hardlink, which is not what is happening.
hardlink-or-copy is IMO clearer on the fact that the macro does a hardlink
or a copy.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 05/16 v5] core/legal-info: ensure legal-info works in off-line mode
  2016-03-11 17:49 ` [Buildroot] [PATCH 05/16 v5] core/legal-info: ensure legal-info works in off-line mode Yann E. MORIN
@ 2016-03-19 14:47   ` Thomas Petazzoni
  2016-03-19 18:18     ` Yann E. MORIN
  2016-04-28 21:57     ` Yann E. MORIN
  0 siblings, 2 replies; 46+ messages in thread
From: Thomas Petazzoni @ 2016-03-19 14:47 UTC (permalink / raw)
  To: buildroot

Hello,

On Fri, 11 Mar 2016 18:49:18 +0100, 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 by adding 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 new rule depends, like the normal download mechanism, on a stamp
> file, so that we do not emit a spurious hash-check message on successive
> runs of 'make source'.
> 
> 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.

I am wondering what is the motivation for making "make legal-info"
absolutely executable off-line, after a "make source". I was about to
apply this patch, so my opinion is definitely not firm on this, but
I have some concern:

 * Now "make source" is downloading more stuff than is actually needed
   to do the build. And potentially a *lot* more this is going to
   download the source code for the toolchain, which you absolutely
   don't care about. So for a CodeSoucery ARM toolchain, instead of
   download just the 97 MB of the toolchain, you would download 97 MB +
   278 MB of source code. You're basically quadrupling the data to be
   downloaded for the toolchain itself.

 * Now, after a successful build, "make source" will no longer be a
   no-op operation, because "make source" will also download the files
   needed for legal-info. This is IMO rather unexpected.

Now whether those two concerns are really sufficient to justify a
different implementation can be discussed. One possible solution is to
have a "make legal-info-source", which would download whatever
legal-info needs to be executed offline.

That being said, we're really talking about a few corner cases, because
essentially, only the external toolchains currently make use of
<pkg>_ACTUAL_SOURCE.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 06/16 v5] core/pkg-generic: add variable to store the package rawname-version
  2016-03-11 17:49 ` [Buildroot] [PATCH 06/16 v5] core/pkg-generic: add variable to store the package rawname-version Yann E. MORIN
@ 2016-03-19 14:48   ` Thomas Petazzoni
  2016-03-19 18:20     ` Yann E. MORIN
  0 siblings, 1 reply; 46+ messages in thread
From: Thomas Petazzoni @ 2016-03-19 14:48 UTC (permalink / raw)
  To: buildroot

Dear Yann E. MORIN,

On Fri, 11 Mar 2016 18:49:19 +0100, Yann E. MORIN wrote:

>  $(2)_VERSION := $$(call sanitize,$$($(2)_DL_VERSION))
> +$(2)_RAWNAME_VERSION = $$($(2)_RAWNAME)-$$($(2)_VERSION)

Since we have:

$(2)_BASE_NAME  =  $(1)-$$($(2)_VERSION)

I would prefer this new variable to be declared right after this one,
and named $(2)_RAW_BASE_NAME or $(2)_RAWBASE_NAME.

Thanks

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 07/16 v5] core/legal-info: install source archives in their own sub-dir
  2016-03-11 17:49 ` [Buildroot] [PATCH 07/16 v5] core/legal-info: install source archives in their own sub-dir Yann E. MORIN
@ 2016-03-19 14:51   ` Thomas Petazzoni
  0 siblings, 0 replies; 46+ messages in thread
From: Thomas Petazzoni @ 2016-03-19 14:51 UTC (permalink / raw)
  To: buildroot

Yann,

On Fri, 11 Mar 2016 18:49:20 +0100, 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>

Reviewed-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

I was about to apply this one, but it depends on the hardlink-copy
thingy, for which I had comments.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 09/16 v5] core/apply-patches: store full path of applied patches
  2016-03-11 17:49 ` [Buildroot] [PATCH 09/16 v5] core/apply-patches: store full path of applied patches Yann E. MORIN
@ 2016-03-19 15:03   ` Thomas Petazzoni
  2016-03-19 18:51     ` Yann E. MORIN
  0 siblings, 1 reply; 46+ messages in thread
From: Thomas Petazzoni @ 2016-03-19 15:03 UTC (permalink / raw)
  To: buildroot

Yann,

On Fri, 11 Mar 2016 18:49:22 +0100, Yann E. MORIN wrote:

> diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
> index 201278d..20a1552 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

This seems unrelated to the patch in question, and is not explained
anywhere.

However, I have some more global concern about this approach. I really
dislike the idea that we rely on a helper script filling up a file,
that we later read from a completely different place in the package
infrastructure. It really feels like "let's dump my data there and
fetch it from here".

So here are other proposals:

 * The package infra already knows which patches should be applied
   (bundled patches, global patch dir, etc.), so it is technically able
   to get the list of patches. Yes it's a bit annoying because the
   logic to derive the list of patches is already inside the
   apply-patches script. But maybe it's because too much smart stuff is
   done in the apply-patch script without the package infrastructure
   being aware.

 * Alternatively, add an option to apply-patch.sh that will not apply
   the patches, but show the list of patches that would be applied.
   Like "apply-patch -l" for example. Then, when doing the legal-info,
   you simply call "apply-patch -l" to retrieve the list of patches
   that you need to copy.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 11/16 v5] core/legal-info: renumber saved patches
  2016-03-11 17:49 ` [Buildroot] [PATCH 11/16 v5] core/legal-info: renumber saved patches Yann E. MORIN
@ 2016-03-19 15:05   ` Thomas Petazzoni
  2016-03-19 23:34     ` Yann E. MORIN
  0 siblings, 1 reply; 46+ messages in thread
From: Thomas Petazzoni @ 2016-03-19 15:05 UTC (permalink / raw)
  To: buildroot

Dear Yann E. MORIN,

On Fri, 11 Mar 2016 18:49:24 +0100, Yann E. MORIN wrote:
> Patches we save can come from various locations;
>   - bundled with Buildroot
>   - downloaded
>   - from one or more global-patch-dir
> 
> It is possible that two patches lying into different locations have the
> same basename, like so (first is bundled, second is from an hypothetical
> global-patch-dir):
>     package/foo/0001-fix-Makefile.patch
>     /path/to/my/patches/foo/0001-fix-Makefile.patch
> 
> In that case, we'd save only the second patch, overwriting the first.
> 
> We fix that by forcibly prefixing saved patches with a new numbering, to
> guarantee the unicity of saved files.
> 
> The unfortunate side-effects are that:
>   - most saved patches will be twice-numbered,
>   - the series file is now redundant.
> 
> While the former is mostly not nice-looking, we shouldn't try to strip
> any leading numbering, as that might not be a numbering (e.g.
> 42-retries.patch which is a patch add 42 retries).
> 
> The latter is not really problematic. A lot of tools (and people) can
> work with, and prefer to have a series file.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Luca Ceresoli <luca@lucaceresoli.net>
> Cc: Arnout Vandecappelle <arnout@mind.be>
> 
> ---
> Note that I've made this a separate change, not because the patch would
> be too complex if squahed with the previous, but rather to have a more
> detailed commit log about the reason for the renumbering; squashing the
> two changes together would make for a really long commit log, at the
> risk of being more difficult to follow.
> 
> Note: an alternative solution was suggested by Luca (and Arnout?), which
> would be to check that no two patches have the same basename or bail-out
> otherwise. I choose to keep the renumbering, because it follows the path
> of least resistance (i.e. it does not break existing setups).

I second the position of Luca and Arnout. This renumbering is
complicated and leads to ugly patch names that no longer match the
original file name. So please simply bail-out if two patches have the
same name. It's highly unlikely, so I don't think we should renumber
all the patches and have different file names just to avoid this
unlikely case.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 12/16 v5] core/legal-info: also save extra downloads
  2016-03-11 17:49 ` [Buildroot] [PATCH 12/16 v5] core/legal-info: also save extra downloads Yann E. MORIN
@ 2016-03-19 15:14   ` Thomas Petazzoni
  2016-03-20 16:33     ` Arnout Vandecappelle
  0 siblings, 1 reply; 46+ messages in thread
From: Thomas Petazzoni @ 2016-03-19 15:14 UTC (permalink / raw)
  To: buildroot

Dear Yann E. MORIN,

On Fri, 11 Mar 2016 18:49:25 +0100, Yann E. MORIN wrote:
> Some packages, like perl, download extra files that end up as part of
> the source that Buildroot builds. Up until now, those files were not
> saved in the legal-info output.
> 
> Add those files to the legal-info output.
> 
> The unfortunate side-effect is that we will also save the secondary
> archive for the external blackfin toolchains; however, we already do
> save the binary release of some external toolchains when they do not
> provide actual source archives.
> 
> This is inherently bad, as those are not source archives, but solving
> this is a bigger concern, for another series...

I see two possible ways around this:

 - Have two separate variables, like <pkg>_EXTRA_SOURCE (to be used for
   perl, contains real source code that needs to be saved during
   legal-info), and <pkg>_EXTRA_DOWNLOADS (to be used for external
   toolchains).

 - Just like you don't copy <pkg>_SOURCE to legal-info when
   <pkg>_ACTUAL_SOURCE is defined, don't copy <pkg>_EXTRA_DOWNLOADS
   when <pkg>_ACTUAL_SOURCE is defined.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 13/16 v5] core/legal-info: generate a hash of all saved files
  2016-03-11 17:49 ` [Buildroot] [PATCH 13/16 v5] core/legal-info: generate a hash of all saved files Yann E. MORIN
@ 2016-03-19 15:21   ` Thomas Petazzoni
  2016-03-19 23:40     ` Yann E. MORIN
  0 siblings, 1 reply; 46+ messages in thread
From: Thomas Petazzoni @ 2016-03-19 15:21 UTC (permalink / raw)
  To: buildroot

Yann,

On Fri, 11 Mar 2016 18:49:26 +0100, Yann E. MORIN wrote:
> 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>
> Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
> Tested-by: Luca Ceresoli <luca@lucaceresoli.net>

It's adding more stuff to the main Makefile (while I'd like to remove
stuff from that Makefile), for something that is quite specific. But
OK, I can see the usefulness of this hash file, so:

Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

However, I'm really surprised by this LEGAL_WARNINGS file. It's exactly
the type of hidden file (like .applied_patches) I don't like. One
example where it fails badly is if you interrupt "legal-info" in the
middle. Since the legal-warning and legal-warning-pkg macro only
concatenate to this file, then at the end of the second,
non-interrupted, make invocation, you would got some warnings from the
first run.

Why aren't we simply displaying the warnings as we go? Probably because
they were mixed with some other messages (like the messages about
download some stuff). But in this case, we can serialize the steps by
having all the download done first, then all the "aggregation" done, so
that warnings can be displayed directly without this hidden file.

Of course, this is unrelated to your change, and more a comment for
Luca.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 14/16 v5] core/legal-info: allow ignoring packages from the legal-info
  2016-03-11 17:49 ` [Buildroot] [PATCH 14/16 v5] core/legal-info: allow ignoring packages from the legal-info Yann E. MORIN
@ 2016-03-19 15:29   ` Thomas Petazzoni
  2016-03-19 23:48     ` Yann E. MORIN
  0 siblings, 1 reply; 46+ messages in thread
From: Thomas Petazzoni @ 2016-03-19 15:29 UTC (permalink / raw)
  To: buildroot

Dear Yann E. MORIN,

On Fri, 11 Mar 2016 18:49:27 +0100, Yann E. MORIN wrote:
> It might be necessary to not even mention a package in the output of
> legal-info:
> 
>   - virtual packages 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.

What is the difference with <pkg>_REDISTRIBUTE = NO ? I know
REDISTRIBUTE = NO packages are mentioned in legal-info, but their
source code is not copied to the legal-info stuff.

But does it make sense to have two separate things? Why do REDISTRIBUTE
= NO packages get mentioned in the legal-info if their source code is
anyway not saved.

> Introduce the new FOO_LEGAL_INGORE variable that a package can set

typoe: IGNORE

> 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 mentioned in the maniufest, its source archive,

typo: manifest

> 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>
> Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> Tested-by: Luca Ceresoli <luca@lucaceresoli.net>
> 
> ---
> Changes v1 -> v2:
>   - introduce a new variable, instead of making _REDISTRIBUTE a
>     tri-state  (Thomas, Peter, Luca)

Ah, we discussed using REDISTRIBUTE, I remember. But do we need a
tri-state ? Do we really have REDISTRIBUTE = NO packages that we want
to see mentioned in the legal-info output ?

> +* +LIBFOO_LEGAL_IGNORE+ can be set to +YES+ or +NO+ (the default) to indicate

To me, the naming of the variable looks like inverted logic. What about:

LIBFOO_SAVE_LEGAL_INFO = YES (default) / NO

but obviously, <pkg>_SAVE_LEGAL_INFO is a bit confusing with
<pkg>_REDISTRIBUTE.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 01/16 v5] toolchain/external: add hashes for actual sources
  2016-03-11 17:49 ` [Buildroot] [PATCH 01/16 v5] toolchain/external: add hashes for actual sources Yann E. MORIN
@ 2016-03-19 15:31   ` Thomas Petazzoni
  0 siblings, 0 replies; 46+ messages in thread
From: Thomas Petazzoni @ 2016-03-19 15:31 UTC (permalink / raw)
  To: buildroot

Hello,

On Fri, 11 Mar 2016 18:49:14 +0100, Yann E. MORIN wrote:
> As we currently download the actual sources as part of saving the
> legal-info, we do not check the hashes of those downloads.
> 
> That's because, during legal-info, there is not package involved, and
> thus there's no path to an actual .hash file.
> 
> However, this precludes legal-info from working in off-line mode. A
> subsequent patch will make it possible to do so, and actual sources will
> be downloaded as another classical package download.
> 
> This will have two consequences:
> 
>   - first, we will be able to add hashes for actual sources, so we can
>     ensure their integrity,
> 
>   - second, and as a direct consequence of the above, when a .hash file
>     is present, it would have to list all the hashes for that package,
>     or that would be treated as an error.
> 
> Currently, the only package that falls in this case is the external-
> toolchain, for which we have means to retrieve the sources for some of
> the toolchains.
> 
> So we just add hashes for those actual external-toolchain sources we may
> have to download.
> 
> Those hashes are not used for now, but they'll come into play a few
> patches down.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>  toolchain/toolchain-external/toolchain-external.hash | 13 +++++++++++++
>  1 file changed, 13 insertions(+)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 02/16 v5] core/pkg-utils: add macro to hardlink-or-copy
  2016-03-19 14:23   ` Thomas Petazzoni
@ 2016-03-19 16:08     ` Arnout Vandecappelle
  2016-03-19 23:33       ` Yann E. MORIN
  2016-03-19 23:11     ` Yann E. MORIN
  1 sibling, 1 reply; 46+ messages in thread
From: Arnout Vandecappelle @ 2016-03-19 16:08 UTC (permalink / raw)
  To: buildroot

On 03/19/16 15:23, Thomas Petazzoni wrote:
> Dear Yann E. MORIN,
>
> On Fri, 11 Mar 2016 18:49:15 +0100, Yann E. MORIN wrote:
>
>> +################################################################################
>> +# 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*
>> +# argument 3 is the basename of the destination file (optional, defaults to
>> +#            the basename of the source file.
>> +#
>> +# examples:
>> +#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir)
>> +#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir,new-name)
>> +#
>> +# Notes:
>> +#  - this is NOT an atomic operation,
>> +#  - this is only a wrapper to a shell script, so that it can be used with
>> +#    shell-level variables, like in a for loop.
>> +################################################################################
>> +define hardlink-copy
>> +	support/scripts/hardlink-copy "$(strip $(1))" "$(strip $(2))" "$(strip $(3))"
>> +endef
>
> While in certain cases, I agree that a shell wrapper is nice and
> useful, here I really don't see the point of a shell wrapper. You can
> simply do (untested):
>
> define hardlink-or-copy-inner
> 	rm -f $(2)
> 	ln -f $(1) $(2) 2>/dev/null || cp -f $(1) $(2)
> endef
>
> define hardlink-or-copy
> 	$(call hardlink-or-copy-inner,$(1),$(if $(3),$(2)/$(3),$(2)/$(basename $(1))))
> endef

  I disagree. This may be slightly shorter in lines of code, but I think the 
shell script is much more readable.

  However, I do wonder if it makes sense to have a make function for it if there 
already is a shell script. We don't do that for apply-patches, so why would we 
do it for this one?

>
> Note that I think the name of the macro should be hardlink-or-copy and
> not hardlink-copy. The name hardlink-copy seems to indicate that you
> are copying a hardlink, which is not what is happening.
> hardlink-or-copy is IMO clearer on the fact that the macro does a hardlink
> or a copy.

  I agree with that.

  Regards,
  Arnout

>
> Thanks!
>
> Thomas
>


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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 05/16 v5] core/legal-info: ensure legal-info works in off-line mode
  2016-03-19 14:47   ` Thomas Petazzoni
@ 2016-03-19 18:18     ` Yann E. MORIN
  2016-04-28 21:57     ` Yann E. MORIN
  1 sibling, 0 replies; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-19 18:18 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2016-03-19 15:47 +0100, Thomas Petazzoni spake thusly:
> On Fri, 11 Mar 2016 18:49:18 +0100, 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 by adding 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 new rule depends, like the normal download mechanism, on a stamp
> > file, so that we do not emit a spurious hash-check message on successive
> > runs of 'make source'.
> > 
> > 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.
> 
> I am wondering what is the motivation for making "make legal-info"
> absolutely executable off-line, after a "make source".

I think you (sort of) got it backwards.

The reason is not about doing "make source; make legal-info", but to be
able to do "make source" and store all in a local mirror for example.

Companies like not to rely on third-party /storage/ and want to mirror
the whole world, just in case (and that's sane; one can not rely on a
third party for legal stuff, especially one that is not sub-contracted
to that effect, like most FLOSS projects are).

So, really, the case for "make source" to also retrieve the actual
sources is about being able to not rely on the upstream for legal-info.

> I was about to
> apply this patch, so my opinion is definitely not firm on this, but
> I have some concern:
> 
>  * Now "make source" is downloading more stuff than is actually needed
>    to do the build. And potentially a *lot* more this is going to
>    download the source code for the toolchain, which you absolutely
>    don't care about. So for a CodeSoucery ARM toolchain, instead of
>    download just the 97 MB of the toolchain, you would download 97 MB +
>    278 MB of source code. You're basically quadrupling the data to be
>    downloaded for the toolchain itself.

Yes, this is unfortunate when one is not interested in legal compliance
(e.g. for a purely internal use without redistribution).

>  * Now, after a successful build, "make source" will no longer be a
>    no-op operation, because "make source" will also download the files
>    needed for legal-info. This is IMO rather unexpected.

I see. This can indeed be seen as unexpected.

However, I was pondering another change, whereby the build would
*always* trigger legal-info. I am absolutely not sure about that one,
but I like that "make" generates *all* output artifacts, of which I
consider legal-info to be part. But this is for another day, if ever...

> Now whether those two concerns are really sufficient to justify a
> different implementation can be discussed. One possible solution is to
> have a "make legal-info-source", which would download whatever
> legal-info needs to be executed offline.

I'm not so sure. One interesting point in hooking into "make source" is
that everyine already knows about it. I am not too fond of adding yet
another top-level make target...

But if that would allow for the merging of that series faster, I'd be OK
with that.

> That being said, we're really talking about a few corner cases, because
> essentially, only the external toolchains currently make use of
> <pkg>_ACTUAL_SOURCE.

And when the user has a local download directory outside of Buildroot,
it acts as a cache, and it will be doewnloaded only once.

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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 06/16 v5] core/pkg-generic: add variable to store the package rawname-version
  2016-03-19 14:48   ` Thomas Petazzoni
@ 2016-03-19 18:20     ` Yann E. MORIN
  0 siblings, 0 replies; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-19 18:20 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2016-03-19 15:48 +0100, Thomas Petazzoni spake thusly:
> On Fri, 11 Mar 2016 18:49:19 +0100, Yann E. MORIN wrote:
> >  $(2)_VERSION := $$(call sanitize,$$($(2)_DL_VERSION))
> > +$(2)_RAWNAME_VERSION = $$($(2)_RAWNAME)-$$($(2)_VERSION)
> Since we have:
> 
> $(2)_BASE_NAME  =  $(1)-$$($(2)_VERSION)
> 
> I would prefer this new variable to be declared right after this one,
> and named $(2)_RAW_BASE_NAME or $(2)_RAWBASE_NAME.

OK.

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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 09/16 v5] core/apply-patches: store full path of applied patches
  2016-03-19 15:03   ` Thomas Petazzoni
@ 2016-03-19 18:51     ` Yann E. MORIN
  2016-03-19 22:37       ` Yann E. MORIN
  0 siblings, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-19 18:51 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2016-03-19 16:03 +0100, Thomas Petazzoni spake thusly:
> On Fri, 11 Mar 2016 18:49:22 +0100, Yann E. MORIN wrote:
> 
> > diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
> > index 201278d..20a1552 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
> 
> This seems unrelated to the patch in question, and is not explained
> anywhere.

Ah, indeed it's not trivial. When we apply our bundled patches, only the
relative path to those patches are stored. So we make them canonical by
prepending the current path to that relative path.

Strictly speaking, I am not sure it is needed, but for consistency, all
paths are stored as full paths.

> However, I have some more global concern about this approach. I really
> dislike the idea that we rely on a helper script filling up a file,
> that we later read from a completely different place in the package
> infrastructure. It really feels like "let's dump my data there and
> fetch it from here".

I know that feeling, that makes one not comfortable and bothered.
Yet, I fail to see a problem in there; there are other cases where we
save stuff in a file, and re-use it later from another place, like the
graph-size stuff.

BTW, what did we so far save the list of patches for to begin with?

> So here are other proposals:
> 
>  * The package infra already knows which patches should be applied
>    (bundled patches, global patch dir, etc.), so it is technically able
>    to get the list of patches. Yes it's a bit annoying because the
>    logic to derive the list of patches is already inside the
>    apply-patches script. But maybe it's because too much smart stuff is
>    done in the apply-patch script without the package infrastructure
>    being aware.

Hmmm... I consider all our support/scripts/ stuff to *be* part of the
infra, like the mkusers script. apply-patches *is* part of the infra;
it's just written in a shel script rather than in make (which would
undoubtly be unreadable). Also, remember you suggested we offload all
out complex make code for handling external toolchains, into a (set of)
well scripts? It is similar to me.

>  * Alternatively, add an option to apply-patch.sh that will not apply
>    the patches, but show the list of patches that would be applied.
>    Like "apply-patch -l" for example. Then, when doing the legal-info,
>    you simply call "apply-patch -l" to retrieve the list of patches
>    that you need to copy.

What I don't like in this case, is that you decorelate applying the
patches from listing them. By storing the list of patches at the time
they are applied, we guarantee the list is consistent with the patches
that were applied. By postponing the listing later, we can no longer
offer that guarantee.

Yes, we can't guarantee that the patches we later copy are the ones that
were actually applied; true. That's why I believe that legal-info should
be part of the "standard" build.

Anyway, I won't fight that one; all that I care about is that patches
are actually saved, so I'll go for the -l option to apply-patches.

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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 09/16 v5] core/apply-patches: store full path of applied patches
  2016-03-19 18:51     ` Yann E. MORIN
@ 2016-03-19 22:37       ` Yann E. MORIN
  2016-03-20 13:47         ` Thomas Petazzoni
  0 siblings, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-19 22:37 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2016-03-19 19:51 +0100, Yann E. MORIN spake thusly:
> On 2016-03-19 16:03 +0100, Thomas Petazzoni spake thusly:
> > On Fri, 11 Mar 2016 18:49:22 +0100, Yann E. MORIN wrote:
> > 
> > > diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
> > > index 201278d..20a1552 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
[--SNIP--]
> > So here are other proposals:
> > 
> >  * The package infra already knows which patches should be applied
> >    (bundled patches, global patch dir, etc.), so it is technically able
> >    to get the list of patches. Yes it's a bit annoying because the
> >    logic to derive the list of patches is already inside the
> >    apply-patches script. But maybe it's because too much smart stuff is
> >    done in the apply-patch script without the package infrastructure
> >    being aware.

e-reading this, I don;t see a proposal in there. Did I miss something,
or did you forget to add something? ;-)

> >  * Alternatively, add an option to apply-patch.sh that will not apply
> >    the patches, but show the list of patches that would be applied.
> >    Like "apply-patch -l" for example. Then, when doing the legal-info,
> >    you simply call "apply-patch -l" to retrieve the list of patches
> >    that you need to copy.

Well, it is in fact a bit more complex than just running apply-patches
to get the list of patches.

First, some package do call apply-patches manually; there are 15 such
packages, some cal.ling apply-patches more than once (gcc, linux, linux
headers, uboot).

Second, in that case, some patches are applied conditionally. We do not
want to duplicate that logic for legal-info.

So, I stand that the best solution is my proposal, to store the list of
applied patches at the tiem they are applied, and use that later on for
the legal-info output.

If there is a better proposal, or a simple way to fix yours, I'm all
ears.

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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 02/16 v5] core/pkg-utils: add macro to hardlink-or-copy
  2016-03-19 14:23   ` Thomas Petazzoni
  2016-03-19 16:08     ` Arnout Vandecappelle
@ 2016-03-19 23:11     ` Yann E. MORIN
  2016-03-20 13:29       ` Thomas Petazzoni
  1 sibling, 1 reply; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-19 23:11 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2016-03-19 15:23 +0100, Thomas Petazzoni spake thusly:
> On Fri, 11 Mar 2016 18:49:15 +0100, Yann E. MORIN wrote:
> 
> > +################################################################################
> > +# 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*
> > +# argument 3 is the basename of the destination file (optional, defaults to
> > +#            the basename of the source file.
> > +#
> > +# examples:
> > +#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir)
> > +#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir,new-name)
> > +#
> > +# Notes:
> > +#  - this is NOT an atomic operation,
> > +#  - this is only a wrapper to a shell script, so that it can be used with
> > +#    shell-level variables, like in a for loop.
> > +################################################################################
> > +define hardlink-copy
> > +	support/scripts/hardlink-copy "$(strip $(1))" "$(strip $(2))" "$(strip $(3))"
> > +endef
> 
> While in certain cases, I agree that a shell wrapper is nice and
> useful, here I really don't see the point of a shell wrapper. You can
> simply do (untested):
> 
> define hardlink-or-copy-inner
> 	rm -f $(2)
> 	ln -f $(1) $(2) 2>/dev/null || cp -f $(1) $(2)
> endef
> 
> define hardlink-or-copy
> 	$(call hardlink-or-copy-inner,$(1),$(if $(3),$(2)/$(3),$(2)/$(basename $(1))))
> endef

Well, that does not work. That's basically what I did in the previous
iteration, but in some cases, $(3) could be a shell expansion and it is
not empty at the time the macro is called, but ends up being empty after
the shell expansion. See the discussion we had with Luca about this:
    http://lists.busybox.net/pipermail/buildroot/2016-February/152634.html

> Note that I think the name of the macro should be hardlink-or-copy and
> not hardlink-copy. The name hardlink-copy seems to indicate that you
> are copying a hardlink, which is not what is happening.
> hardlink-or-copy is IMO clearer on the fact that the macro does a hardlink
> or a copy.

OK, will change.

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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 02/16 v5] core/pkg-utils: add macro to hardlink-or-copy
  2016-03-19 16:08     ` Arnout Vandecappelle
@ 2016-03-19 23:33       ` Yann E. MORIN
  2016-03-20 13:21         ` Thomas Petazzoni
  2016-03-22 22:32         ` Arnout Vandecappelle
  0 siblings, 2 replies; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-19 23:33 UTC (permalink / raw)
  To: buildroot

Arnout, All,

On 2016-03-19 17:08 +0100, Arnout Vandecappelle spake thusly:
> On 03/19/16 15:23, Thomas Petazzoni wrote:
> >On Fri, 11 Mar 2016 18:49:15 +0100, Yann E. MORIN wrote:
> >
> >>+################################################################################
> >>+# 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*
> >>+# argument 3 is the basename of the destination file (optional, defaults to
> >>+#            the basename of the source file.
> >>+#
> >>+# examples:
> >>+#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir)
> >>+#   $(call hardlink-copy,/path/to/source/file,/path/to/destination/dir,new-name)
> >>+#
> >>+# Notes:
> >>+#  - this is NOT an atomic operation,
> >>+#  - this is only a wrapper to a shell script, so that it can be used with
> >>+#    shell-level variables, like in a for loop.
> >>+################################################################################
> >>+define hardlink-copy
> >>+	support/scripts/hardlink-copy "$(strip $(1))" "$(strip $(2))" "$(strip $(3))"
> >>+endef
> >
> >While in certain cases, I agree that a shell wrapper is nice and
> >useful, here I really don't see the point of a shell wrapper. You can
> >simply do (untested):
> >
> >define hardlink-or-copy-inner
> >	rm -f $(2)
> >	ln -f $(1) $(2) 2>/dev/null || cp -f $(1) $(2)
> >endef
> >
> >define hardlink-or-copy
> >	$(call hardlink-or-copy-inner,$(1),$(if $(3),$(2)/$(3),$(2)/$(basename $(1))))
> >endef
> 
>  I disagree. This may be slightly shorter in lines of code, but I think the
> shell script is much more readable.

And as explained in the commit log, it does not work if any of the
argument needs shell-level expansion (see my other reply to Thomas).

>  However, I do wonder if it makes sense to have a make function for it if
> there already is a shell script. We don't do that for apply-patches, so why
> would we do it for this one?

As explained in the ocmmit log:
    We still keep the make macro, as it is easier to [call], and allows
    to $(strip) its arguments in a convenient fashion.

Note: I now use "call" when the commit log used "write". What is easier
is not to write the macro (well, it is, but that was not the point of
that sentence), but writing code that calls the macro.

Also, I think it is better to have callers call a make macro, even if
backe by a shell script. It is more "in-line" with how we do other
stuff, like the github helper. Our helpers are make macros, it is more
homogeneous.

> >Note that I think the name of the macro should be hardlink-or-copy and
> >not hardlink-copy. The name hardlink-copy seems to indicate that you
> >are copying a hardlink, which is not what is happening.
> >hardlink-or-copy is IMO clearer on the fact that the macro does a hardlink
> >or a copy.
> 
>  I agree with that.

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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 11/16 v5] core/legal-info: renumber saved patches
  2016-03-19 15:05   ` Thomas Petazzoni
@ 2016-03-19 23:34     ` Yann E. MORIN
  0 siblings, 0 replies; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-19 23:34 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2016-03-19 16:05 +0100, Thomas Petazzoni spake thusly:
> On Fri, 11 Mar 2016 18:49:24 +0100, Yann E. MORIN wrote:
[--SNIP--]
> > Note: an alternative solution was suggested by Luca (and Arnout?), which
> > would be to check that no two patches have the same basename or bail-out
> > otherwise. I choose to keep the renumbering, because it follows the path
> > of least resistance (i.e. it does not break existing setups).
> 
> I second the position of Luca and Arnout. This renumbering is
> complicated and leads to ugly patch names that no longer match the
> original file name. So please simply bail-out if two patches have the
> same name. It's highly unlikely, so I don't think we should renumber
> all the patches and have different file names just to avoid this
> unlikely case.

Done.

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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 13/16 v5] core/legal-info: generate a hash of all saved files
  2016-03-19 15:21   ` Thomas Petazzoni
@ 2016-03-19 23:40     ` Yann E. MORIN
  0 siblings, 0 replies; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-19 23:40 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2016-03-19 16:21 +0100, Thomas Petazzoni spake thusly:
> On Fri, 11 Mar 2016 18:49:26 +0100, Yann E. MORIN wrote:
> > 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>
> > Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
> > Tested-by: Luca Ceresoli <luca@lucaceresoli.net>
> 
> It's adding more stuff to the main Makefile (while I'd like to remove
> stuff from that Makefile), for something that is quite specific. But
> OK, I can see the usefulness of this hash file, so:
> 
> Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> However, I'm really surprised by this LEGAL_WARNINGS file. It's exactly
> the type of hidden file (like .applied_patches) I don't like. One
> example where it fails badly is if you interrupt "legal-info" in the
> middle. Since the legal-warning and legal-warning-pkg macro only
> concatenate to this file, then at the end of the second,
> non-interrupted, make invocation, you would got some warnings from the
> first run.

Nope, because the very first thing we do for legal-info is to trash the
existing directory before starting to collect any legal-info.

However, I would contend that we should leave that file in place. It is
critical that some stuff could not be saved, so that all-caps file is a
godd indication that something is wrong.

> Why aren't we simply displaying the warnings as we go? Probably because
> they were mixed with some other messages (like the messages about
> download some stuff). But in this case, we can serialize the steps by
> having all the download done first, then all the "aggregation" done, so
> that warnings can be displayed directly without this hidden file.

Yes, we could do that, but I'd still keep the file. Delegate to the user
the responsibility to remove it.

> Of course, this is unrelated to your change, and more a comment for
> Luca.

I can have a look as a follow-up series.

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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 14/16 v5] core/legal-info: allow ignoring packages from the legal-info
  2016-03-19 15:29   ` Thomas Petazzoni
@ 2016-03-19 23:48     ` Yann E. MORIN
  0 siblings, 0 replies; 46+ messages in thread
From: Yann E. MORIN @ 2016-03-19 23:48 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2016-03-19 16:29 +0100, Thomas Petazzoni spake thusly:
> On Fri, 11 Mar 2016 18:49:27 +0100, Yann E. MORIN wrote:
> > It might be necessary to not even mention a package in the output of
> > legal-info:
> > 
> >   - virtual packages 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.
> 
> What is the difference with <pkg>_REDISTRIBUTE = NO ? I know
> REDISTRIBUTE = NO packages are mentioned in legal-info, but their
> source code is not copied to the legal-info stuff.

"FOO_LEGAL_IGNORE = YES" mnens that the package will not even be listed
in the manifest.

Sometimes, there are (proprietary) packages that are under NDA, and even
the mere hint at the use of that package is forbidden. So we need to be
able to represent that situation.

> But does it make sense to have two separate things? Why do REDISTRIBUTE
> = NO packages get mentioned in the legal-info if their source code is
> anyway not saved.

Because the license may require it? For example, the boot codes for the
RPi are BSD-licensed, so we have to provide the license file, so
rpi-firmware has to be in the manifest. But we do not have the source
for those blobs...

[--SNIP--]
Typoes fixed, thanks.

> > 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>
> > Reviewed-by: Luca Ceresoli <luca@lucaceresoli.net>
> > Tested-by: Luca Ceresoli <luca@lucaceresoli.net>
> > 
> > ---
> > Changes v1 -> v2:
> >   - introduce a new variable, instead of making _REDISTRIBUTE a
> >     tri-state  (Thomas, Peter, Luca)
> 
> Ah, we discussed using REDISTRIBUTE, I remember. But do we need a
> tri-state ? Do we really have REDISTRIBUTE = NO packages that we want
> to see mentioned in the legal-info output ?

Yes, see above.

> > +* +LIBFOO_LEGAL_IGNORE+ can be set to +YES+ or +NO+ (the default) to indicate
> 
> To me, the naming of the variable looks like inverted logic. What about:
> 
> LIBFOO_SAVE_LEGAL_INFO = YES (default) / NO
> 
> but obviously, <pkg>_SAVE_LEGAL_INFO is a bit confusing with
> <pkg>_REDISTRIBUTE.

Yes, I am not completely sold on the _LEGAL_IGNORE name, but your
proposal is not better IMHO...

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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 02/16 v5] core/pkg-utils: add macro to hardlink-or-copy
  2016-03-19 23:33       ` Yann E. MORIN
@ 2016-03-20 13:21         ` Thomas Petazzoni
  2016-03-22 22:32         ` Arnout Vandecappelle
  1 sibling, 0 replies; 46+ messages in thread
From: Thomas Petazzoni @ 2016-03-20 13:21 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 20 Mar 2016 00:33:42 +0100, Yann E. MORIN wrote:

> >  I disagree. This may be slightly shorter in lines of code, but I think the
> > shell script is much more readable.
> 
> And as explained in the commit log, it does not work if any of the
> argument needs shell-level expansion (see my other reply to Thomas).

What about the pure make based implementation?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 02/16 v5] core/pkg-utils: add macro to hardlink-or-copy
  2016-03-19 23:11     ` Yann E. MORIN
@ 2016-03-20 13:29       ` Thomas Petazzoni
  0 siblings, 0 replies; 46+ messages in thread
From: Thomas Petazzoni @ 2016-03-20 13:29 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 20 Mar 2016 00:11:43 +0100, Yann E. MORIN wrote:

> > define hardlink-or-copy
> > 	$(call hardlink-or-copy-inner,$(1),$(if $(3),$(2)/$(3),$(2)/$(basename $(1))))
> > endef
> 
> Well, that does not work. That's basically what I did in the previous
> iteration, but in some cases, $(3) could be a shell expansion and it is
> not empty at the time the macro is called, but ends up being empty after
> the shell expansion. See the discussion we had with Luca about this:
>     http://lists.busybox.net/pipermail/buildroot/2016-February/152634.html

Hum, I see. We wouldn't have this problem if we were to know, at the
infra level, the entire list of patches that get applied. But indeed,
if we need to use the file generated by apply-patches, using a pure
make function doesn't seem to work.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 09/16 v5] core/apply-patches: store full path of applied patches
  2016-03-19 22:37       ` Yann E. MORIN
@ 2016-03-20 13:47         ` Thomas Petazzoni
  2016-03-20 16:28           ` Arnout Vandecappelle
  0 siblings, 1 reply; 46+ messages in thread
From: Thomas Petazzoni @ 2016-03-20 13:47 UTC (permalink / raw)
  To: buildroot

Hello,

On Sat, 19 Mar 2016 23:37:46 +0100, Yann E. MORIN wrote:

> > >  * The package infra already knows which patches should be applied
> > >    (bundled patches, global patch dir, etc.), so it is technically able
> > >    to get the list of patches. Yes it's a bit annoying because the
> > >    logic to derive the list of patches is already inside the
> > >    apply-patches script. But maybe it's because too much smart stuff is
> > >    done in the apply-patch script without the package infrastructure
> > >    being aware.
> 
> e-reading this, I don;t see a proposal in there. Did I miss something,
> or did you forget to add something? ;-)

Well, the proposal is to make the infrastructure aware of patches that
are applied.

> > >  * Alternatively, add an option to apply-patch.sh that will not apply
> > >    the patches, but show the list of patches that would be applied.
> > >    Like "apply-patch -l" for example. Then, when doing the legal-info,
> > >    you simply call "apply-patch -l" to retrieve the list of patches
> > >    that you need to copy.
> 
> Well, it is in fact a bit more complex than just running apply-patches
> to get the list of patches.
> 
> First, some package do call apply-patches manually; there are 15 such
> packages, some cal.ling apply-patches more than once (gcc, linux, linux
> headers, uboot).
> 
> Second, in that case, some patches are applied conditionally. We do not
> want to duplicate that logic for legal-info.
> 
> So, I stand that the best solution is my proposal, to store the list of
> applied patches at the tiem they are applied, and use that later on for
> the legal-info output.

As suggested above, one way of doing this is to extend the
infrastructure so that no package does a manual APPLY_PATCHES call. I
haven't looked at the 15 different places where we call APPLY_PATCHES
manually, so I have no idea what is the complexity of handling that at
the infrastructure level, but it looks like some extension to the
infrastructure would allow to radically simplify the logic to apply
patches in U-Boot, Barebox, Linux, AT91Bootstrap, etc. Indeed, the only
reason why they have special logic is because the infrastructure
doesn't allow you passing a local directory in <pkg>_PATCH.

Then, for packages like cvs, input-tools, mii-diag, netcat-openbsd,
setserial, sysvinit and thttpd, the only reason why they call
APPLY_PATCHES is to apply Debian patches, which are already part of the
sources. So in fact, for those packages, your proposed logic would be
somewhat incorrect, as it would store patches which are already in the
source tarball.

To me, your proposal of having apply-patches.sh fill in a file goes
completely backward compared to what we've done with _EXTRA_DOWNLOADS.
We've added _EXTRA_DOWNLOADS precisely to make sure that the
infrastructure is aware of every file we download, instead of having
random packages do their own downloading.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 09/16 v5] core/apply-patches: store full path of applied patches
  2016-03-20 13:47         ` Thomas Petazzoni
@ 2016-03-20 16:28           ` Arnout Vandecappelle
  0 siblings, 0 replies; 46+ messages in thread
From: Arnout Vandecappelle @ 2016-03-20 16:28 UTC (permalink / raw)
  To: buildroot

On 03/20/16 14:47, Thomas Petazzoni wrote:
> Hello,
>
> On Sat, 19 Mar 2016 23:37:46 +0100, Yann E. MORIN wrote:
>
>>>>   * The package infra already knows which patches should be applied
>>>>     (bundled patches, global patch dir, etc.), so it is technically able
>>>>     to get the list of patches. Yes it's a bit annoying because the
>>>>     logic to derive the list of patches is already inside the
>>>>     apply-patches script. But maybe it's because too much smart stuff is
>>>>     done in the apply-patch script without the package infrastructure
>>>>     being aware.
>>
>> e-reading this, I don;t see a proposal in there. Did I miss something,
>> or did you forget to add something? ;-)
>
> Well, the proposal is to make the infrastructure aware of patches that
> are applied.

  From a philosophical standpoint, I think this is indeed the best approach. 
However, since even the simpler proposal below is already going to be very 
complicated, and since this series already consists of 16 patches, I would 
propose to leave that for later and accept Yann's approach for the time being. 
It's not so horrible to be unacceptable IMHO, it's just not great.

  Regards,
  Arnout

>
>>>>   * Alternatively, add an option to apply-patch.sh that will not apply
>>>>     the patches, but show the list of patches that would be applied.
>>>>     Like "apply-patch -l" for example. Then, when doing the legal-info,
>>>>     you simply call "apply-patch -l" to retrieve the list of patches
>>>>     that you need to copy.
>>
>> Well, it is in fact a bit more complex than just running apply-patches
>> to get the list of patches.
>>
>> First, some package do call apply-patches manually; there are 15 such
>> packages, some cal.ling apply-patches more than once (gcc, linux, linux
>> headers, uboot).
>>
>> Second, in that case, some patches are applied conditionally. We do not
>> want to duplicate that logic for legal-info.
>>
>> So, I stand that the best solution is my proposal, to store the list of
>> applied patches at the tiem they are applied, and use that later on for
>> the legal-info output.
>
> As suggested above, one way of doing this is to extend the
> infrastructure so that no package does a manual APPLY_PATCHES call. I
> haven't looked at the 15 different places where we call APPLY_PATCHES
> manually, so I have no idea what is the complexity of handling that at
> the infrastructure level, but it looks like some extension to the
> infrastructure would allow to radically simplify the logic to apply
> patches in U-Boot, Barebox, Linux, AT91Bootstrap, etc. Indeed, the only
> reason why they have special logic is because the infrastructure
> doesn't allow you passing a local directory in <pkg>_PATCH.
>
> Then, for packages like cvs, input-tools, mii-diag, netcat-openbsd,
> setserial, sysvinit and thttpd, the only reason why they call
> APPLY_PATCHES is to apply Debian patches, which are already part of the
> sources. So in fact, for those packages, your proposed logic would be
> somewhat incorrect, as it would store patches which are already in the
> source tarball.
>
> To me, your proposal of having apply-patches.sh fill in a file goes
> completely backward compared to what we've done with _EXTRA_DOWNLOADS.
> We've added _EXTRA_DOWNLOADS precisely to make sure that the
> infrastructure is aware of every file we download, instead of having
> random packages do their own downloading.
>
> Thomas
>


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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 12/16 v5] core/legal-info: also save extra downloads
  2016-03-19 15:14   ` Thomas Petazzoni
@ 2016-03-20 16:33     ` Arnout Vandecappelle
  0 siblings, 0 replies; 46+ messages in thread
From: Arnout Vandecappelle @ 2016-03-20 16:33 UTC (permalink / raw)
  To: buildroot

On 03/19/16 16:14, Thomas Petazzoni wrote:
> Dear Yann E. MORIN,
>
> On Fri, 11 Mar 2016 18:49:25 +0100, Yann E. MORIN wrote:
>> Some packages, like perl, download extra files that end up as part of
>> the source that Buildroot builds. Up until now, those files were not
>> saved in the legal-info output.
>>
>> Add those files to the legal-info output.
>>
>> The unfortunate side-effect is that we will also save the secondary
>> archive for the external blackfin toolchains; however, we already do
>> save the binary release of some external toolchains when they do not
>> provide actual source archives.
>>
>> This is inherently bad, as those are not source archives, but solving
>> this is a bigger concern, for another series...
>
> I see two possible ways around this:
>
>   - Have two separate variables, like <pkg>_EXTRA_SOURCE (to be used for
>     perl, contains real source code that needs to be saved during
>     legal-info), and <pkg>_EXTRA_DOWNLOADS (to be used for external
>     toolchains).

  I don't like this very much, because it adds infra for something that is used 
only once.


>
>   - Just like you don't copy <pkg>_SOURCE to legal-info when
>     <pkg>_ACTUAL_SOURCE is defined, don't copy <pkg>_EXTRA_DOWNLOADS
>     when <pkg>_ACTUAL_SOURCE is defined.

  This sounds like a good idea. However, I'm sure that at some point we'll run 
into a conflict (i.e. a package that has ACTUAL_SOURCE defined but also wants 
EXTRA DOWNLOADS of sources). To be solved when that happens, but we need a good 
commit message or comment that basically summarises this entire discussion so we 
know the implications when that happens.

  Regards,
  Arnout

>
> Thomas
>


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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 02/16 v5] core/pkg-utils: add macro to hardlink-or-copy
  2016-03-19 23:33       ` Yann E. MORIN
  2016-03-20 13:21         ` Thomas Petazzoni
@ 2016-03-22 22:32         ` Arnout Vandecappelle
  1 sibling, 0 replies; 46+ messages in thread
From: Arnout Vandecappelle @ 2016-03-22 22:32 UTC (permalink / raw)
  To: buildroot

On 03/20/16 00:33, Yann E. MORIN wrote:
> Arnout, All,
>
> On 2016-03-19 17:08 +0100, Arnout Vandecappelle spake thusly:
[snip]
>>   However, I do wonder if it makes sense to have a make function for it if
>> there already is a shell script. We don't do that for apply-patches, so why
>> would we do it for this one?
>
> As explained in the ocmmit log:
>      We still keep the make macro, as it is easier to [call],

  I honestly don't see why it is easier to call...

# Save the source tarball
	$$(Q)$$(HARDLINK_OR_COPY) \
		$$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL) \
		$$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))


# Save patches and generate the series file
	$$(Q)while read f; do \
		$$(HARDLINK_OR_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


 > and allows
 >      to $(strip) its arguments in a convenient fashion.

  Why is this needed?

>
> Note: I now use "call" when the commit log used "write". What is easier
> is not to write the macro (well, it is, but that was not the point of
> that sentence), but writing code that calls the macro.
>
> Also, I think it is better to have callers call a make macro, even if
> backe by a shell script. It is more "in-line" with how we do other
> stuff, like the github helper. Our helpers are make macros, it is more
> homogeneous.

  $(call ...) is ugly, there is no way you're going to convince me it's better 
than simply calling a shell script. Oh, and the github helper should go :-)

  Regards,
  Arnout


[snip]


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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 15/16 v5] core/pkg-virtual: ignore from legal-info output
  2016-03-11 17:49 ` [Buildroot] [PATCH 15/16 v5] core/pkg-virtual: ignore from legal-info output Yann E. MORIN
@ 2016-03-27 20:47   ` Arnout Vandecappelle
  0 siblings, 0 replies; 46+ messages in thread
From: Arnout Vandecappelle @ 2016-03-27 20:47 UTC (permalink / raw)
  To: buildroot

On 03/11/16 18:49, Yann E. MORIN wrote:
> Virtual packages really have nothing to save for legal-info, so we just
> ignore them, to avoid spurious "extracting" and "patching" messages
> while running 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>

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

  Regards,
  Arnout

>
> ---
> Changes v3 -> v4:
>    - rephrase the .Note  (Luca)
>    - drop now-incorrect blurb from the commit log  (Luca)
>
> Changes v1 -> v2:
>    - use the new _LEGAL_IGNORE variable  (Thomas, Luca, Peter)
> ---
>   docs/manual/adding-packages-virtual.txt | 5 +++++
>   package/pkg-virtual.mk                  | 2 ++
>   2 files changed, 7 insertions(+)
>
> diff --git a/docs/manual/adding-packages-virtual.txt b/docs/manual/adding-packages-virtual.txt
> index a5f17a2..3857927 100644
> --- a/docs/manual/adding-packages-virtual.txt
> +++ b/docs/manual/adding-packages-virtual.txt
> @@ -56,6 +56,11 @@ 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:
> +The +virtual-package+ infrastructure automatically marks virtual packages to be
> +excluded from the +legal-info+ output (by internally setting +FOO_LEGAL_IGNORE+
> +to 'YES').
> +
>   ==== 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 244c1d0..445f09a 100644
> --- a/package/pkg-virtual.mk
> +++ b/package/pkg-virtual.mk
> @@ -59,6 +59,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))
>


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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 16/16 v5] legal-info: explicitly state how patches are licensed
  2016-03-11 17:49 ` [Buildroot] [PATCH 16/16 v5] legal-info: explicitly state how patches are licensed Yann E. MORIN
@ 2016-03-27 20:49   ` Arnout Vandecappelle
  0 siblings, 0 replies; 46+ messages in thread
From: Arnout Vandecappelle @ 2016-03-27 20:49 UTC (permalink / raw)
  To: buildroot

On 03/11/16 18:49, Yann E. MORIN wrote:
> From: Luca Ceresoli <luca@lucaceresoli.net>
>
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
> ---
>   support/legal-info/README.header | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/support/legal-info/README.header b/support/legal-info/README.header
> index 418de14..feee104 100644
> --- a/support/legal-info/README.header
> +++ b/support/legal-info/README.header
> @@ -18,7 +18,7 @@ This material is composed of the following items.
>      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.
> +   applied. Patches are under the same license as the original package.

  When a package has several licenses for several of its components, the patches 
are actually under the license of the components they touch. So I would 
summarize this as:

  Patches are under the same license of the files that they modify in the 
original package.

  Regards,
  Arnout

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

^ permalink raw reply	[flat|nested] 46+ messages in thread

* [Buildroot] [PATCH 05/16 v5] core/legal-info: ensure legal-info works in off-line mode
  2016-03-19 14:47   ` Thomas Petazzoni
  2016-03-19 18:18     ` Yann E. MORIN
@ 2016-04-28 21:57     ` Yann E. MORIN
  1 sibling, 0 replies; 46+ messages in thread
From: Yann E. MORIN @ 2016-04-28 21:57 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2016-03-19 15:47 +0100, Thomas Petazzoni spake thusly:
> On Fri, 11 Mar 2016 18:49:18 +0100, 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 by adding 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 new rule depends, like the normal download mechanism, on a stamp
> > file, so that we do not emit a spurious hash-check message on successive
> > runs of 'make source'.
> > 
> > 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.
> 
> I am wondering what is the motivation for making "make legal-info"
> absolutely executable off-line, after a "make source".

One situation is a company that has a build farm wihch does not have
acces to the internet, except for one machine that is only used to run
"make source" to populate a mirror (either a "primary mirror" or an
NFS-mounted directory) of the archives, that the other machines have
acces to and can then tap into.

In this case, the machines running "make legal-info" do not have acces
to the internet, so we need to have "make source" be complete, so it has
to also download the actual sources. This is an off-line legal-info.

Regards,
Yann E. MORIN.

> I was about to
> apply this patch, so my opinion is definitely not firm on this, but
> I have some concern:
> 
>  * Now "make source" is downloading more stuff than is actually needed
>    to do the build. And potentially a *lot* more this is going to
>    download the source code for the toolchain, which you absolutely
>    don't care about. So for a CodeSoucery ARM toolchain, instead of
>    download just the 97 MB of the toolchain, you would download 97 MB +
>    278 MB of source code. You're basically quadrupling the data to be
>    downloaded for the toolchain itself.
> 
>  * Now, after a successful build, "make source" will no longer be a
>    no-op operation, because "make source" will also download the files
>    needed for legal-info. This is IMO rather unexpected.
> 
> Now whether those two concerns are really sufficient to justify a
> different implementation can be discussed. One possible solution is to
> have a "make legal-info-source", which would download whatever
> legal-info needs to be executed offline.
> 
> That being said, we're really talking about a few corner cases, because
> essentially, only the external toolchains currently make use of
> <pkg>_ACTUAL_SOURCE.
> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

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

^ permalink raw reply	[flat|nested] 46+ messages in thread

end of thread, other threads:[~2016-04-28 21:57 UTC | newest]

Thread overview: 46+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-11 17:49 [Buildroot] [PATCH 00/16 v5] legal-info improvements and completeness (branch yem/legal-3) Yann E. MORIN
2016-03-11 17:49 ` [Buildroot] [PATCH 01/16 v5] toolchain/external: add hashes for actual sources Yann E. MORIN
2016-03-19 15:31   ` Thomas Petazzoni
2016-03-11 17:49 ` [Buildroot] [PATCH 02/16 v5] core/pkg-utils: add macro to hardlink-or-copy Yann E. MORIN
2016-03-19 14:23   ` Thomas Petazzoni
2016-03-19 16:08     ` Arnout Vandecappelle
2016-03-19 23:33       ` Yann E. MORIN
2016-03-20 13:21         ` Thomas Petazzoni
2016-03-22 22:32         ` Arnout Vandecappelle
2016-03-19 23:11     ` Yann E. MORIN
2016-03-20 13:29       ` Thomas Petazzoni
2016-03-11 17:49 ` [Buildroot] [PATCH 03/16 v5] core/legal-info: use the macro to install source archives Yann E. MORIN
2016-03-11 17:49 ` [Buildroot] [PATCH 04/16 v5] core/pkg-generic: reorder variables definitions for legal-info Yann E. MORIN
2016-03-11 17:49 ` [Buildroot] [PATCH 05/16 v5] core/legal-info: ensure legal-info works in off-line mode Yann E. MORIN
2016-03-19 14:47   ` Thomas Petazzoni
2016-03-19 18:18     ` Yann E. MORIN
2016-04-28 21:57     ` Yann E. MORIN
2016-03-11 17:49 ` [Buildroot] [PATCH 06/16 v5] core/pkg-generic: add variable to store the package rawname-version Yann E. MORIN
2016-03-19 14:48   ` Thomas Petazzoni
2016-03-19 18:20     ` Yann E. MORIN
2016-03-11 17:49 ` [Buildroot] [PATCH 07/16 v5] core/legal-info: install source archives in their own sub-dir Yann E. MORIN
2016-03-19 14:51   ` Thomas Petazzoni
2016-03-11 17:49 ` [Buildroot] [PATCH 08/16 v5] core/legal-info: add package version to license directory Yann E. MORIN
2016-03-11 17:49 ` [Buildroot] [PATCH 09/16 v5] core/apply-patches: store full path of applied patches Yann E. MORIN
2016-03-19 15:03   ` Thomas Petazzoni
2016-03-19 18:51     ` Yann E. MORIN
2016-03-19 22:37       ` Yann E. MORIN
2016-03-20 13:47         ` Thomas Petazzoni
2016-03-20 16:28           ` Arnout Vandecappelle
2016-03-11 17:49 ` [Buildroot] [PATCH 10/16 v5] core/legal-info: also save patches Yann E. MORIN
2016-03-11 17:49 ` [Buildroot] [PATCH 11/16 v5] core/legal-info: renumber saved patches Yann E. MORIN
2016-03-19 15:05   ` Thomas Petazzoni
2016-03-19 23:34     ` Yann E. MORIN
2016-03-11 17:49 ` [Buildroot] [PATCH 12/16 v5] core/legal-info: also save extra downloads Yann E. MORIN
2016-03-19 15:14   ` Thomas Petazzoni
2016-03-20 16:33     ` Arnout Vandecappelle
2016-03-11 17:49 ` [Buildroot] [PATCH 13/16 v5] core/legal-info: generate a hash of all saved files Yann E. MORIN
2016-03-19 15:21   ` Thomas Petazzoni
2016-03-19 23:40     ` Yann E. MORIN
2016-03-11 17:49 ` [Buildroot] [PATCH 14/16 v5] core/legal-info: allow ignoring packages from the legal-info Yann E. MORIN
2016-03-19 15:29   ` Thomas Petazzoni
2016-03-19 23:48     ` Yann E. MORIN
2016-03-11 17:49 ` [Buildroot] [PATCH 15/16 v5] core/pkg-virtual: ignore from legal-info output Yann E. MORIN
2016-03-27 20:47   ` Arnout Vandecappelle
2016-03-11 17:49 ` [Buildroot] [PATCH 16/16 v5] legal-info: explicitly state how patches are licensed Yann E. MORIN
2016-03-27 20:49   ` Arnout Vandecappelle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).