[PATCH 0/2] Improve usability of checksums

[PATCH 0/2] Improve usability of checksums

[Joshua Lock]

I'd heard some complaints around the usability of checksums and, having done
some recipe work recently, must agree with them.
This series ams to make using checksums less intrusive by:

a) Removing the requirement that if one type of checksum is defined the other
must be too.
b) Reporting all sum mismatches at once
c) Enabling sums to be defined as a parameter of a SRC_URI

Regards,
Joshua

The following changes since commit dca46cc2e1c75b6add2c4801e2994a4812745f5b:

  fetch2: Export additional variables to the fetchers (2011-10-05 14:23:16 +0100)

are available in the git repository at:
  git://github.com/incandescant/bitbake checksum
  https://github.com/incandescant/bitbake/tree/checksum

Joshua Lock (2):
  fetch2: improve usability of checksums
  fetch2: enable checksum definition as SRC_URI parameter

 lib/bb/fetch2/__init__.py |   82 +++++++++++++++++++++++++--------------------
 1 files changed, 46 insertions(+), 36 deletions(-)

-- 
1.7.6.4

[PATCH 1/2] fetch2: improve usability of checksums

[Joshua Lock]

This patch improves the usability of checksums by only requiring one checksum
be defined.

Further, checksum verification will provide as much information as possible
at, rather than a bit at a time. No longer will you need to run fetch, see an
md5sum mismatch, fix it, run fetch, seen an sha256sum mismatch, fix it and
fetch again. If neither checksum is defined we now report both missing sums
at once - rather than one after the other.
Finally, if both sums are incorrect, we'll report both incorrect sums at the
same time.

