pkg-download: check hashes before the download (branch yem/download-hash)

Hello All!

This series introduces a way to check hashes prior to doing a download.

This is required for when upstream silently update their release tarballs without renaming them, and the user is left with a stray locally cached tarball that no longer match the hashes with have for that package.

In so doing, this series:
- moves the check for a cached file into the wrapper;
- moves the post-download check for hashes into the wrapper;
- adds a pre-download check for hashes in the wrapper.

Doing the pre-download checks in the Makefile, like the post-download checks were done, made the Makefile a bit harder to read. On the other hand, we have a download wrapper shell script, so it is easier to do trickey stuff in there (shell syntax) than in the Makefile (make syntax can become unreadable pretty fast).

This has a side effect of cleaning up the pkg-download.mk Makefile, too, but that was not the goal.

Regards,
Yann E. MORIN.

The following changes since commit 1af2db0f77cc98d57012f238afbceb203b9b739b:

qt5base: add error handling to for loop (2014-12-01 23:33:14 +0100)

are available in the git repository at:

git://git.busybox.net/~ymorin/git/buildroot yem/download-hash

for you to fetch changes up to e1cc728a60b6a250ea395ee429dba7f470b1f8a8:

pkg-download: check hasahes for locally cached files (2014-12-02 00:11:05 +0100)

----------------------------------------------------------------
Yann E. MORIN (4):
pkg-download: check for already downloaded file in the download wrapper
pkg-download: fix arguments to hash checking script
pkg-download: verify the hashes from the download wrapper
pkg-download: check hasahes for locally cached files

 package/pkg-download.mk     | 36 +++++++++++-------------------------
 support/download/check-hash | 18 ++++++++++--------
 support/download/wrapper    | 22 +++++++++++++++++++++-
 3 files changed, 42 insertions(+), 34 deletions(-)

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

Instead of repeating the same test again and again in all our download rules, just delegate the check for an already downloaded file to the download wrapper.

This clears up the path for doing the hash checks on a cached file before the download.

---
 package/pkg-download.mk  | 8 --------
 support/download/wrapper | 5 +++++
 2 files changed, 5 insertions(+), 8 deletions(-)

index f3409bd..f6ceadb 100644
@@ -94,7 +94,6 @@ endef
 # Messages for the type of clone used are provided to ease debugging in case of
 # problems
 define DOWNLOAD_GIT
-	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	$(EXTRA_ENV) support/download/wrapper git \
 		$(DL_DIR)/$($(PKG)_SOURCE) \
 		$($(PKG)_SITE) \

@@ -114,7 +113,6 @@ endef
 
 
 define DOWNLOAD_BZR
-	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	$(EXTRA_ENV) support/download/wrapper bzr \
 		$(DL_DIR)/$($(PKG)_SOURCE) \
 		$($(PKG)_SITE) \

@@ -131,7 +129,6 @@ define SHOW_EXTERNAL_DEPS_BZR
 endef
 
 define DOWNLOAD_CVS
-	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	$(EXTRA_ENV) support/download/wrapper cvs \
 		$(DL_DIR)/$($(PKG)_SOURCE) \
 		$(call stripurischeme,$(call qstrip,$($(PKG)_SITE))) \

@@ -150,7 +147,6 @@ define SHOW_EXTERNAL_DEPS_CVS
 endef
 
 define DOWNLOAD_SVN
-	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	$(EXTRA_ENV) support/download/wrapper svn \
 		$(DL_DIR)/$($(PKG)_SOURCE) \
 		$($(PKG)_SITE) \

@@ -170,7 +166,6 @@ endef
 # Note that filepath is relative to the user's home directory, so you may want
 # to prepend the path with a slash: scp://[user@]host:/absolutepath
 define DOWNLOAD_SCP
-	test -e $(DL_DIR)/$(2) || \
 	$(EXTRA_ENV) support/download/wrapper scp \
 		$(DL_DIR)/$(2) \
 		'$(call stripurischeme,$(call qstrip,$(1)))' && \

@@ -187,7 +182,6 @@ endef
 
 
 define DOWNLOAD_HG
-	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	$(EXTRA_ENV) support/download/wrapper hg \
 		$(DL_DIR)/$($(PKG)_SOURCE) \
 		$($(PKG)_SITE) \

@@ -207,7 +201,6 @@ endef
 
 
 define DOWNLOAD_WGET
