[Buildroot] [PATCH 1/7 v4] support/download: make hash file optional

[Buildroot] [PATCH 1/7 v4] support/download: make hash file optional

[Yann E. MORIN]

Currently, specifying a hash file for our download wrapper is mandatory.

However, when we download a git, svn, bzr, hg or cvs tree, there's by
design no hash to check the download against.

Since we're going to have hash checking mandatory when a hash file
exists, this would break those downloads from a repository.

So, make specifying a hash file optional when calling our download
wrapper and bail out early from the check-hash script if no hash file is
specified.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Reviewed-by: Samuel Martin <s.martin49@gmail.com>
---
 support/download/check-hash | 2 +-
 support/download/dl-wrapper | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/support/download/check-hash b/support/download/check-hash
index 4c07274..cee64ef 100755
--- a/support/download/check-hash
+++ b/support/download/check-hash
@@ -23,7 +23,7 @@ file="${2}"
 base="${3}"
 
 # Does the hash-file exist?
-if [ ! -f "${h_file}" ]; then
+if [ -z "${h_file}" -o ! -f "${h_file}" ]; then
     exit 0
 fi
 
diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
index 3b30840..514118c 100755
--- a/support/download/dl-wrapper
+++ b/support/download/dl-wrapper
@@ -44,9 +44,6 @@ main() {
     if [ -z "${output}" ]; then
         error "no output specified, use -o\n"
     fi
-    if [ -z "${hfile}" ]; then
-        error "no hash-file specified, use -H\n"
-    fi
 
     # If the output file already exists, do not download it again
     if [ -e "${output}" ]; then
-- 
1.9.1

[Buildroot] [PATCH 2/7 v4] package infra: do not check hashes when downloading from a repository

[Yann E. MORIN]

When downloading from a repository, we have no way to ensure the
reproducibility of the generated archives, so we can't check the hashes.

Do not specifiy a hash file in those cases.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Reviewed-by: Samuel Martin <s.martin49@gmail.com>

---
Changes v2 -> v3:
  - add blurb in the manual  (Arnout)
---
 docs/manual/adding-packages-directory.txt | 6 ++++++
 package/pkg-download.mk                   | 5 -----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/docs/manual/adding-packages-directory.txt b/docs/manual/adding-packages-directory.txt
index 1ce9a3b..132c702 100644
--- a/docs/manual/adding-packages-directory.txt
+++ b/docs/manual/adding-packages-directory.txt
@@ -474,4 +474,10 @@ environment variable +BR2_ENFORCE_CHECK_HASH+ to a non-empty value, and
 there is no hash for a downloaded file, Buildroot considers this an
 error, deletes the downloaded file, and aborts.
 
+Sources that are downloaded from a version control system (git, subversion,
+etc...) can not have a hash, because the version control system and tar
+may not create exactly the same file (dates, files ordering...), so the
+hash could be wrong even for a valid download. Therefore, the hash check
+is entirely skipped for such sources.
+
 If the +.hash+ file is missing, then no check is done at all.
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 5e74519..e274712 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -87,7 +87,6 @@ github = https://github.com/$(1)/$(2)/archive/$(3)
 define DOWNLOAD_GIT
 	$(EXTRA_ENV) $(DL_WRAPPER) -b git \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
-		-H $(PKGDIR)/$($(PKG)_RAWNAME).hash \
 		$(QUIET) \
 		-- \
 		$($(PKG)_SITE) \
@@ -109,7 +108,6 @@ endef
 define DOWNLOAD_BZR
 	$(EXTRA_ENV) $(DL_WRAPPER) -b bzr \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
-		-H $(PKGDIR)/$($(PKG)_RAWNAME).hash \
 		$(QUIET) \
 		-- \
 		$($(PKG)_SITE) \
@@ -128,7 +126,6 @@ endef
 define DOWNLOAD_CVS
 	$(EXTRA_ENV) $(DL_WRAPPER) -b cvs \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
-		-H $(PKGDIR)/$($(PKG)_RAWNAME).hash \
 		$(QUIET) \
 		-- \
 		$(call stripurischeme,$(call qstrip,$($(PKG)_SITE))) \
@@ -149,7 +146,6 @@ endef
 define DOWNLOAD_SVN
 	$(EXTRA_ENV) $(DL_WRAPPER) -b svn \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
-		-H $(PKGDIR)/$($(PKG)_RAWNAME).hash \
 		$(QUIET) \
 		-- \
 		$($(PKG)_SITE) \
@@ -189,7 +185,6 @@ endef
 define DOWNLOAD_HG
 	$(EXTRA_ENV) $(DL_WRAPPER) -b hg \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
-		-H $(PKGDIR)/$($(PKG)_RAWNAME).hash \
 		$(QUIET) \
 		-- \
 		$($(PKG)_SITE) \
-- 
1.9.1

[Buildroot] [PATCH 0/7 v4] support/download: be more aggressive on missing hashes (branch yem/dl-hash-2)

[Yann E. MORIN]

Hello All!

This series makes hashes mandatory when a .hash file exists.

Currently, we treat missing hashes as a mere warning. Unfortunately,
that often goes un-noticed by most users, and thus we get a lot of
package bumps that are missing the new hashes corresponding to the new
version.

We now make that a hard error, so users really notice something is
wrong.

Of course, if no .hash file exists, the behaviour is as yet unchanged.


Changes v3 -> v4:
  - fix typoes  (Samuel)

Changes v2 -> v3:
  - further differentiat hash check errors  (Arnout)
  - add 'none' hash type  (Arnout)
  - further improve the manual  (Arnout)

Changes v1 -> v2:
  - make it work for downloads from git/svn/... repositories


Regards,
Yann E. MORIN.


The following changes since commit f3383b0eabab16865c630710f71d318bcc9e9f38:

  python-ws4py: new package (2015-04-01 00:05:15 +0200)

are available in the git repository at:

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

for you to fetch changes up to 4a4bae18e06b5e4205eb64c1384485c920325aa3:

  support/download: warn when there's no .hash file (2015-04-01 00:11:28 +0200)

----------------------------------------------------------------
Yann E. MORIN (7):
      support/download: make hash file optional
      package infra: do not check hashes when downloading from a repository
      support/download: add explicit no-hash support
      support/download: return different exit codes for different failures
      support/download: properly catch missing hashes
      support/download: always fail when there's no hash
      support/download: warn when there's no .hash file

 docs/manual/adding-packages-directory.txt | 27 +++++++++++++++++++++------
 package/pkg-download.mk                   |  5 -----
 support/download/check-hash               | 28 +++++++++++++++++++---------
 support/download/dl-wrapper               | 14 ++++++++++----
 4 files changed, 50 insertions(+), 24 deletions(-)

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

[Buildroot] [PATCH 3/7 v4] support/download: add explicit no-hash support

[Yann E. MORIN]

Add support to explicitly state that an archive has no hash.

This can be used for archives downloaded from a repository, like a
git-clone or a subversion checkout, or using the github helper.

This will come in handy when we'll eventually make hashes mandatory as
soon as a .hash file exists: for some packages, like gcc, some versions
are downloaded as archives from upstream, while other versions may come
from a GitHub repository (via the github herlper).

In this case, a .hash file would exist, that contains hashes for the
downloaded tarballs, but archives downloaded from the repository would
not have a hash (since it is currently not possible to have reproducible
such archives). So, we'd need a way to explicitly state there is no
hash, on purpose, for those archives.

So, add 'none' as a new type of hash.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Reviewed-by: Samuel Martin <s.martin49@gmail.com>

---
Changes v3 -> v4:
  - code formatting  (Arnout)
---
 docs/manual/adding-packages-directory.txt | 13 +++++++++++--
 support/download/check-hash               |  4 ++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/docs/manual/adding-packages-directory.txt b/docs/manual/adding-packages-directory.txt
index 132c702..0f8bb64 100644
--- a/docs/manual/adding-packages-directory.txt
+++ b/docs/manual/adding-packages-directory.txt
@@ -423,8 +423,9 @@ The format of this file is one line for each file for which to check the
 hash, each line being space-separated, with these three fields:
 
 * the type of hash, one of:
-** +sha1+, +sha224+, +sha256+, +sha384+, +sha512+
+** +sha1+, +sha224+, +sha256+, +sha384+, +sha512+, +none+
 * the hash of the file:
+** for +none+, one or more non-space chars, usually just the string +xxx+
 ** for +sha1+, 40 hexadecimal characters
 ** for +sha224+, 56 hexadecimal characters
 ** for +sha256+, 64 hexadecimal characters
@@ -448,9 +449,14 @@ comment line above the hashes.
 *Note:* the number of spaces does not matter, so one can use spaces to
 properly align the different fields.
 
+The +none+ hash type is reserved to those archives downloaded from a
+repository, like a 'git clone', a 'subversion checkout'... or archives
+downloaded with the xref:github-download-url[github helper].
+
 The example below defines a +sha1+ and a +sha256+ published by upstream for
 the main +libfoo-1.2.3.tar.bz2+ tarball, plus two locally-computed hashes,
-a +sha256+ for a downloaded patch, and a +sha1+ for a downloaded binary blob:
+a +sha256+ for a downloaded patch, a +sha1+ for a downloaded binary blob,
+and an archive with no hash:
 
 ----
 # Hashes from: http://www.foosoftware.org/download/libfoo-1.2.3.tar.bz2.{sha1,sha256}:
@@ -460,6 +466,9 @@ sha256 efc8103cc3bcb06bda6a781532d12701eb081ad83e8f90004b39ab81b65d4369 libfoo-1
 # No upstream hashes for the following:
 sha256 ff52101fb90bbfc3fe9475e425688c660f46216d7e751c4bbdb1dc85cdccacb9 libfoo-fix-blabla.patch
 sha1   2d608f3c318c6b7557d551a5a09314f03452f1a1                         libfoo-data.bin
+
+# Explicitly no hash for that file, comes from a git-clone:
+none   xxx                                                              libfoo-1234.tar.gz
 ----
 
 If the +.hash+ file is present, and it contains one or more hashes for a
diff --git a/support/download/check-hash b/support/download/check-hash
index cee64ef..7a30d5b 100755
--- a/support/download/check-hash
+++ b/support/download/check-hash
@@ -38,7 +38,11 @@ check_one_hash() {
     # Note: md5 is supported, but undocumented on purpose.
     # Note: sha3 is not supported, since there is currently no implementation
     #       (the NIST has yet to publish the parameters).
+    # Note: 'none' means there is explicitly no hash for that file.
     case "${_h}" in
+        none)
+            return 0
+            ;;
         md5|sha1)                       ;;
         sha224|sha256|sha384|sha512)    ;;
         *) # Unknown hash, exit with error
