Merge lp:~robru/cupstream2distro/faster-content-mismatch-detection into lp:cupstream2distro

[Robert Bruce Park]

Be faster about detecting versions with content mismatches.

[Robert Bruce Park (robru)]

Looks excellent in staging.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1242
http://jenkins.qa.ubuntu.com/job/cu2d-choo-choo-ci/918/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/cu2d-choo-choo-ci/918/rebuild

review: Approve (continuous-integration)

[lp:~robru/cupstream2distro/faster-content-mismatch-detection]

1243. By Robert Bruce Park on 2015-11-21

Log when a status is found.

=== modified file 'citrain/recipes/base.py'
--- citrain/recipes/base.py	2015-11-21 04:41:12 +0000
+++ citrain/recipes/base.py	2015-11-21 06:36:05 +0000
@@ -187,15 +187,10 @@
         self.target = None
         self.dest = dest
 
-    def get_version(self):
-        """Identify what version this package is."""
-        for get_ver in (self.get_package_version, self.get_archive_version):
-            version = get_ver()
-            if version:
-                return version
-
     def check_status(self):
         """Identify this package's progress through the train."""
+        version = self.get_package_version()
+        source = self.get_source_from_archive(version=version)
         checks = (
             self.check_silo_version,
             self.check_new_commits,
@@ -206,35 +201,34 @@
             self.check_diff_exists,
             self.check_destination_version,
         )
-        version = self.get_version()
         for check in checks:
             logging.info('%s: %s', self.name, check.__doc__.split('\n')[0])
-            message = check(version)
+            message = check(source=source)
             if message:
+                logging.info('%s: %s', self.name, message)
                 return message
         return 'Successfully built'
 
-    def check_silo_version(self, version):
+    def check_silo_version(self, source=None):
         """Check that local version matches silo version."""
         version = self.get_package_version()
-        silo_version = self.get_archive_version()
-        if not silo_version:
+        if version and not source:
+            return '{} not in PPA'.format(version)
+        if not source:
             return 'Ready to build'
-        if version and version != silo_version:
-            return '{} not in PPA'.format(version)
 
-    def check_new_commits(self, version):
+    def check_new_commits(self, source=None):
         """Not built from merges, skipping new commit check."""
         pass
 
-    def check_destination_version(self, version=None):
+    def check_destination_version(self, source=None):
         """Check that destination version is listed in our changelog."""
         dest_version = self.get_archive_version(self.dest)
         log_value_of.dest_version('Dest archive')
         if dest_version and self.package_changelog_has(dest_version) is False:
             return DEST_MISSING.format(dest_version)
 
-    def check_autopkgtest_status(self, version=None):
+    def check_autopkgtest_status(self, source=None):
         """Check current status of autopkgtests, if any."""
         found = set()
         for arch in AUTOPKGTEST_ARCHES:
@@ -267,11 +261,9 @@
         else:
             return self.inspect_tar_files()
 
-    def _check_build_status(self, version=None):
+    def _check_build_status(self, source=None):
         """Report the build pass/fail status in the PPA."""
         watcher = ArchWatcher(self.arches)
-        source = self.get_source_from_archive(
-            version=version or self.get_version())
         states = defaultdict(list)
         if source:
             for build in source.getBuilds():
@@ -282,13 +274,13 @@
                     states[state].append(build.arch_tag)
         return states
 
-    def check_build_status(self, version):
+    def check_build_status(self, source=None):
         """Report any build failures from the PPA."""
-        states = self._check_build_status(version)
+        states = self._check_build_status(source)
         if states.keys() - {'Successfully built'}:
             return pretty_print_states(states, fmt=ARCH_FMT)
 
-    def check_diff_exists(self, version=None):
+    def check_diff_exists(self, source=None):
         """Complain if the full diff is missing."""
         if not exists(self.full_diff):
             return 'Diff missing'

