Checksum failure encountered with download

Checksum failure encountered with download

[Martin Jansa]

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

Hi,

when fetcher fails with ChecksumError it shows error like in subject and
tries another MIRRORs

Would anyone object to keep fetched source with wrong checksum in
downloads directory (renamed so it does not conflict with file with
correct checksum downloaded later)?

Right now it just removes the file:
                        # Remove any incomplete fetch
                        m.clean(ud, self.d)

The problem with this is when upstream repacks some archive I would like
to be able to compare old tarball and new one.

My proposal is to add md5sum of downloaded file to ChecksumError and
rename it to <file>.<md5sum> in "if isinstance(e, ChecksumError):"

Does it make sense (should I send patch for this)?

FWIW: Emerge in gentoo does something similar.

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

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

Re: Checksum failure encountered with download

[Richard Purdie]

On Thu, 2013-04-11 at 15:30 +0200, Martin Jansa wrote:
> Hi,
> 
> when fetcher fails with ChecksumError it shows error like in subject and
> tries another MIRRORs
> 
> Would anyone object to keep fetched source with wrong checksum in
> downloads directory (renamed so it does not conflict with file with
> correct checksum downloaded later)?
> 
> Right now it just removes the file:
>                         # Remove any incomplete fetch
>                         m.clean(ud, self.d)
> 
> The problem with this is when upstream repacks some archive I would like
> to be able to compare old tarball and new one.
> 
> My proposal is to add md5sum of downloaded file to ChecksumError and
> rename it to <file>.<md5sum> in "if isinstance(e, ChecksumError):"
> 
> Does it make sense (should I send patch for this)?

I'm fine with that.

Cheers,

Richard

Re: [bitbake-devel] Checksum failure encountered with download

[Richard Purdie]

On Thu, 2013-04-11 at 15:30 +0200, Martin Jansa wrote:
> Hi,
> 
> when fetcher fails with ChecksumError it shows error like in subject and
> tries another MIRRORs
> 
> Would anyone object to keep fetched source with wrong checksum in
> downloads directory (renamed so it does not conflict with file with
> correct checksum downloaded later)?
> 
> Right now it just removes the file:
>                         # Remove any incomplete fetch
>                         m.clean(ud, self.d)
> 
> The problem with this is when upstream repacks some archive I would like
> to be able to compare old tarball and new one.
> 
> My proposal is to add md5sum of downloaded file to ChecksumError and
> rename it to <file>.<md5sum> in "if isinstance(e, ChecksumError):"
> 
> Does it make sense (should I send patch for this)?

I'm fine with that.

Cheers,

Richard

[PATCH] fetch2: rename file with bad checksum instead of removing it completely

[Martin Jansa]

* this can be useful when someone wan't to compare old file with
  bad checksum and new one

Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
 lib/bb/fetch2/__init__.py | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/lib/bb/fetch2/__init__.py b/lib/bb/fetch2/__init__.py
index 1bf67dd..dd1cc93 100644
--- a/lib/bb/fetch2/__init__.py
+++ b/lib/bb/fetch2/__init__.py
@@ -74,6 +74,9 @@ class FetchError(BBFetchException):
 
 class ChecksumError(FetchError):
     """Exception when mismatched checksum encountered"""
+    def __init__(self, message, url = None, checksum = None):
+        self.checksum = checksum
+        FetchError.__init__(self, message, url)
 
 class NoChecksumError(FetchError):
     """Exception when no checksum is specified, but BB_STRICT_CHECKSUM is set"""
@@ -561,7 +564,7 @@ def verify_checksum(u, ud, d):
         msg = msg + '\nIf this change is expected (e.g. you have upgraded to a new version without updating the checksums) then you can use these lines within the recipe:\nSRC_URI[%s] = "%s"\nSRC_URI[%s] = "%s"\nOtherwise you should retry the download and/or check with upstream to determine if the file has become corrupted or otherwise unexpectedly modified.\n' % (ud.md5_name, md5data, ud.sha256_name, sha256data)
 
     if len(msg):
-        raise ChecksumError('Checksum mismatch!%s' % msg, u)
+        raise ChecksumError('Checksum mismatch!%s' % msg, u, md5data)
 
 
 def update_stamp(u, ud, d):
@@ -804,6 +807,7 @@ def try_mirror_url(newuri, origud, ud, ld, check = False):
         if isinstance(e, ChecksumError):
             logger.warn("Mirror checksum failure for url %s (original url: %s)\nCleaning and trying again." % (newuri, origud.url))
             logger.warn(str(e))