-- 
1.9.1

[Buildroot] [PATCH 4/7 v4] support/download: return different exit codes for different failures

[Yann E. MORIN]

Return different exit codes depending on the error that occured:

  0: no error (hash file missing, or all hashes match)
  1: unknown option
  2: hash file exists, but at least one hash in error
  3: hash file exists, but no hash for file to check
  4: hash file exists, but at least one hash type unknown

This will be used in a later patch to decide whether the downloaded file
should be kept or removed.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Samuel Martin <s.martin49@gmail.com>
Cc: Arnout Vandecappelle <arnout@mind.be>

---
Changes v3 -> v4:
  - typo in comment  (Samuel)

Changes v2 -> v3:
  - further differentiate errors for unknown hashes and unknown
    command-line options  (Arnout)

Changes v1 -> v2:
  - typoes in script and commit  (Samuel)
---
 support/download/check-hash | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/support/download/check-hash b/support/download/check-hash
index 7a30d5b..ab416c7 100755
--- a/support/download/check-hash
+++ b/support/download/check-hash
@@ -9,6 +9,15 @@ set -e
 #   $3: the final basename of the file, to which it will be ultimately
 #       saved as, to be able to match it to the corresponding hashes
 #       in the .hash file
+#
+# Exit codes:
+#   0:  the hash file exists and the file to check matches all its hashes,
+#       or the hash file does not exist
+#   1:  unknown command-line option
+#   2:  the hash file exists and the file to check does not match at least
+#       one of its hashes
+#   3:  the hash file exists and there was no hash to check the file against
+#   4:  the hash file exists and at least one hash type is unknown
 
 while getopts :q OPT; do
     case "${OPT}" in
