launchpad-reviewers team mailing list archive

[Colin Watson <mp+454759@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:deathrow-sha256 into launchpad:master.

Commit message:
deathrow: Compare SHA-256 hashes instead of MD5

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/454759

There's no obvious reason to use the weaker hashes when checking whether we can remove files from archives, and this is a small step towards not having to store MD5 hashes in the first place.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:deathrow-sha256 into launchpad:master.

diff --git a/lib/lp/archivepublisher/deathrow.py b/lib/lp/archivepublisher/deathrow.py
index 439971e..36c280f 100644
--- a/lib/lp/archivepublisher/deathrow.py
+++ b/lib/lp/archivepublisher/deathrow.py
@@ -160,7 +160,7 @@ class DeathRow:
 
         return (sources, binaries)
 
-    def canRemove(self, publication_class, filename, file_md5):
+    def canRemove(self, publication_class, filename, file_sha256):
         """Check if given (filename, MD5) can be removed from the pool.
 
         Check the archive reference-counter implemented in:
@@ -199,7 +199,7 @@ class DeathRow:
             [
                 LibraryFileAlias.content == LibraryFileContent.id,
                 LibraryFileAlias.filename == filename,
-                LibraryFileContent.md5 == file_md5,
+                LibraryFileContent.sha256 == file_sha256,
             ]
         )
 
@@ -247,9 +247,9 @@ class DeathRow:
             files = pub_record.files
             for pub_file in files:
                 filename = pub_file.libraryfile.filename
-                file_md5 = pub_file.libraryfile.content.md5
+                file_sha256 = pub_file.libraryfile.content.sha256
 
-                self.logger.debug("Checking %s (%s)" % (filename, file_md5))
+                self.logger.debug("Checking %s (%s)" % (filename, file_sha256))
 
                 # Calculating the file path in pool.
                 pub_file_details = (
@@ -264,15 +264,17 @@ class DeathRow:
                 # verified. If the verification was already made and the
                 # file is condemned queue the publishing record for removal
                 # otherwise just continue the iteration.
-                if (filename, file_md5) in considered_files:
+                if (filename, file_sha256) in considered_files:
                     self.logger.debug("Already verified.")
                     if file_path in condemned_files:
                         condemned_records.add(pub_record)
                     continue
-                considered_files.add((filename, file_md5))
+                considered_files.add((filename, file_sha256))
 
                 # Check if the removal is allowed, if not continue.
-                if not self.canRemove(publication_class, filename, file_md5):
+                if not self.canRemove(
+                    publication_class, filename, file_sha256
+                ):
                     self.logger.debug("Cannot remove.")
                     continue
 
diff --git a/lib/lp/archivepublisher/tests/deathrow.rst b/lib/lp/archivepublisher/tests/deathrow.rst
index cbe463f..ded2fd6 100644
--- a/lib/lp/archivepublisher/tests/deathrow.rst
+++ b/lib/lp/archivepublisher/tests/deathrow.rst
@@ -268,8 +268,10 @@ Run DeathRow against the current 'removable' context.
     DEBUG 4 Sources
     DEBUG 3 Binaries
     ...
-    DEBUG Checking superseded_666.dsc (02129bb861061d1a052c592e2dc6b383)
-    DEBUG Checking obsolete_666.dsc (02129bb861061d1a052c592e2dc6b383)
+    DEBUG Checking superseded_666.dsc
+    (4b68ab3847feda7d6c62c1fbcbeebfa35eab7351ed5e78f4ddadea5df64b8015)
+    DEBUG Checking obsolete_666.dsc
+    (4b68ab3847feda7d6c62c1fbcbeebfa35eab7351ed5e78f4ddadea5df64b8015)
     ...
     INFO Removing 7 files marked for reaping
     DEBUG Removing superseded/superseded_666.dsc from main
@@ -367,9 +369,11 @@ Now DeathRow considers 'stuck-bin' publications.
     >>> death_row.reap()
     DEBUG 0 Sources
     DEBUG 2 Binaries
-    DEBUG Checking stuck-bin_666_i386.deb (21c2e59531c8710156d34a3c30ac81d5)
+    DEBUG Checking stuck-bin_666_i386.deb
+    (bbeebd879e1dff6918546dc0c179fdde505f2a21591c9a9c96e36b054ec5af83)
     DEBUG Cannot remove.
-    DEBUG Checking stuck-bin_666_i386.deb (21c2e59531c8710156d34a3c30ac81d5)
+    DEBUG Checking stuck-bin_666_i386.deb
+    (bbeebd879e1dff6918546dc0c179fdde505f2a21591c9a9c96e36b054ec5af83)
     DEBUG Already verified.
     INFO Removing 0 files marked for reaping
     INFO Total bytes freed: 0
@@ -393,10 +397,13 @@ single pass.
     >>> death_row.reap()
     DEBUG 0 Sources
     DEBUG 3 Binaries
-    DEBUG Checking stuck-bin_666_i386.deb (21c2e59531c8710156d34a3c30ac81d5)
-    DEBUG Checking stuck-bin_666_i386.deb (21c2e59531c8710156d34a3c30ac81d5)
+    DEBUG Checking stuck-bin_666_i386.deb
+    (bbeebd879e1dff6918546dc0c179fdde505f2a21591c9a9c96e36b054ec5af83)
+    DEBUG Checking stuck-bin_666_i386.deb
+    (bbeebd879e1dff6918546dc0c179fdde505f2a21591c9a9c96e36b054ec5af83)
     DEBUG Already verified.
-    DEBUG Checking stuck-bin_666_i386.deb (21c2e59531c8710156d34a3c30ac81d5)
+    DEBUG Checking stuck-bin_666_i386.deb
+    (bbeebd879e1dff6918546dc0c179fdde505f2a21591c9a9c96e36b054ec5af83)
     DEBUG Already verified.
     INFO Removing 1 files marked for reaping
     DEBUG Removing removed-ignored/stuck-bin_666_i386.deb from main