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

This helper 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, so if a third argument is specified, it is treated as the basename of the destination file.

Add a make variable to easily call that script.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

---
Changes v5 -> v6:
- rename script (Thomas, Arnout)
- drop the macro (Thomas, Arnout)
- drop Luca's reviewed-by and tested-by tags, because the macro disapeared
- fix redundancy in comments in the script

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             |  4 ++++
 support/scripts/hardlink-or-copy | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)
 create mode 100755 support/scripts/hardlink-or-copy

package/pkg-utils.mk +0 -0

index f88313a..2692576 100644

package/pkg-utils.mk +1 -0

+
 #
 # legal-info helper functions
 #

support/scripts/hardlink-or-copy new file +9 -0

new file mode 100755
index 0000000..b581c51
@@ -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 source file as basename
+# for the destination file.

Quoted 11 lines

This helper 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, so if a third argument is specified, it is treated as the basename of the destination file.

Add a make variable to easily call that script.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

I agree that it's a good idea to move this to a script. I have a few coding
style issues with it, but it's in things for we don't have a set style anyway so my word is not law :-). There is only one thing really wrong...

Quoted 39 lines

---
Changes v5 -> v6:
- rename script (Thomas, Arnout)
- drop the macro (Thomas, Arnout)
- drop Luca's reviewed-by and tested-by tags, because the macro disapeared
- fix redundancy in comments in the script

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 | 4 ++++
support/scripts/hardlink-or-copy | 34 ++++++++++++++++++++++++++++++++++

2 files changed, 38 insertions(+)

create mode 100755 support/scripts/hardlink-or-copy

package/pkg-utils.mk +0 -0

index f88313a..2692576 100644
@@ -113,6 +113,10 @@ $$(error Package error: use $(2) instead of $(1). Please fix your .mk file)

endif
endef

package/pkg-utils.mk $$(error Package error: use $(2) instead of $(1). Please fix your .mk file) · line 118

Quoted 3 lines

package/pkg-utils.mk +3 -0

+# hardlink-or-copy -- helper script to first try to hardlink,
+# and fallback to copy.
+HARDLINK_OR_COPY := support/scripts/hardlink-or-copy

= instead of :=

However (and here starts the difference of opinion in coding style): why do
you need to define a variable for this? In general, I'm not a big fan of variables that just contain a constant string. For numbers, it makes sense, because a number is meaningless by itself. But a string is already something that is searchable and identifiable. And in this specific case, the variable name is even the same as the value... So what's the point? OK, you save 13 characters in every place where you use it (that's in only 2 (TWO) places, by the way).

We do have a few existing cases of such variables, but IMHO they are
equally useless. And mostly introduced by you, BTW :-)

Quoted 22 lines

#

# legal-info helper functions
#

support/scripts/hardlink-or-copy new file +13 -0

new file mode 100755
index 0000000..b581c51
@@ -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 source file as basename
+# for the destination file.
+#
+# In either case, remove the destination prior to doing the
+# hardlink-or-copy.
+#

support/scripts/hardlink-or-copy line 19

Quoted 6 lines

support/scripts/hardlink-or-copy new file +6 -0

+# Note that this is NOT an atomic operation.
+
+set -e
+
+main() {
+    local src_f="${1}"

Personally I prefer my variable names to be more explicit: src_file

support/scripts/hardlink-or-copy line 26

Quoted 7 lines

support/scripts/hardlink-or-copy new file +7 -0

+    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}" )"

I thought you were more into ${src_f##*/}?

support/scripts/hardlink-or-copy line 31

Quoted 5 lines

support/scripts/hardlink-or-copy new file +5 -0

+    fi
+
+    mkdir -p "${dst_d}"
+    rm -f "${dst_f}"
+    ln -f "${src_f}" "${dst_f}" 2>/dev/null || cp -f "${src_f}" "${dst_f}"

Since there's an rm just above, is there any point in adding the -f still
here? Yes, for race conditions, but then the rm wasn't much use to begin with.

support/scripts/hardlink-or-copy line 34

Quoted 3 lines

support/scripts/hardlink-or-copy new file +3 -0

+}
+
+main "${@}"

Anothing coding style disagreement. What's the point of putting your
complete shell script in a function? I don't really have arguments against it, but do you have reasons why you think it's better like that?

Note that currently we have both styles of shell scripts, though the ones
with a main function are a minority.

And none of the python scripts have a
main.

Regards,
Arnout

Signature · 6 lines

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

Arnout, All,