@@ -48,7 +57,7 @@ check_one_hash() {
         *) # Unknown hash, exit with error
             printf "ERROR: unknown hash '%s' for '%s'\n"  \
                    "${_h}" "${base}" >&2
-            exit 1
+            exit 4
             ;;
     esac
 
@@ -64,7 +73,7 @@ check_one_hash() {
     printf "ERROR: got     : %s\n" "${_hash}" >&2
     printf "ERROR: Incomplete download, or man-in-the-middle (MITM) attack\n" >&2
 
-    exit 1
+    exit 2
 }
 
 # Do we know one or more hashes for that file?
@@ -87,7 +96,7 @@ done <"${h_file}"
 if [ ${nb_checks} -eq 0 ]; then
     if [ -n "${BR2_ENFORCE_CHECK_HASH}" ]; then
         printf "ERROR: No hash found for %s\n" "${base}" >&2
-        exit 1
+        exit 3
     else
         printf "WARNING: No hash found for %s\n" "${base}" >&2
     fi
-- 
1.9.1

[Buildroot] [PATCH 5/7 v4] support/download: properly catch missing hashes

[Yann E. MORIN]

When checking hashes reports no hash for a file, and this is treated as
an error (now: because BR2_ENFORCE_CHECK_HASH is set; later: because
that will be the new and only behaviour), exit promptly in error.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
Reviewed-by: Samuel Martin <s.martin49@gmail.com>