=== modified file 'citrain/recipes/merge.py'
--- citrain/recipes/merge.py	2015-11-18 23:05:03 +0000
+++ citrain/recipes/merge.py	2015-11-21 06:36:05 +0000
@@ -276,7 +276,7 @@
         if env.ALLOW_UNAPPROVED != 'true':
             self.enforce_merge_states(PUBLISHABLE_STATES)
 
-    def check_new_commits(self, version=None):
+    def check_new_commits(self, source=None):
         """Report if this package needs a rebuild, or not."""
         if check_for_unbuilt_revids(self.name, self.merges):
             return 'Needs rebuild due to new commits'

=== modified file 'cupstream2distro/archive.py'
--- cupstream2distro/archive.py	2015-11-20 23:42:57 +0000
+++ cupstream2distro/archive.py	2015-11-21 06:36:05 +0000
@@ -23,7 +23,7 @@
 from os.path import join
 from operator import attrgetter
 from six.moves.urllib.parse import unquote
-from six.moves.urllib.request import urlretrieve
+from six.moves.urllib.request import urlretrieve, urlopen
 
 from cupstream2distro.version import V
 from cupstream2distro.errors import ArchiveError
@@ -93,10 +93,9 @@
                 if binary.architecture_specific}
         return set()
 
-    def archive_hashes(self, sourcepub):
-        """Return the binary file sha1 hashes for comparison."""
-        return {binary.binaryFileUrls(include_meta=True)[0]['sha1']
-                for binary in sourcepub.getPublishedBinaries()}
+    def archive_id(self, sourcepub):
+        """Return some data that uniquely identifies a package's contents."""
+        return urlopen(sourcepub.changelogUrl()).read(4096)
 
     def get_archive_version(self, archive=None, series=None):
         """Get current version for a package name in that series.
@@ -121,7 +120,9 @@
         kwargs = dict(distro_series=self.series, source_name=self.name)
         if version:
             kwargs['version'] = version
-        return newest(archive or self.silo_ppa, **kwargs)
+        source = newest(archive or self.silo_ppa, **kwargs)
+        if getattr(source, 'status', None) in ('Pending', 'Published'):
+            return source
 
     def copy_to_archive(self, source, archive=None):
         """Copy this source package to a given archive.
@@ -203,31 +204,36 @@
             log_value_of.shortname()
             return shortname
 
-    def archive_pocket(self, version):
+    def archive_pocket(self, source=None):
         """Ask the archive what pocket this version is in.
 
-        :param version: The version you want to check for.
+        :param source: The source package publishing history you want to check.
         :returns: The name of the pocket it's in, or None.
         """
-        kwargs = dict(
-            distro_series=self.series, source_name=self.name, version=version)
-        dest = newest(self.dest, **kwargs)
-        if dest:
-            ours = newest(self.silo_ppa, **kwargs)
-            if self.archive_hashes(ours) == self.archive_hashes(dest):
-                return dest.pocket + ' pocket'
-            logging.info('%s at dest has different contents.', version)
+        if source:
+            dest = newest(
+                self.dest,
+                distro_series=self.series,
+                source_name=self.name,
+                version=source.source_package_version)
+            if dest:
+                if self.archive_id(source) == self.archive_id(dest):
+                    return dest.pocket + ' pocket'
+                return 'Needs rebuild due to burned version number'
 
-    def archive_queue(self, version):
+    def archive_queue(self, source=None):
         """Check what queue this version of the package is in.
 
         :param version: The version you want to check for.
         :returns: The name of the queue it's in, or None.
         """
-        uploads = self.series.getPackageUploads(
-            exact_match=True, name=self.name, version=version)
-        if uploads.total_size > 0:
-            return uploads[0].status.upper() + ' queue'
+        if source:
+            uploads = self.series.getPackageUploads(
+                version=source.source_package_version,
+                exact_match=True,
+                name=self.name)
+            if uploads.total_size > 0:
+                return uploads[0].status.upper() + ' queue'
 
 
 class ArchWatcher(object):