On 2016-05-04 00:53 +0200, Arnout Vandecappelle spake thusly:

Quoted 1 line · Yann E. MORIN

Quoted 16 lines

package/pkg-utils.mk $$(error Package error: use $(2) instead of $(1). Please fix your .mk file) · line 118

Quoted 3 lines

package/pkg-utils.mk +3 -0

+# hardlink-or-copy -- helper script to first try to hardlink,
+# and fallback to copy.
+HARDLINK_OR_COPY := support/scripts/hardlink-or-copy

= instead of :=

However (and here starts the difference of opinion in coding style): why do
you need to define a variable for this? In general, I'm not a big fan of variables that just contain a constant string. For numbers, it makes sense, because a number is meaningless by itself. But a string is already something that is searchable and identifiable. And in this specific case, the variable name is even the same as the value... So what's the point? OK, you save 13 characters in every place where you use it (that's in only 2 (TWO) places, by the way).

Quoted 2 lines

We do have a few existing cases of such variables, but IMHO they are
equally useless. And mostly introduced by you, BTW :-)

Well, when the thing s used in multiple places, I find it "better" to use a variable so as to be able to change all locations at once with a single edit (even if we don't plan on renaming or moving it).

Quoted 6 lines

support/scripts/hardlink-or-copy line 31

Quoted 2 lines

support/scripts/hardlink-or-copy new file +2 -0

+    rm -f "${dst_f}"
+    ln -f "${src_f}" "${dst_f}" 2>/dev/null || cp -f "${src_f}" "${dst_f}"

Since there's an rm just above, is there any point in adding the -f still
here? Yes, for race conditions, but then the rm wasn't much use to begin with.

That's what was diuscussed in the previous reviews. And didn't you explcitly mentioned we should use an explicit rm -f :

    http://lists.busybox.net/pipermail/buildroot/2016-February/151140.html

;-)

Quoted 7 lines

support/scripts/hardlink-or-copy line 34

Quoted 3 lines

support/scripts/hardlink-or-copy new file +3 -0

+}
+
+main "${@}"

Anothing coding style disagreement. What's the point of putting your
complete shell script in a function? I don't really have arguments against it, but do you have reasons why you think it's better like that?

I am not sure what advantages it has, technically-wise (i.e. I am not sure it allows things that would not be possible without main).

I've been writing (bash) shell scripts for ages now, and I find it very cnvenient to use a main():

- do not use global variables (even though variables are inherited by sub-functions): declare all variables used in the function and only use that, with explciitly docuemented exceptions;

- it helps organise the code: it is cleaner in my head;

- I'm not the only one: https://google.github.io/styleguide/shell.xml

Quoted 3 lines

Note that currently we have both styles of shell scripts, though the ones
with a main function are a minority.

... and probably mines. ;-)

Regards,
Yann E. MORIN.

Quoted 15 lines

And none of the python scripts have a
main.

Regards,
Arnout

--

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

Quoted 4 lines

Arnout, All,

On 2016-05-04 00:53 +0200, Arnout Vandecappelle spake thusly:

Quoted 1 line · Yann E. MORIN

[snip]

Quoted 19 lines

Quoted 18 lines

package/pkg-utils.mk $$(error Package error: use $(2) instead of $(1). Please fix your .mk file) · line 118

Quoted 3 lines

package/pkg-utils.mk +3 -0

+# hardlink-or-copy -- helper script to first try to hardlink,
+# and fallback to copy.
+HARDLINK_OR_COPY := support/scripts/hardlink-or-copy

= instead of :=

However (and here starts the difference of opinion in coding style): why do you need to define a variable for this? In general, I'm not a big fan of variables that just contain a constant string. For numbers, it makes sense, because a number is meaningless by itself. But a string is already something that is searchable and identifiable. And in this specific case, the variable name is even the same as the value... So what's the point? OK, you save 13 characters in every place where you use it (that's in only 2 (TWO) places, by the way).

We do have a few existing cases of such variables, but IMHO they are equally useless. And mostly introduced by you, BTW :-)

Quoted 3 lines

Well, when the thing s used in multiple places, I find it "better" to use a variable so as to be able to change all locations at once with a single edit (even if we don't plan on renaming or moving it).

I was going to build a big argument about why I disagree, but I see in v7
you already dropped the variable so let's leave it at that :-)

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

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

---
Changes v5 -> v6:
- directly call the helper script (Thomas, Arnout)
- drop Luca's and Arnout's tags, as the code did change a bit

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

Changes v2 -> v3:
- comment the @ -> $(Q) change (Arnout)

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

package/pkg-generic.mk +0 -0

index 3904c09..5c2e46d 100644

package/pkg-generic.mk +1 -0

+				  $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))
 endif # redistribute
 
 endif # other packages