---
Changes v2 -> v3:
  - change the exit code to 1 (no need to differentiate exit codes here)
---
 support/download/dl-wrapper | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
index 514118c..ef2d872 100755
--- a/support/download/dl-wrapper
+++ b/support/download/dl-wrapper
@@ -45,10 +45,19 @@ main() {
         error "no output specified, use -o\n"
     fi
 
-    # If the output file already exists, do not download it again
+    # If the output file already exists and:
+    # - there's no .hash file: do not download it again and exit promptly
+    # - matches all its hashes: do not download it again and exit promptly
+    # - fails at least one of its hashes: force a re-download
+    # - there's no hash (but a .hash file): consider it a hard error
     if [ -e "${output}" ]; then
         if support/download/check-hash ${quiet} "${hfile}" "${output}" "${output##*/}"; then
             exit 0
+        elif [ ${?} -ne 2 ]; then
+            # Do not remove the file, otherwise it might get re-downloaded
+            # from a later location (i.e. primary -> upstream -> mirror).
+            # Do not print a message, check-hash already did.
+            exit 1
         fi
         rm -f "${output}"
         warn "Re-downloading '%s'...\n" "${output##*/}"
-- 
1.9.1

[Buildroot] [PATCH 6/7 v4] support/download: always fail when there's no hash

[Yann E. MORIN]

At the time we introduced hashes, we did not want to be too harsh in the
beginning, and give people some time to adapt and accept the hashes. So
we so far only whined^Wwarned about a missing hash (when the .hash file
exists).

Some time has passed now, and people are still missing updating hashes
when bumping packages.

Let's make that warning a little bit more annoying...

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Reviewed-by: Samuel Martin <s.martin49@gmail.com>

---
Changes v2 -> v3:
  - further improve the manual  (Arnout)
---
 docs/manual/adding-packages-directory.txt | 8 ++++----
 support/download/check-hash               | 8 ++------
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/docs/manual/adding-packages-directory.txt b/docs/manual/adding-packages-directory.txt
index 0f8bb64..0051fea 100644
--- a/docs/manual/adding-packages-directory.txt
+++ b/docs/manual/adding-packages-directory.txt
@@ -478,10 +478,10 @@ not match, Buildroot considers this an error, deletes the downloaded file,
 and aborts.
 
 If the +.hash+ file is present, but it does not contain a hash for a
-downloaded file, no check is done for that file. If you set the
-environment variable +BR2_ENFORCE_CHECK_HASH+ to a non-empty value, and
-there is no hash for a downloaded file, Buildroot considers this an
-error, deletes the downloaded file, and aborts.
+downloaded file, Buildroot considers this an error and aborts. However,
+the downloaded file is left in the download directory since this
+typically indicates that the +.hash+ file is wrong but the downloaded
+file is probably OK.
 
 Sources that are downloaded from a version control system (git, subversion,
 etc...) can not have a hash, because the version control system and tar
diff --git a/support/download/check-hash b/support/download/check-hash
index ab416c7..13a76b8 100755
--- a/support/download/check-hash
+++ b/support/download/check-hash
@@ -94,10 +94,6 @@ while read t h f; do
 done <"${h_file}"
 
 if [ ${nb_checks} -eq 0 ]; then
-    if [ -n "${BR2_ENFORCE_CHECK_HASH}" ]; then
-        printf "ERROR: No hash found for %s\n" "${base}" >&2
-        exit 3
-    else
-        printf "WARNING: No hash found for %s\n" "${base}" >&2
-    fi
+    printf "ERROR: No hash found for %s\n" "${base}" >&2
+    exit 3
 fi
-- 
1.9.1

[Buildroot] [PATCH 7/7 v4] support/download: warn when there's no .hash file

[Yann E. MORIN]

Instead of silently accepting a missing .hash file, print a warning.

This can be grepped from a build log, to find packages that still have
no hash, with the long-term goal of adding hashes for all packages.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Reviewed-by: Samuel Martin <s.martin49@gmail.com>
---
 support/download/check-hash | 1 +
 1 file changed, 1 insertion(+)

diff --git a/support/download/check-hash b/support/download/check-hash
index 13a76b8..3483077 100755
--- a/support/download/check-hash
+++ b/support/download/check-hash
@@ -33,6 +33,7 @@ base="${3}"
 
 # Does the hash-file exist?
 if [ -z "${h_file}" -o ! -f "${h_file}" ]; then
+    printf "WARNING: no hash file for %s\n" "${base}" >&2
     exit 0
 fi
 
-- 
1.9.1

[Buildroot] [PATCH 0/7 v4] support/download: be more aggressive on missing hashes (branch yem/dl-hash-2)

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Wed,  1 Apr 2015 00:15:03 +0200, Yann E. MORIN wrote:
> Hello All!
> 
> This series makes hashes mandatory when a .hash file exists.
> 
> Currently, we treat missing hashes as a mere warning. Unfortunately,
> that often goes un-noticed by most users, and thus we get a lot of
> package bumps that are missing the new hashes corresponding to the new
> version.
> 
> We now make that a hard error, so users really notice something is
> wrong.
> 
> Of course, if no .hash file exists, the behaviour is as yet unchanged.

I applied this and was going to push it, but I believe there's still a
problem.

If I change strace.hash so that there is no hash matching the tarball
name of strace, then I get two times the error:

$ make strace-extract
ERROR: No hash found for strace-4.10.tar.xz
ERROR: No hash found for strace-4.10.tar.xz
package/pkg-generic.mk:73: recipe for target '/home/thomas/projets/buildroot/output/build/strace-4.10/.stamp_downloaded' failed

I haven't looked too deeply, but I believe it's because check-hash
returns 3, so dl-wrapper exits with error code 1, which means that the
pkg-download.mk logic concludes that the download from the upstream
location has failed, so it retries with sources.buildroot.net, and the
same thing happens.

Is this expected?

Best regards,

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

[Buildroot] [PATCH 0/7 v4] support/download: be more aggressive on missing hashes (branch yem/dl-hash-2)

[Yann E. MORIN]

Thomas, All,

On 2015-04-01 22:08 +0200, Thomas Petazzoni spake thusly:
> On Wed,  1 Apr 2015 00:15:03 +0200, Yann E. MORIN wrote:
> > This series makes hashes mandatory when a .hash file exists.
[--SNIP--]
> I applied this and was going to push it, but I believe there's still a
> problem.
> 
> If I change strace.hash so that there is no hash matching the tarball
> name of strace, then I get two times the error:
> 
> $ make strace-extract
> ERROR: No hash found for strace-4.10.tar.xz
> ERROR: No hash found for strace-4.10.tar.xz
> package/pkg-generic.mk:73: recipe for target '/home/thomas/projets/buildroot/output/build/strace-4.10/.stamp_downloaded' failed
> 
> I haven't looked too deeply, but I believe it's because check-hash
> returns 3, so dl-wrapper exits with error code 1, which means that the
> pkg-download.mk logic concludes that the download from the upstream
> location has failed, so it retries with sources.buildroot.net, and the
> same thing happens.
> 
> Is this expected?

That's at least the result I expect, yes.

I know this might look weird, indeed. However, I did not find a
simple
way to avoid this.

The problem is that we have a biggish block of succesive
conditions in
package/pkg-download:

    define DOWNLOAD_INNER
        $(Q)if test -n "$(call qstrip,$(BR2_PRIMARY_SITE))" ; then \
                case "$(call geturischeme,$(BR2_PRIMARY_SITE))" in \
                        scp) $(call $(DL_MODE)_SCP,$(BR2_PRIMARY_SITE)/$(2),$(2)) && exit ;; \
                        *) $(call $(DL_MODE)_WGET,$(BR2_PRIMARY_SITE)/$(2),$(2)) && exit ;; \
                esac ; \
        fi ; \
        if test "$(BR2_PRIMARY_SITE_ONLY)" = "y" ; then \
                exit 1 ; \
        fi ; \
        if test -n "$(1)" ; then \
                if test -z "$($(PKG)_SITE_METHOD)" ; then \
                        scheme="$(call geturischeme,$(1))" ; \
                else \
                        scheme="$($(PKG)_SITE_METHOD)" ; \
                fi ; \
                case "$$scheme" in \
                        git) $($(DL_MODE)_GIT) && exit ;; \
                        svn) $($(DL_MODE)_SVN) && exit ;; \
                        cvs) $($(DL_MODE)_CVS) && exit ;; \
                        bzr) $($(DL_MODE)_BZR) && exit ;; \
                        file) $($(DL_MODE)_LOCALFILES) && exit ;; \
                        scp) $($(DL_MODE)_SCP) && exit ;; \
                        hg) $($(DL_MODE)_HG) && exit ;; \
                        *) $(call $(DL_MODE)_WGET,$(1),$(2)) && exit ;; \
                esac ; \
        fi ; \
        if test -n "$(call qstrip,$(BR2_BACKUP_SITE))" ; then \
                $(call $(DL_MODE)_WGET,$(BR2_BACKUP_SITE)/$(2),$(2)) && exit ; \
        fi ; \
        exit 1
    endef