+            self.rename_bad_checksum(ud, e.checksum)
         elif isinstance(e, NoChecksumError):
             raise
         else:
@@ -1388,6 +1392,7 @@ class Fetch(object):
                         if isinstance(e, ChecksumError):
                             logger.warn("Checksum failure encountered with download of %s - will attempt other sources if available" % u)
                             logger.debug(1, str(e))
+                            self.rename_bad_checksum(ud, e.checksum)
                         elif isinstance(e, NoChecksumError):
                             raise
                         else:
@@ -1495,6 +1500,18 @@ class Fetch(object):
             if ud.lockfile:
                 bb.utils.unlockfile(lf)
 
+    def rename_bad_checksum(self, ud, suffix):
+        """
+        Renames files to have suffix from parameter
+        """
+
+        if ud.localpath is None:
+            return
+
+        new_localpath = "%s_bad-checksum_%s" % (ud.localpath, suffix)
+        bb.warn("Renaming %s to %s" % (ud.localpath, new_localpath))
+        bb.utils.movefile(ud.localpath, new_localpath)
+
 from . import cvs
 from . import git
 from . import gitsm
-- 
1.8.1.5

Re: Checksum failure encountered with download

[Trevor Woerner]

Sorry for not noticing this sooner.

On Thu, Apr 11, 2013 at 9:30 AM, Martin Jansa <martin.jansa@gmail.com> wrote:
> My proposal is to add md5sum of downloaded file to ChecksumError and
> rename it to <file>.<md5sum> in "if isinstance(e, ChecksumError):"

Would it be possible to make this behaviour configurable for those who
might be concerned about disk space? Or perhaps while renaming and
saving it, also move it into the $TMPDIR directory and/or tie this
feature in with rm_work?

I'm just thinking that over time one's DL_DIR could potentially become
rather cluttered with lots of failed downloads. Only some people would
be interested in seeing the failures and the rest would have to remove
them manually. I'm also thinking of the scenario where the DL_DIR is
shared/sync'ed among a group of people/computers; that could add up to
quite a bit of data to share.

Re: [bitbake-devel] Checksum failure encountered with download

[Trevor Woerner]

Sorry for not noticing this sooner.

On Thu, Apr 11, 2013 at 9:30 AM, Martin Jansa <martin.jansa@gmail.com> wrote:
> My proposal is to add md5sum of downloaded file to ChecksumError and
> rename it to <file>.<md5sum> in "if isinstance(e, ChecksumError):"

Would it be possible to make this behaviour configurable for those who
might be concerned about disk space? Or perhaps while renaming and
saving it, also move it into the $TMPDIR directory and/or tie this
feature in with rm_work?

I'm just thinking that over time one's DL_DIR could potentially become
rather cluttered with lots of failed downloads. Only some people would
be interested in seeing the failures and the rest would have to remove
them manually. I'm also thinking of the scenario where the DL_DIR is
shared/sync'ed among a group of people/computers; that could add up to
quite a bit of data to share.

Re: Checksum failure encountered with download

[Martin Jansa]

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

On Fri, Apr 12, 2013 at 03:44:41PM -0400, Trevor Woerner wrote:
> Sorry for not noticing this sooner.
> 
> On Thu, Apr 11, 2013 at 9:30 AM, Martin Jansa <martin.jansa@gmail.com> wrote:
> > My proposal is to add md5sum of downloaded file to ChecksumError and
> > rename it to <file>.<md5sum> in "if isinstance(e, ChecksumError):"
> 
> Would it be possible to make this behaviour configurable for those who
> might be concerned about disk space? Or perhaps while renaming and
> saving it, also move it into the $TMPDIR directory and/or tie this
> feature in with rm_work?
> 
> I'm just thinking that over time one's DL_DIR could potentially become
> rather cluttered with lots of failed downloads. Only some people would
> be interested in seeing the failures and the rest would have to remove
> them manually. I'm also thinking of the scenario where the DL_DIR is
> shared/sync'ed among a group of people/computers; that could add up to
> quite a bit of data to share.

I can make it configurable, but I think that bad-checksums should be
only in few exceptional cases, while package upgrades are common.

That said 90% of my downloads directory are old versions I keep just to
be able to recreate old builds with upstream sources already gone and
now I've one renamed bad-checksum tarball from evas-1.7.6.1...

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

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

Re: [bitbake-devel] Checksum failure encountered with download

[Martin Jansa]

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