-	test -e $(DL_DIR)/$(2) || \
 	$(EXTRA_ENV) support/download/wrapper wget \
 		$(DL_DIR)/$(2) \
 		'$(call qstrip,$(1))' && \

@@ -223,7 +216,6 @@ define SHOW_EXTERNAL_DEPS_WGET
 endef
 
 define DOWNLOAD_LOCALFILES
-	test -e $(DL_DIR)/$(2) || \
 	$(EXTRA_ENV) support/download/wrapper cp \
 		$(DL_DIR)/$(2) \
 		$(call stripurischeme,$(call qstrip,$(1))) && \

index 320a37e..e3ab3a1 100755
@@ -28,6 +28,11 @@ helper="${1}"
 output="${2}"
 shift 2
 
+# If the output file already exists, do not download it again
+if [ -e "${output}" ]; then
+    exit 0
+fi
+
 # tmpd is a temporary directory in which helpers may store intermediate
 # by-products of the download.
 # tmpf is the file in which the helpers should put the downloaded content.

1.9.1

Dear Yann E. MORIN,

It has the consequence of doing another fork+exec of a shell script for all files, even if they have already been downloaded. I don't know if this is really measurable compared to the overall build time, though, except maybe on some cases like "make source". So maybe this is OK.

Best regards,

Thomas

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

Thomas, All,

On 2014-12-02 09:26 +0100, Thomas Petazzoni spake thusly:

Well, if I remember correctly, make is smart enough to detect simple commands and not spawn a shell for them, and just does the fork-exec itself rather than calling system().

So, when counting the fork+exec you'd get:

- simple command (program arg arg arg): make does a fork()+exec() to the script, the kernel sees the she-bang and the exec is a call to the shell to interpret the script

- command lists (command1 || command2), make does a system(), which in turn does a fork()+exec() to the shell to interpret the command list.

So we're breaking even in that case. We even gain a call to system(), so we should be a little tiny bit faster! ;-p

And anyway, a fork+exec is not that costly nowaday. Probably, on a cache-cold system, the IOs will largely over-shadow the fork+exec pair I am afraid... ;-)

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

The argument are correctly used, but incorrectly documented. Inverse the comments to match the actual usage.

---
 support/download/check-hash | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 067e7a2..13e361a 100755
@@ -3,8 +3,8 @@ set -e
 
 # Helper to check a file matches its known hash
 # Call it with:
-#   $1: the full path to the file to check
-#   $2: the path of the file containing all the the expected hashes
+#   $1: the path of the file containing all the the expected hashes
+#   $2: the full path to the file to check
 
 h_file="${1}"
 file="${2}"

1.9.1

Instead of repeating the check in our download rules, delegate the check of the hashes to the download wrapper.

This needs three different changes:

- add a new argument to the download wrapper, that is the full path to the hash file; if the hash file does not exist, that does not change the current behaviour, as the existence of the hash file is done by the check-hash script;

- add a third argument to the check-hash script, to be the basename of the file to check; this is required because we no longer check the final file with the finale filename, but anintermediate file with a temporary filename;

- do the actual cal to the check-hash script form within the download wrapper.

This further paves the way to doing pre-download checks of the hashes for the locally cached files.

---
 package/pkg-download.mk     | 28 +++++++++++-----------------
 support/download/check-hash | 14 ++++++++------
 support/download/wrapper    | 11 ++++++++++-
 3 files changed, 29 insertions(+), 24 deletions(-)

index f6ceadb..c10430f 100644
@@ -58,17 +58,6 @@ domainseparator = $(if $(1),$(1),/)
 # github(user,package,version): returns site of GitHub repository
 github = https://github.com/$(1)/$(2)/archive/$(3)
 
-# Helper for checking a tarball's checksum
-# If the hash does not match, remove the incorrect file
-# $(1): the path to the file with the hashes
-# $(2): the full path to the file to check
-define VERIFY_HASH
-	if ! support/download/check-hash $(1) $(2) $(if $(QUIET),>/dev/null); then \
-		rm -f $(2); \
-		exit 1; \
-	fi
-endef
-
 ################################################################################
 # The DOWNLOAD_* helpers are in charge of getting a working copy
 # of the source repository for their corresponding SCM,

@@ -96,6 +85,7 @@ endef
 define DOWNLOAD_GIT
 	$(EXTRA_ENV) support/download/wrapper git \
 		$(DL_DIR)/$($(PKG)_SOURCE) \