So, we would need to catch missing-hash failure in:
  - primary site,
  - upstream loction,
  - backup site.

But catching missing-hash failure needs a bit of a convoluted script
syntax, something like:

    if $($(DL_MODE)_GIT); then \
        : success; \
        exit; \
    else \
        if [ ${?} -eq 1 ]; then \
            : normal failure; \
        else \
            : missing-hash failure; \
            exit 1; \
        fi; \
    fi

And repeat for all other download methods...

That's a bit cumbersome to write. Unless we introduce yet another macro
that expands this code.

Your opinion?

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH 0/7 v4] support/download: be more aggressive on missing hashes (branch yem/dl-hash-2)

[Arnout Vandecappelle]

On 01/04/15 22:52, Yann E. MORIN wrote:
> Thomas, All,
> 
> On 2015-04-01 22:08 +0200, Thomas Petazzoni spake thusly:
>> On Wed,  1 Apr 2015 00:15:03 +0200, Yann E. MORIN wrote:
>>> This series makes hashes mandatory when a .hash file exists.
> [--SNIP--]
>> I applied this and was going to push it, but I believe there's still a
>> problem.
>>
>> If I change strace.hash so that there is no hash matching the tarball
>> name of strace, then I get two times the error:
>>
>> $ make strace-extract
>> ERROR: No hash found for strace-4.10.tar.xz
>> ERROR: No hash found for strace-4.10.tar.xz
>> package/pkg-generic.mk:73: recipe for target '/home/thomas/projets/buildroot/output/build/strace-4.10/.stamp_downloaded' failed
>>
>> I haven't looked too deeply, but I believe it's because check-hash
>> returns 3, so dl-wrapper exits with error code 1, which means that the
>> pkg-download.mk logic concludes that the download from the upstream
>> location has failed, so it retries with sources.buildroot.net, and the
>> same thing happens.
>>
>> Is this expected?
> 
> That's at least the result I expect, yes.
> 
> I know this might look weird, indeed. However, I did not find a
> simple
> way to avoid this.

 I think this weird behaviour is acceptable (for now). It is indeed hard to