Fixes part of [YOCTO #1399]

Signed-off-by: Joshua Lock <josh@linux.intel.com>
---
 lib/bb/fetch2/__init__.py |   72 +++++++++++++++++++++++---------------------
 1 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index fb6138b..b2b9d5c 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -93,28 +93,6 @@ class ParameterError(BBFetchException):
          BBFetchException.__init__(self, msg)
          self.args = (message, url)
 
-class MD5SumError(BBFetchException):
-    """Exception raised when a MD5 checksum of a file does not match for a downloaded file"""
-    def __init__(self, path, wanted, got, url):
-         msg = "File: '%s' has md5 checksum %s when %s was expected (from URL: '%s')" % (path, got, wanted, url)
-         self.url = url
-         self.path = path
-         self.wanted = wanted
-         self.got = got
-         BBFetchException.__init__(self, msg)
-         self.args = (path, wanted, got, url)
-
-class SHA256SumError(MD5SumError):
-    """Exception raised when a SHA256 checksum of a file does not match for a downloaded file"""
-    def __init__(self, path, wanted, got, url):
-         msg = "File: '%s' has sha256 checksum %s when %s was expected (from URL: '%s')" % (path, got, wanted, url)
-         self.url = url
-         self.path = path
-         self.wanted = wanted
-         self.got = got
-         BBFetchException.__init__(self, msg)
-         self.args = (path, wanted, got, url)
-
 class NetworkAccess(BBFetchException):
     """Exception raised when network access is disabled but it is required."""
     def __init__(self, url, cmd):
@@ -271,8 +249,8 @@ def verify_checksum(u, ud, d):
     verify the MD5 and SHA256 checksum for downloaded src
 
     return value:
-        - True: checksum matched
-        - False: checksum unmatched
+        - True: a checksum matched
+        - False: neither checksum matched
 
     if checksum is missing in recipes file, "BB_STRICT_CHECKSUM" decide the return value.
     if BB_STRICT_CHECKSUM = "1" then return false as unmatched, otherwise return true as
@@ -285,20 +263,46 @@ def verify_checksum(u, ud, d):
     md5data = bb.utils.md5_file(ud.localpath)
     sha256data = bb.utils.sha256_file(ud.localpath)
 
-    if (ud.md5_expected == None or ud.sha256_expected == None):
-        logger.warn('Missing SRC_URI checksum for %s, consider adding to the recipe:\n'
-                    'SRC_URI[%s] = "%s"\nSRC_URI[%s] = "%s"',
-                    ud.localpath, ud.md5_name, md5data,
-                    ud.sha256_name, sha256data)
-        if bb.data.getVar("BB_STRICT_CHECKSUM", d, True) == "1":
-            raise FetchError("No checksum specified for %s." % u, u)
-        return
+    # If strict checking enabled and neither sum defined, raise error
+    strict = bb.data.getVar("BB_STRICT_CHECKSUM", d, True) or None
+    if (strict and ud.md5_expected == None and ud.sha256_expected == None):
+        raise FetchError('No checksum specified for %s, please add at least one to the recipe:\n'
+                         'SRC_URI[%s] = "%s"\nSRC_URI[%s] = "%s"', u,
+                         ud.localpath, ud.md5_name, md5data,
+                         ud.sha256_name, sha256data)
+
+    # Log missing sums so user can more easily add them
+    if ud.md5_expected == None:
+        logger.warn('Missing md5 SRC_URI checksum for %s, consider adding to the recipe:\n'
+                    'SRC_URI[%s] = "%s"',
+                    ud.localpath, ud.md5_name, md5data)
+
+    if ud.sha256_expected == None:
+        logger.warn('Missing sha256 SRC_URI checksum for %s, consider adding to the recipe:\n'
+                    'SRC_URI[%s] = "%s"',
+                    ud.localpath, ud.sha256_name, sha256data)
+
+    md5mismatch = False
+    sha256mismatch = False
 
     if ud.md5_expected != md5data:
-        raise MD5SumError(ud.localpath, ud.md5_expected, md5data, u)
+        md5mismatch = True
 
     if ud.sha256_expected != sha256data:
-        raise SHA256SumError(ud.localpath, ud.sha256_expected, sha256data, u)
+        sha256mismatch = True
+
+    # We want to alert the user if a checksum is defined in the recipe but
+    # it does not match.
+    msg = ""
+    if md5mismatch and ud.md5_expected:
+        msg = msg + "\nFile: '%s' has %s checksum %s when %s was expected (from URL: '%s')" % (ud.localpath, 'md5', ud.md5_expected, md5data, u)
+
+    if sha256mismatch and ud.sha256_expected:
+        msg = msg + "\nFile: '%s' has %s checksum %s when %s was expected (from URL: '%s')" % (ud.localpath, 'sha256', ud.sha256_expected, sha256data, u)
+
+    if len(msg):
+        raise FetchError('Checksum mismatch!%s' % msg, u)
+
 
 def update_stamp(u, ud, d):
     """
-- 
1.7.6.4

Re: [PATCH 1/2] fetch2: improve usability of checksums

[Chris Larson]

On Tue, Oct 11, 2011 at 10:07 AM, Joshua Lock <josh@linux.intel.com> wrote:
> This patch improves the usability of checksums by only requiring one checksum
> be defined.
>
> Further, checksum verification will provide as much information as possible
> at, rather than a bit at a time. No longer will you need to run fetch, see an
> md5sum mismatch, fix it, run fetch, seen an sha256sum mismatch, fix it and
> fetch again. If neither checksum is defined we now report both missing sums
> at once - rather than one after the other.
> Finally, if both sums are incorrect, we'll report both incorrect sums at the
> same time.
>
> Fixes part of [YOCTO #1399]

Nicely done, the fetch/mismatch cycle was quite an annoying one :)
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

Re: [PATCH 1/2] fetch2: improve usability of checksums

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 1058 bytes --]

On Wed, Oct 12, 2011 at 12:21:42PM -0700, Chris Larson wrote:
> On Tue, Oct 11, 2011 at 10:07 AM, Joshua Lock <josh@linux.intel.com> wrote:
> > This patch improves the usability of checksums by only requiring one checksum
> > be defined.
> >
> > Further, checksum verification will provide as much information as possible
> > at, rather than a bit at a time. No longer will you need to run fetch, see an
> > md5sum mismatch, fix it, run fetch, seen an sha256sum mismatch, fix it and
> > fetch again. If neither checksum is defined we now report both missing sums
> > at once - rather than one after the other.
> > Finally, if both sums are incorrect, we'll report both incorrect sums at the
> > same time.
> >
> > Fixes part of [YOCTO #1399]
> 
> Nicely done, the fetch/mismatch cycle was quite an annoying one :)

yeah, great, thanks!

finally it looks like 
http://git.openembedded.org/openembedded/commit?id=353b4dc7223f8dd50fcf21d7c031b9dd91a724a2
again :)

Regards,

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]

Re: [PATCH 1/2] fetch2: improve usability of checksums

[Joshua Lock]

On 10/11/2011 10:07 AM, Joshua Lock wrote:
>      if ud.sha256_expected != sha256data:
> -        raise SHA256SumError(ud.localpath, ud.sha256_expected, sha256data, u)
> +        sha256mismatch = True
> +
> +    # We want to alert the user if a checksum is defined in the recipe but
> +    # it does not match.
> +    msg = ""
> +    if md5mismatch and ud.md5_expected:
> +        msg = msg + "\nFile: '%s' has %s checksum %s when %s was expected (from URL: '%s')" % (ud.localpath, 'md5', ud.md5_expected, md5data, u)
> +
> +    if sha256mismatch and ud.sha256_expected:
> +        msg = msg + "\nFile: '%s' has %s checksum %s when %s was expected (from URL: '%s')" % (ud.localpath, 'sha256', ud.sha256_expected, sha256data, u)
> +

Just realised I was passing the expected and data parameters in the
wrong order - I've pushed a fix to the same branch.

Cheers,
Joshua
-- 
Joshua Lock
        Yocto Project "Johannes factotum"
        Intel Open Source Technology Centre

[PATCH 2/2] fetch2: enable checksum definition as SRC_URI parameter

[Joshua Lock]

URI parameters should be able to be defined as a parameter of the SRC_URI,
this patch enables thus for checksums.

An example;

SRC_URI = "http://pkgconfig.freedesktop.org/releases/pkg-config-${PV}.tar.gz;md5sum=a3270bab3f4b69b7dc6dbdacbcae9745;sha256sum=3ba691ee2431f32ccb8efa131e59bf23e37f122dc66791309023ca6dcefcd10e"

Addresses the remainder of [YOCTO #1399]

Signed-off-by: Joshua Lock <josh@linux.intel.com>
---
 lib/bb/fetch2/__init__.py |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index b2b9d5c..6c3886b 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -562,8 +562,14 @@ class FetchData(object):
         else:
             self.md5_name = "md5sum"
             self.sha256_name = "sha256sum"
-        self.md5_expected = bb.data.getVarFlag("SRC_URI", self.md5_name, d)
-        self.sha256_expected = bb.data.getVarFlag("SRC_URI", self.sha256_name, d)
+        if self.md5_name in self.parm:
+            self.md5_expected = self.parm[self.md5_name]
+        else:
+            self.md5_expected = bb.data.getVarFlag("SRC_URI", self.md5_name, d)
+        if self.sha256_name in self.parm:
+            self.sha256_expected = self.parm[self.sha256_name]
+        else:
+            self.sha256_expected = bb.data.getVarFlag("SRC_URI", self.sha256_name, d)
 
         self.names = self.parm.get("name",'default').split(',')
 
-- 
1.7.6.4

Re: [PATCH 2/2] fetch2: enable checksum definition as SRC_URI parameter

[Khem Raj]

On Tue, Oct 11, 2011 at 10:07 AM, Joshua Lock <josh@linux.intel.com> wrote:
> URI parameters should be able to be defined as a parameter of the SRC_URI,
> this patch enables thus for checksums.
>
> An example;
>
> SRC_URI = "http://pkgconfig.freedesktop.org/releases/pkg-config-${PV}.tar.gz;md5sum=a3270bab3f4b69b7dc6dbdacbcae9745;sha256sum=3ba691ee2431f32ccb8efa131e59bf23e37f122dc66791309023ca6dcefcd10e"

this should go into manual/documentation as well.

>
> Addresses the remainder of [YOCTO #1399]
>
> Signed-off-by: Joshua Lock <josh@linux.intel.com>
> ---
>  lib/bb/fetch2/__init__.py |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
> index b2b9d5c..6c3886b 100644
> --- a/lib/bb/fetch2/__init__.py
> +++ b/lib/bb/fetch2/__init__.py
> @@ -562,8 +562,14 @@ class FetchData(object):
>         else:
>             self.md5_name = "md5sum"
>             self.sha256_name = "sha256sum"
> -        self.md5_expected = bb.data.getVarFlag("SRC_URI", self.md5_name, d)
> -        self.sha256_expected = bb.data.getVarFlag("SRC_URI", self.sha256_name, d)
> +        if self.md5_name in self.parm:
> +            self.md5_expected = self.parm[self.md5_name]
> +        else:
> +            self.md5_expected = bb.data.getVarFlag("SRC_URI", self.md5_name, d)
> +        if self.sha256_name in self.parm:
> +            self.sha256_expected = self.parm[self.sha256_name]
> +        else:
> +            self.sha256_expected = bb.data.getVarFlag("SRC_URI", self.sha256_name, d)
>
>         self.names = self.parm.get("name",'default').split(',')
>
> --
> 1.7.6.4
>
>
> _______________________________________________
> bitbake-devel mailing list
> bitbake-devel@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/bitbake-devel
>

Re: [PATCH 2/2] fetch2: enable checksum definition as SRC_URI parameter

[Joshua Lock]

On Tue, 2011-10-11 at 11:05 -0700, Khem Raj wrote:
> On Tue, Oct 11, 2011 at 10:07 AM, Joshua Lock <josh@linux.intel.com> wrote:
> > URI parameters should be able to be defined as a parameter of the SRC_URI,
> > this patch enables thus for checksums.
> >
> > An example;
> >
> > SRC_URI = "http://pkgconfig.freedesktop.org/releases/pkg-config-${PV}.tar.gz;md5sum=a3270bab3f4b69b7dc6dbdacbcae9745;sha256sum=3ba691ee2431f32ccb8efa131e59bf23e37f122dc66791309023ca6dcefcd10e"
> 
> this should go into manual/documentation as well.

Good point, I pushed an extra patch onto the branch for the manual:
https://github.com/incandescant/bitbake/commit/fcf17a39ce89b5911ffd9f89ff13505a7a7633fd

Regards,
Joshua
-- 
Joshua Lock
        Yocto Project "Johannes factotum"
        Intel Open Source Technology Centre

Re: [PATCH 0/2] Improve usability of checksums

[Scott Garman]

On 10/11/2011 10:07 AM, Joshua Lock wrote:
> I'd heard some complaints around the usability of checksums and, having done
> some recipe work recently, must agree with them.
> This series ams to make using checksums less intrusive by:
>
> a) Removing the requirement that if one type of checksum is defined the other
> must be too.
> b) Reporting all sum mismatches at once
> c) Enabling sums to be defined as a parameter of a SRC_URI

Very nice, thank you!

Acked-by: Scott Garman <scott.a.garman@intel.com>

-- 
Scott Garman
Embedded Linux Engineer - Yocto Project
Intel Open Source Technology Center

Re: [PATCH 0/2] Improve usability of checksums

[Richard Purdie]

On Tue, 2011-10-11 at 10:07 -0700, Joshua Lock wrote:
> I'd heard some complaints around the usability of checksums and, having done
> some recipe work recently, must agree with them.
> This series ams to make using checksums less intrusive by:
> 
> a) Removing the requirement that if one type of checksum is defined the other
> must be too.
> b) Reporting all sum mismatches at once
> c) Enabling sums to be defined as a parameter of a SRC_URI
> 
> Regards,
> Joshua
> 
> The following changes since commit dca46cc2e1c75b6add2c4801e2994a4812745f5b:
> 
>   fetch2: Export additional variables to the fetchers (2011-10-05 14:23:16 +0100)
> 
> are available in the git repository at:
>   git://github.com/incandescant/bitbake checksum
>   https://github.com/incandescant/bitbake/tree/checksum
> 
> Joshua Lock (2):
>   fetch2: improve usability of checksums
>   fetch2: enable checksum definition as SRC_URI parameter

Merged to master (or will be when I can actually push the changes). 

Cheers,

Richard