+		$(PKGDIR)/$($(PKG)_NAME).hash \
 		$($(PKG)_SITE) \
 		$($(PKG)_DL_VERSION) \
 		$($(PKG)_BASE_NAME)

@@ -115,6 +105,7 @@ endef
 define DOWNLOAD_BZR
 	$(EXTRA_ENV) support/download/wrapper bzr \
 		$(DL_DIR)/$($(PKG)_SOURCE) \
+		$(PKGDIR)/$($(PKG)_NAME).hash \
 		$($(PKG)_SITE) \
 		$($(PKG)_DL_VERSION) \
 		$($(PKG)_BASE_NAME)

@@ -131,6 +122,7 @@ endef
 define DOWNLOAD_CVS
 	$(EXTRA_ENV) support/download/wrapper cvs \
 		$(DL_DIR)/$($(PKG)_SOURCE) \
+		$(PKGDIR)/$($(PKG)_NAME).hash \
 		$(call stripurischeme,$(call qstrip,$($(PKG)_SITE))) \
 		$($(PKG)_DL_VERSION) \
 		$($(PKG)_RAWNAME) \

@@ -149,6 +141,7 @@ endef
 define DOWNLOAD_SVN
 	$(EXTRA_ENV) support/download/wrapper svn \
 		$(DL_DIR)/$($(PKG)_SOURCE) \
+		$(PKGDIR)/$($(PKG)_NAME).hash \
 		$($(PKG)_SITE) \
 		$($(PKG)_DL_VERSION) \
 		$($(PKG)_BASE_NAME)

@@ -168,8 +161,8 @@ endef
 define DOWNLOAD_SCP
 	$(EXTRA_ENV) support/download/wrapper scp \
 		$(DL_DIR)/$(2) \