avoid, and anyway this is not something that a normal user would ever encounter.

 In a perfect world the entire download logic would move to a script (which
admittedly would get a sh*tload of arguments). Then it would be much simpler to
exit properly.


 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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 2/7 v4] package infra: do not check hashes when downloading from a repository

[Arnout Vandecappelle]

On 01/04/15 00:15, Yann E. MORIN wrote:
> When downloading from a repository, we have no way to ensure the
> reproducibility of the generated archives, so we can't check the hashes.
> 
> Do not specifiy a hash file in those cases.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 And now I also reviewed the manual update.

 Regards,
 Arnout

> Reviewed-by: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> Changes v2 -> v3:
>   - add blurb in the manual  (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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 4/7 v4] support/download: return different exit codes for different failures

[Arnout Vandecappelle]

On 01/04/15 00:15, Yann E. MORIN wrote:
> Return different exit codes depending on the error that occured:
> 
>   0: no error (hash file missing, or all hashes match)
>   1: unknown option
>   2: hash file exists, but at least one hash in error
>   3: hash file exists, but no hash for file to check
>   4: hash file exists, but at least one hash type unknown
> 
> This will be used in a later patch to decide whether the downloaded file
> should be kept or removed.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Samuel Martin <s.martin49@gmail.com>
> Cc: Arnout Vandecappelle <arnout@mind.be>

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


 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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 2/7 v4] package infra: do not check hashes when downloading from a repository