On Fri, Apr 12, 2013 at 03:44:41PM -0400, Trevor Woerner wrote:
> Sorry for not noticing this sooner.
> 
> On Thu, Apr 11, 2013 at 9:30 AM, Martin Jansa <martin.jansa@gmail.com> wrote:
> > My proposal is to add md5sum of downloaded file to ChecksumError and
> > rename it to <file>.<md5sum> in "if isinstance(e, ChecksumError):"
> 
> Would it be possible to make this behaviour configurable for those who
> might be concerned about disk space? Or perhaps while renaming and
> saving it, also move it into the $TMPDIR directory and/or tie this
> feature in with rm_work?
> 
> I'm just thinking that over time one's DL_DIR could potentially become
> rather cluttered with lots of failed downloads. Only some people would
> be interested in seeing the failures and the rest would have to remove
> them manually. I'm also thinking of the scenario where the DL_DIR is
> shared/sync'ed among a group of people/computers; that could add up to
> quite a bit of data to share.

I can make it configurable, but I think that bad-checksums should be
only in few exceptional cases, while package upgrades are common.

That said 90% of my downloads directory are old versions I keep just to
be able to recreate old builds with upstream sources already gone and
now I've one renamed bad-checksum tarball from evas-1.7.6.1...

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

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

Re: Checksum failure encountered with download

[Richard Purdie]

On Fri, 2013-04-12 at 22:37 +0200, Martin Jansa wrote:
> On Fri, Apr 12, 2013 at 03:44:41PM -0400, Trevor Woerner wrote:
> > Sorry for not noticing this sooner.
> > 
> > On Thu, Apr 11, 2013 at 9:30 AM, Martin Jansa <martin.jansa@gmail.com> wrote:
> > > My proposal is to add md5sum of downloaded file to ChecksumError and
> > > rename it to <file>.<md5sum> in "if isinstance(e, ChecksumError):"
> > 
> > Would it be possible to make this behaviour configurable for those who
> > might be concerned about disk space? Or perhaps while renaming and
> > saving it, also move it into the $TMPDIR directory and/or tie this
> > feature in with rm_work?
> > 
> > I'm just thinking that over time one's DL_DIR could potentially become
> > rather cluttered with lots of failed downloads. Only some people would
> > be interested in seeing the failures and the rest would have to remove
> > them manually. I'm also thinking of the scenario where the DL_DIR is
> > shared/sync'ed among a group of people/computers; that could add up to
> > quite a bit of data to share.
> 
> I can make it configurable, but I think that bad-checksums should be
> only in few exceptional cases, while package upgrades are common.
> 
> That said 90% of my downloads directory are old versions I keep just to
> be able to recreate old builds with upstream sources already gone and
> now I've one renamed bad-checksum tarball from evas-1.7.6.1...

Agreed, I'd hope this isn't something which happens that often.

Cheers,

Richard

Re: [bitbake-devel] Checksum failure encountered with download

[Richard Purdie]

On Fri, 2013-04-12 at 22:37 +0200, Martin Jansa wrote:
> On Fri, Apr 12, 2013 at 03:44:41PM -0400, Trevor Woerner wrote:
> > Sorry for not noticing this sooner.
> > 
> > On Thu, Apr 11, 2013 at 9:30 AM, Martin Jansa <martin.jansa@gmail.com> wrote:
> > > My proposal is to add md5sum of downloaded file to ChecksumError and
> > > rename it to <file>.<md5sum> in "if isinstance(e, ChecksumError):"
> > 
> > Would it be possible to make this behaviour configurable for those who
> > might be concerned about disk space? Or perhaps while renaming and
> > saving it, also move it into the $TMPDIR directory and/or tie this
> > feature in with rm_work?
> > 
> > I'm just thinking that over time one's DL_DIR could potentially become
> > rather cluttered with lots of failed downloads. Only some people would
> > be interested in seeing the failures and the rest would have to remove
> > them manually. I'm also thinking of the scenario where the DL_DIR is
> > shared/sync'ed among a group of people/computers; that could add up to
> > quite a bit of data to share.
> 
> I can make it configurable, but I think that bad-checksums should be
> only in few exceptional cases, while package upgrades are common.
> 
> That said 90% of my downloads directory are old versions I keep just to
> be able to recreate old builds with upstream sources already gone and
> now I've one renamed bad-checksum tarball from evas-1.7.6.1...

Agreed, I'd hope this isn't something which happens that often.

Cheers,

Richard