-		'$(call stripurischeme,$(call qstrip,$(1)))' && \
-	$(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
+		$(PKGDIR)/$($(PKG)_NAME).hash \
+		'$(call stripurischeme,$(call qstrip,$(1)))'
 endef
 
 define SOURCE_CHECK_SCP

@@ -184,6 +177,7 @@ endef
 define DOWNLOAD_HG
 	$(EXTRA_ENV) support/download/wrapper hg \
 		$(DL_DIR)/$($(PKG)_SOURCE) \
+		$(PKGDIR)/$($(PKG)_NAME).hash \
 		$($(PKG)_SITE) \
 		$($(PKG)_DL_VERSION) \
 		$($(PKG)_BASE_NAME)

@@ -203,8 +197,8 @@ endef
 define DOWNLOAD_WGET
 	$(EXTRA_ENV) support/download/wrapper wget \
 		$(DL_DIR)/$(2) \
-		'$(call qstrip,$(1))' && \
-	$(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
+		$(PKGDIR)/$($(PKG)_NAME).hash \
+		'$(call qstrip,$(1))'
 endef
 
 define SOURCE_CHECK_WGET

@@ -218,8 +212,8 @@ endef
 define DOWNLOAD_LOCALFILES
 	$(EXTRA_ENV) support/download/wrapper cp \
 		$(DL_DIR)/$(2) \
-		$(call stripurischeme,$(call qstrip,$(1))) && \
-	$(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
+		$(PKGDIR)/$($(PKG)_NAME).hash \
+		$(call stripurischeme,$(call qstrip,$(1)))
 endef
 
 define SOURCE_CHECK_LOCALFILES

index 13e361a..b41a87e 100755
@@ -5,9 +5,11 @@ set -e
 # Call it with:
 #   $1: the path of the file containing all the the expected hashes
 #   $2: the full path to the file to check
+#   $3: the basename of the file to check
 
 h_file="${1}"
 file="${2}"
+base="${3}"
 
 # Does the hash-file exist?
 if [ ! -f "${h_file}" ]; then

@@ -30,7 +32,7 @@ check_one_hash() {
         sha224|sha256|sha384|sha512)    ;;
         *) # Unknown hash, exit with error
             printf "ERROR: unknown hash '%s' for '%s'\n"  \
-                   "${_h}" "${_file##*/}" >&2
+                   "${_h}" "${base}" >&2
             exit 1
             ;;
     esac

@@ -38,11 +40,11 @@ check_one_hash() {
     # Do the hashes match?
     _hash=$( ${_h}sum "${_file}" |cut -d ' ' -f 1 )
     if [ "${_hash}" = "${_known}" ]; then
-        printf "%s: OK (%s: %s)\n" "${_file##*/}" "${_h}" "${_hash}"
+        printf "%s: OK (%s: %s)\n" "${base}" "${_h}" "${_hash}"
         return 0
     fi
 
-    printf "ERROR: %s has wrong %s hash:\n" "${_file##*/}" "${_h}" >&2
+    printf "ERROR: %s has wrong %s hash:\n" "${base}" "${_h}" >&2
     printf "ERROR: expected: %s\n" "${_known}" >&2
     printf "ERROR: got     : %s\n" "${_hash}" >&2
     printf "ERROR: Incomplete download, or man-in-the-middle (MITM) attack\n" >&2

@@ -59,7 +61,7 @@ while read t h f; do
             continue
             ;;
         *)
-            if [ "${f}" = "${file##*/}" ]; then
+            if [ "${f}" = "${base}" ]; then
                 check_one_hash "${t}" "${h}" "${file}"
                 : $((nb_checks++))
             fi

@@ -69,9 +71,9 @@ done <"${h_file}"
 
 if [ ${nb_checks} -eq 0 ]; then
     if [ -n "${BR2_ENFORCE_CHECK_HASH}" ]; then
-        printf "ERROR: No hash found for %s\n" "${file}" >&2
+        printf "ERROR: No hash found for %s\n" "${base}" >&2
         exit 1
     else
-        printf "WARNING: No hash found for %s\n" "${file}" >&2
+        printf "WARNING: No hash found for %s\n" "${base}" >&2
     fi
 fi

index e3ab3a1..2f0f016 100755
@@ -8,6 +8,7 @@
 # Call it with:
 #   $1: name of the helper (eg. cvs, git, cp...)
 #   $2: full path to the file in which to save the download
+#   $3: full path to the hash file
 #   $*: additional arguments to the helper in $1
 # Environment:
 #   BUILD_DIR: the path to Buildroot's build dir

@@ -26,7 +27,8 @@ set -e
 
 helper="${1}"
 output="${2}"
-shift 2
+hfile="${3}"
+shift 3
 
 # If the output file already exists, do not download it again
 if [ -e "${output}" ]; then

@@ -60,6 +62,13 @@ fi
 # cd back to free the temp-dir, so we can remove it later
 cd "${OLDPWD}"
 
+# Check if the downloaded file is sane, and matches the stored hashes
+# for that file
+if ! support/download/check-hash "${hfile}" "${tmpf}" "${output##*/}"; then
+    rm -rf "${tmpd}"
+    exit 1
+fi
+
 # tmp_output is in the same directory as the final output, so we can
 # later move it atomically.
 tmp_output="$( mktemp "${output}.XXXXXX" )"

1.9.1

Dear Yann E. MORIN,

It is really time to use getopt in the download wrapper scripts, as we have discussed with Fabio recently regarding the fully silent build thing.

finale -> final

anintermediate -> an intermediate

cal?

form -> from.

Thomas

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

Thomas, All,

On 2014-12-02 09:29 +0100, Thomas Petazzoni spake thusly:

Yeah, I was afraid you'd say that. :-)

But I'm mostly interested on feedback about the change itself, to check hashes on locally cached files.

If that is OK, I'll rework the series.

Yup x2.

call

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

In some cases, upstream just update their releases in-place, without renaming them. When that package is updated in Buildroot, a new hash to match the new upstream release is included in the corresponding .hash file.

As a consequence, users who previously downloaded that package's tarball with an older version of Buildroot, will get stuck with an old archive for that package, and after updating their Buildroot copy, will be greeted with a failed download, due to the local file not matching the new hashes.

So, to avoid this situation, check the hashes prior to doing the download. If the hashes match, consider the locally cached file genuine, and do not download it. However, if the locally cached file does not match the known hashes we have for it, it is promptly removed, and a download is re-attempted.

---
 support/download/wrapper | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

index 2f0f016..161df03 100755
@@ -30,9 +30,15 @@ output="${2}"
 hfile="${3}"
 shift 3
 
-# If the output file already exists, do not download it again
+# If the output file already exists, and its hashes are correct, do
+# not download it again. If it exists, but it does not match the
+# known hashes for it, remove the file and do the download.
 if [ -e "${output}" ]; then
-    exit 0
+    if support/download/check-hash "${hfile}" "${output}" "${output##*/}"; then
+        exit 0
+    fi
+    rm -f "${output}"
+    printf "Re-downloading '%s'...\n" "${output##*/}"
 fi
 
 # tmpd is a temporary directory in which helpers may store intermediate

1.9.1

Dear Yann E. MORIN,

So in essence, from now on, at each build, we are re-checking the hashes, while previously they were checked only when the file was downloaded. Not that great for build time, but well, again maybe the time to check the hashes is negligible compared to the build time. And we can assume that a big tarball, which takes a certain time to hash, will also contain a lot of source code to build, so the time to calculate the hash is proportional to the build time of the package. So if you're ready to spend several minutes to build Qt, you're probably ready to wait a few more seconds to calculate the hash of the Qt tarball each time.

Best regards,

Thomas

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

Hi,

I doubt it adds any significant time. Probably the hash and extract steps are largely I/O bound, so "pre-heating" the page cache by calculating a hash on the tarball before extracting it shouldn't matter much.

Bye, Peter Korsgaard

Dear Peter Korsgaard,

True. Anyway, my comment really wasn't rejecting the idea, just pointing out the additional stuff we are doing.

Thomas

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

Thomas, All,

On 2014-12-02 09:31 +0100, Thomas Petazzoni spake thusly:

I just hashed a 231MiB tarball (qt-everywhere-opensource-src-4.8.6.tar.gz) and it takes (wall-clock time, core-i5 @2.5GHz):
- cache-cold sha1 : 1.914s
- cache-hot sha1 : 0.762s
- cache-hot sha256: 1.270s

(cache-cold sha256 is not easily done, I'd have to either umount /home or reboot...)

Needless to say this is negligible when compared to the build time of Qt (I do not have the numbers available for now, but I doubt it would be less than 30 seconds on my machine).

I will add those numbers in the commit log next time I respin.

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

Thomas, All,

On 2014-12-03 19:51 +0100, Yann E. MORIN spake thusly:

Please also note that this in fact does not change the current behaviour. Even today, we are checking the hashes for locally cached files, so this solution adds no overhead.

To understand why, here's a summary of what happened before and after thise series:

before: after:

check if file is cached check if the file is cached
if not cached, if cached,
download the file check the hashes
check the hashes if match,
stop
rm cached file
downlaod file check hashes

So, as you can see, in case the file is already cached locally, and the hashes match, we don't do much more than today, except the checks are not done in the same order, which allows us to attempt a re-download in case of hash mismatch.

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

Dear Yann E. MORIN,

define DOWNLOAD_WGET

        test -e $(DL_DIR)/$(2) || \
        $(EXTRA_ENV) support/download/wrapper wget \
                $(DL_DIR)/$(2) \
                '$(call qstrip,$(1))' && \
        $(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))

endef

Hum, the construct

	test -e <foo> || download && check-hash

is not very easy to understand in terms of priorities, but indeed, regardless of whether the download is executed or not, the check-hash part is executed.

Thanks for the explanation!

Thomas

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

Thomas, All,

On 2014-12-06 13:53 +0100, Thomas Petazzoni spake thusly:
[--SNIP--]

Well, in shell, the || and && operators have the same priority. In fact, POSIX does not state that there should be any priority or that there should be one. The phrasing in POSIX is very ambiguous:

AND Lists

The control operator "&&" denotes an AND list. The format shall be:

command1 [ && command2] ...

First command1 shall be executed. If its exit status is zero, command2 shall be executed, and so on, until a command has a non-zero exit status or there are no more commands left to execute. The commands are expanded only if they are executed.

Exit Status

The exit status of an AND list shall be the exit status of the last command that is executed in the list.

OR Lists

The control operator "||" denotes an OR List. The format shall be:

command1 [ || command2] ...

First, command1 shall be executed. If its exit status is non-zero, command2 shall be executed, and so on, until a command has a zero exit status or there are no more commands left to execute.

Exit Status

The exit status of an OR list shall be the exit status of the last command that is executed in the list.

So, if one wants priority to be allied, one should use a compund command, either {compound-list;} or (compound-list).

Yes, this sucks.

The advantage of moving that to the download wrapper makes the Makefile code shrink considerably and makes it easier to read, since we get rid of these AND-OR lists, we are non-trivial to read.

(Note: I too was very surprised that || and && do not have a priority, and was bitten by it more often than not in the past; still today I do this mistake quite often.)

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