[Yann E. MORIN]

Arnout, All,

On 2015-04-02 23:38 +0200, Arnout Vandecappelle spake thusly:
> On 01/04/15 00:15, Yann E. MORIN wrote:
> > When downloading from a repository, we have no way to ensure the
> > reproducibility of the generated archives, so we can't check the hashes.
> > 
> > Do not specifiy a hash file in those cases.
> > 
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> 
>  And now I also reviewed the manual update.

Well, I think I just used what you provided in your previous review, so
I assumed your reviewed-by tag was still valid... ;-)

Thanks!

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH 0/7 v4] support/download: be more aggressive on missing hashes (branch yem/dl-hash-2)

[Arnout Vandecappelle]

On 02/04/15 23:34, Arnout Vandecappelle wrote:
> On 01/04/15 22:52, Yann E. MORIN wrote:
>> Thomas, All,
>>
>> On 2015-04-01 22:08 +0200, Thomas Petazzoni spake thusly:
>>> On Wed,  1 Apr 2015 00:15:03 +0200, Yann E. MORIN wrote:
>>>> This series makes hashes mandatory when a .hash file exists.
>> [--SNIP--]
>>> I applied this and was going to push it, but I believe there's still a
>>> problem.
>>>
>>> If I change strace.hash so that there is no hash matching the tarball
>>> name of strace, then I get two times the error:
>>>
>>> $ make strace-extract
>>> ERROR: No hash found for strace-4.10.tar.xz
>>> ERROR: No hash found for strace-4.10.tar.xz
>>> package/pkg-generic.mk:73: recipe for target '/home/thomas/projets/buildroot/output/build/strace-4.10/.stamp_downloaded' failed
>>>
>>> I haven't looked too deeply, but I believe it's because check-hash
>>> returns 3, so dl-wrapper exits with error code 1, which means that the
>>> pkg-download.mk logic concludes that the download from the upstream
>>> location has failed, so it retries with sources.buildroot.net, and the
>>> same thing happens.
>>>
>>> Is this expected?
>>
>> That's at least the result I expect, yes.
>>
>> I know this might look weird, indeed. However, I did not find a
>> simple
>> way to avoid this.
> 
>  I think this weird behaviour is acceptable (for now). It is indeed hard to
> avoid, and anyway this is not something that a normal user would ever encounter.
> 
>  In a perfect world the entire download logic would move to a script (which
> admittedly would get a sh*tload of arguments). Then it would be much simpler to
> exit properly.
> 
> 
>  Regards,
>  Arnout
> 
> [snip]
> 


 BTW, if you don't agree with this, you can still push patches 1 to 5 already.