=== modified file 'tests/unit/test_archive.py'
--- tests/unit/test_archive.py	2015-11-20 23:42:57 +0000
+++ tests/unit/test_archive.py	2015-11-21 06:36:05 +0000
@@ -182,6 +182,7 @@
     def test_get_source_from_archive(self):
         """Return LPlib source package object."""
         source = Mock()
+        source.status = 'Pending'
         self.archive.silo_ppa.getPublishedSources.return_value = [source]
         self.assertEqual(self.archive.get_source_from_archive(), source)
         self.archive.silo_ppa.getPublishedSources.assert_called_once_with(
@@ -347,17 +348,6 @@
         with self.assertRaisesRegexp(ArchiveError, 'Download failed'):
             self.archive.unpack_archive_dsc(path, '1.0-0ubuntu1')
 
-    @patch('cupstream2distro.archive.newest')
-    def test_archive_pocket_mismatch(self, new):
-        """Correctly identify when dest has same version with diff contents."""
-        results = [{'hash'}, {'mismatch'}]
-        new.return_value.source_package_version = '42-0ubuntu1'
-        new.return_value.pocket = 'Proposed'
-        new.return_value.status = 'Pending'
-        self.archive.archive_hashes = Mock(
-            side_effect=lambda src: results.pop())
-        self.assertIsNone(self.archive.archive_pocket('42-0ubuntu1'))
-
     def test_watch_arch(self):
         """Ignore the correct arches."""
         watcher = ArchWatcher(['foo', 'i386', 'amd64'])

=== modified file 'tests/unit/test_recipe_base.py'
--- tests/unit/test_recipe_base.py	2015-11-21 00:12:26 +0000
+++ tests/unit/test_recipe_base.py	2015-11-21 06:36:05 +0000
@@ -95,20 +95,20 @@
         """Correctly identify when package missing from PPA."""
         new.return_value = None
         build = BuildBase('state', self.series, self.dest, self.ppa)
-        build.get_package_version = Mock(return_value='42-0ubuntu1')
+        build.get_package_version = Mock(return_value=None)
         self.assertEqual(build.check_status(), 'Ready to build')
 
     @patch('cupstream2distro.archive.newest')
     def test_check_status_mismatch(self, new):
         """Correctly identify when PPA has wrong version."""
-        new.return_value.source_package_version = '41-0ubuntu1'
-        new.return_value.status = 'Published'
+        new.return_value = None
         build = BuildBase('state', self.series, self.dest, self.ppa)
         build.get_package_version = Mock(return_value='42-0ubuntu1')
         self.assertEqual(build.check_status(), '42-0ubuntu1 not in PPA')
 
     @patch('cupstream2distro.archive.newest')
-    def test_check_status_pocket(self, new):
+    @patch('cupstream2distro.archive.urlopen')
+    def test_check_status_pocket(self, urlopen, new):
         """Correctly identify when PPA has wrong version."""
         new.return_value.source_package_version = '42-0ubuntu1'
         new.return_value.pocket = 'Proposed'
@@ -118,6 +118,20 @@
         self.assertEqual(build.check_status(), 'Proposed pocket')
 
     @patch('cupstream2distro.archive.newest')
+    def test_archive_pocket_mismatch(self, new):
+        """Correctly identify when dest has same version with diff contents."""
+        results = [{'hash'}, {'mismatch'}]
+        new.return_value.source_package_version = '42-0ubuntu1'
+        new.return_value.pocket = 'Proposed'
+        new.return_value.status = 'Pending'
+        build = BuildBase('state', self.series, self.dest, self.ppa)
+        build.get_package_version = Mock(return_value='42-0ubuntu1')
+        build.archive_id = Mock(
+            side_effect=lambda src: results.pop())
+        self.assertEqual(
+            build.check_status(), 'Needs rebuild due to burned version number')
+
+    @patch('cupstream2distro.archive.newest')
     def test_check_status_queue(self, new):
         """Correctly identify when PPA has wrong version."""
         sources = {