Quoted 19 lines

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

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

---
Changes v5 -> v6:
- directly call the helper script (Thomas, Arnout)
- drop Luca's and Arnout's tags, as the code did change a bit

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

Changes v2 -> v3:
- comment the @ -> $(Q) change (Arnout)
---
package/pkg-generic.mk | 6 +++---

1 file changed, 3 insertions(+), 3 deletions(-)

package/pkg-generic.mk ifeq ($$($(2)_REDISTRIBUTE),YES) · line 807

Quoted 13 lines

package/pkg-generic.mk +0 -0

index 3904c09..5c2e46d 100644
@@ -803,9 +803,9 @@ ifeq ($$($(2)_REDISTRIBUTE),YES)
 ifneq ($$($(2)_ACTUAL_SOURCE_TARBALL),$$($(2)_SOURCE))
 	$$(call DOWNLOAD,$$($(2)_ACTUAL_SOURCE_SITE)/$$($(2)_ACTUAL_SOURCE_TARBALL))

endif

package/pkg-generic.mk +2 -3

-# 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)$$(HARDLINK_OR_COPY) $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\

Did you test this? I don't think it can work with that comma...

Ah, but it gets fixed in patch 6 :-)

Regards,
Arnout

Quoted 5 lines

package/pkg-generic.mk +1 -0

+				  $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))

endif # redistribute

endif # other packages

Signature · 6 lines

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

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" <[email protected]>

Cc Luca Ceresoli <[email protected]>

Cc Thomas Petazzoni <[email protected]>

Cc Peter Korsgaard <[email protected]>

Tested-by Luca Ceresoli <[email protected]>

---
Notes: here is a case where one would need to be able to do an off-line legal-info:
- a build farm (e.g. Jenkins slaves) without access to the internet;
- a single machine (not part of the farm) has access to the internet;
- that machine runs "make source" to populate a mirror (a "primary mirror" or an NFS-mounted directory or anything else) that is accessible to the build farm;
- machines in the build farm need the actual sources to run legal-info, doing so off-line.

So, "make source" has to be complete, i.e. it must also download the acutal source archives.

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

package/pkg-generic.mk +7 -0

index cbdf96e..2c405e1 100644
@@ -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

package/pkg-generic.mk +8 -0

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

package/pkg-generic.mk +3 -3

 	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)$$(HARDLINK_OR_COPY) $$(DL_DIR)/$$($(2)_ACTUAL_SOURCE_TARBALL),\
 				  $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))

@@ -896,6 +914,7 @@ endif
 	$(1)-install-staging \
 	$(1)-install-target \
 	$(1)-legal-info \
+	$(1)-legal-source \
 	$(1)-patch \
 	$(1)-rebuild \
 	$(1)-reconfigure \

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" <[email protected]>

Cc Luca Ceresoli <[email protected]>

Acked-by Luca Ceresoli <[email protected]>

Tested-by Luca Ceresoli <[email protected]>

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

Reviewed-by Thomas Petazzoni <[email protected]>

---
Chages v5 -> v6:
- variable with raw name was renamed (Thomas)

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

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

package/pkg-generic.mk +2 -0

index 7aca958..a362b73 100644
@@ -496,6 +496,8 @@ endif
 
 $(2)_REDISTRIBUTE		?= YES
 
+$(2)_REDIST_SOURCES_DIR = $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))/$$($(2)_RAW_BASE_NAME)
+
 # 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

package/pkg-generic.mk +1 -0

+				  $$($(2)_REDIST_SOURCES_DIR)
 endif # redistribute
 
 endif # other packages

Quoted 43 lines

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

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

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

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Luca Ceresoli <[email protected]>

Acked-by Luca Ceresoli <[email protected]>

Tested-by Luca Ceresoli <[email protected]>

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

Reviewed-by Thomas Petazzoni <[email protected]>

---
Chages v5 -> v6:
- variable with raw name was renamed (Thomas)

Changes v1 -> v2:
- perl no longer has a post-legal-info hook (Thomas, Luca)
---
package/pkg-generic.mk | 6 ++++--

1 file changed, 4 insertions(+), 2 deletions(-)

package/pkg-generic.mk +0 -0

index 7aca958..a362b73 100644
@@ -496,6 +496,8 @@ endif
 