They've got 2 reviews and they will be necessary anyway.

 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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 2/7 v4] package infra: do not check hashes when downloading from a repository

[Arnout Vandecappelle]

On 02/04/15 23:49, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2015-04-02 23:38 +0200, Arnout Vandecappelle spake thusly:
>> On 01/04/15 00:15, Yann E. MORIN wrote:
>>> When downloading from a repository, we have no way to ensure the
>>> reproducibility of the generated archives, so we can't check the hashes.
>>>
>>> Do not specifiy a hash file in those cases.
>>>
>>> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>>> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>>> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>>
>>  And now I also reviewed the manual update.
> 
> Well, I think I just used what you provided in your previous review, so
> I assumed your reviewed-by tag was still valid... ;-)

 Ah, that explains the complete lack of spelling mistakes :-P

 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:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 2/7 v4] package infra: do not check hashes when downloading from a repository

[Yann E. MORIN]

Arnout, All,

On 2015-04-02 23:57 +0200, Arnout Vandecappelle spake thusly:
> On 02/04/15 23:49, Yann E. MORIN wrote:
[---SNIP--]
> > Well, I think I just used what you provided in your previous review, so
> > I assumed your reviewed-by tag was still valid... ;-)
> 
>  Ah, that explains the complete lack of spelling mistakes :-P

You now *are* on my List. Beware...

Regards,
Yann E. MORIN.

PS. Muhahaha! ;-)

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

[Buildroot] [PATCH 0/7 v4] support/download: be more aggressive on missing hashes (branch yem/dl-hash-2)

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Wed,  1 Apr 2015 00:15:03 +0200, Yann E. MORIN wrote:

> Yann E. MORIN (7):
>       support/download: make hash file optional
>       package infra: do not check hashes when downloading from a repository
>       support/download: add explicit no-hash support
>       support/download: return different exit codes for different failures
>       support/download: properly catch missing hashes
>       support/download: always fail when there's no hash
>       support/download: warn when there's no .hash file

Following the discussion we had, and Arnout comments, I applied this
series as is. We can always think later of a way of solving the double
error message problem, but it is not very important.

Thanks!

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

[Buildroot] [PATCH 0/7 v4] support/download: be more aggressive on missing hashes (branch yem/dl-hash-2)

[Yann E. MORIN]

All,

On 2015-04-01 00:15 +0200, Yann E. MORIN spake thusly:
> This series makes hashes mandatory when a .hash file exists.

Please refrain from applying this series.

It does not play well with our autobuilders: we have a .hash file for
external toolchains: that works well for pre-built toolchains from the
well-known vendors, but that breaks for custom toolchains to be
downloaded, since there's no hash for those.

I'll see what I can come up with.

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