Checksum failure encountered with download

PATCH New Martin Jansa 3 participants 6 messages Apr 11, 2013 – Apr 15, 2013

lore

Share & tools

lore

Martin Jansa

author

2013-04-11 13:48 UTC #0 lore raw

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.

Signature · 1 line

Martin 'JaMa' Jansa     jabber: [email protected]

Richard Purdie

2013-04-11 21:57 UTC #1 lore raw

Quoted 13 lines · Martin Jansa

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)

Quoted 7 lines

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

Martin Jansa

author

2013-04-12 14:25 UTC #2 lore raw

PATCH 1/1 fetch2: rename file with bad checksum instead of removing it completely

In reply to 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 <[email protected]>

---
 lib/bb/fetch2/__init__.py | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

lib/bb/fetch2/__init__.py +18 -1

index 1bf67dd..dd1cc93 100644
@@ -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

Signature · 1 line

1.8.1.5

Trevor Woerner

2013-04-12 20:02 UTC #3 lore raw

In reply to Martin Jansa

Sorry for not noticing this sooner.

Quoted 2 lines · Martin Jansa

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.

Martin Jansa

author

2013-04-12 20:55 UTC #4 lore raw

Quoted 11 lines · Trevor Woerner

Sorry for not noticing this sooner.
Quoted 2 lines · Martin Jansa
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?

Quoted 6 lines

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

Signature · 1 line

Martin 'JaMa' Jansa     jabber: [email protected]

Richard Purdie

2013-04-15 13:59 UTC #5 lore raw

Quoted 19 lines · Martin Jansa

Quoted 11 lines · Trevor Woerner
Sorry for not noticing this sooner.
Quoted 2 lines · Martin Jansa
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?
Quoted 6 lines
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.

Quoted 6 lines

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