$(2)_REDISTRIBUTE ?= YES

+$(2)_REDIST_SOURCES_DIR = $$(REDIST_SOURCES_DIR_$$(call UPPERCASE,$(4)))/$$($(2)_RAW_BASE_NAME) +
# 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

package/pkg-generic.mk +0 -0

@@ -823,8 +825,8 @@ else
 

package/pkg-generic.mk else · line 828

Quoted 5 lines

package/pkg-generic.mk +1 -2

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

Did I really give my Rev-by without noticing that comma disappearing? :-)

Regards,
Arnout

Quoted 5 lines

package/pkg-generic.mk +1 -0

+				  $$($(2)_REDIST_SOURCES_DIR)

endif # redistribute

endif # other packages

Signature · 6 lines

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

From: Luca Ceresoli <[email protected]>

Signed-off-by Luca Ceresoli <[email protected]>

Acked-by Thomas Petazzoni <[email protected]>

[yann.morin.1998 at free.fr: slightyl tweak after Arnout's review]

Signed-off-by Yann E. MORIN <[email protected]>

Cc Arnout Vandecappelle <[email protected]>

---
Changes v5 -> v6:
- slightly tweak the phrasing (Arnout)

---
 support/legal-info/README.header | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

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

index 418de14..1f3524f 100644
@@ -18,7 +18,8 @@ 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 files that they modify
+   in 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.

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.

TL;DR:

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

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

- patches 1-4 make sure legal-info will work in off-line mode;

- patches 5-7 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 8-11 actually save the patches and extra downloads in the

    legal-info output;

- patch 12 adds a list of hashes for all files in the legal-info

    output;

- patch 15 (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 second solution. The immediate drawback is that in rare circumstances, existing setups (e.g. with patches in a global patch directory) which has duplicatre basenames will cease to work at the time patches are applied.

that most patches will have two indexes, while the immediate advantage is that it accepts input patches with the same basename.

The alternative first solution, although tested and implemented (and previously submitted) has the drawback that most patches will have two indexes, whithe the immediate advantage that it accepts input patches with the same basename (and thus would not break existing setups).

After the previous reviews, it was deemed preferrable to have less ugly patch filenames and break the build, rather than have uglier patch filenames and not break the build, given how unlikely the situation with duplicate basenames is.

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.

Changlog of the series
----------------------

Changes v5 -> v6:
- drop the hardlink-or-copy macro, only keep the script (Thomas, Arnout)
- don;t renumber patches; instead, detect duplicate files basenames and abort (Thomas, Arnout, Luca)
- rename the raw basename variable (Thomas)
- tweak the phrasing about patches licenses (Arnout)
- completely ignoring packages from legal-info was deemd too controversial; drop it (at least for now! ;-) )
- fix misc comments and typoes

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

purge-locales: Handle empty locale directories better (2016-04-28 23:48:09 +0200)

are available in the git repository at:

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

for you to fetch changes up to d7b10938001e539340479d97a2b64f4ab49d3726:

legal-info: explicitly state how patches are licensed (2016-04-29 00:07:26 +0200)

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

Yann E. MORIN (12):
support/scripts: add helper to hardlink-or-copy core/legal-info: use the helper 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 support/apply-patches: bail-out on duplicate patch basenames core/legal-info: also save extra downloads core/legal-info: generate a hash of all saved files

 Makefile                         |  8 ++++-
 package/pkg-generic.mk           | 73 +++++++++++++++++++++++++++-------------
 package/pkg-utils.mk             |  4 +++
 support/legal-info/README.header | 10 +++---
 support/scripts/apply-patches.sh | 17 ++++++++--
 support/scripts/hardlink-or-copy | 34 +++++++++++++++++++
 6 files changed, 115 insertions(+), 31 deletions(-)
 create mode 100755 support/scripts/hardlink-or-copy

Quoted 26 lines

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.

TL;DR:

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

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

- patches 1-4 make sure legal-info will work in off-line mode;

- patches 5-7 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 8-11 actually save the patches and extra downloads in the legal-info output;

Quoted 4 lines

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

- patch 15 (from Luca) explicits patches licensing.

That's patch 13, actually :-)

For 2016.05, we certainly still want patch 13, and probably also patches 1,
2, 5, 6, 7, 8, 9, 11 (because they fix a real issue, that legal-info is incomplete). I do have some comments on these but it's nitpicking mostly.

The other patches are more controversial.

If you would repost, do you think you could take that order?