Merge ~apw/ubuntu-archive-tools:new-review--add-build-analysis2 into ubuntu-archive-tools:main

[Andy Whitcroft]

new-review: add support for dumping build related information

Use the build logs to identify the applied archive pockets at build time. Also use the logs to reconstruct the binary control files for the new and old packages and produce a delta.

diff --git a/new-review b/new-review
index 5a7ddd5..521df2f 100755
--- a/new-review
+++ b/new-review
@@ -66,7 +66,8 @@ class NewReview:
         signing_request = ReviewRequest.select(args, services=self.svcs)
         if signing_request.task_assign(args):
             signing_request.analyse(args)
-            signing_request.binary_new(args)
+            if not args.approve:
+                signing_request.binary_new(args)
 
             signing_bot = SigningBotSubmit()
             signing_bot.submit(args, signing_request)
@@ -102,6 +103,7 @@ class NewReview:
         group.add_argument("--from-suite", dest="from_suite")
         group.add_argument("--to", dest="to_route")
         group.add_argument("--to-suite", dest="to_suite")
+        group.add_argument("--existing-suite", dest="existing_suite")
         group.add_argument("--embargo", action="store_true")
         group = subcmd.add_argument_group(title='type/package options')
         group.add_argument("--force-type")
diff --git a/new_review.py b/new_review.py
index b47bedf..23c82ad 100755
--- a/new_review.py
+++ b/new_review.py
@@ -8,6 +8,7 @@ import hashlib
 import json
 import os
 import re
+import shlex
 import sys
 from argparse import ArgumentParser
 from copy import copy
@@ -81,6 +82,7 @@ class Package:
         self.signing = signing
 
         self.series, self.pocket = Package.suite2series_pocket(self.suite)
+        self.order = order
         self.package_key = (order, name)
 
     @classmethod
@@ -274,9 +276,10 @@ class ReviewRequest:
     def analyse(self, args):
         logger.info("Promotion Review:")
 
-    def signing_reference_suite(self, reference, suite):
+    def signing_reference_suite(self, reference, suite, via="unknown"):
         self.signing_ref = reference
         self.signing_suite = suite
+        self.signing_via = via
 
     def proposed_reference_suite(self, reference, suite):
         self.proposed_ref = reference
@@ -406,6 +409,63 @@ class ReviewRequest:
 
         return binaries
 
+    def build_analyse(self, build):
+        lp = self.svcs.launchpad
+
+        abi_re = re.compile(r"^([0-9]+\.[0-9]+\.[0-9]+)[\.-]([0-9]+)")
+
+        version = build.source_package_version
+        match = abi_re.search(version)
+        if match:
+            abi = match.group(1) + "-" + match.group(2)
+        else:
+            abi = None
+
+        log_url = build.build_log_url
+        if log_url is None:
+            logger.warn("{} build log not available".format(build.arch_tag))
+            return None, None
+        log_url = log_url.replace('https://launchpad.net/', 'https://api.launchpad.net/devel/')
+        log = lp._browser.get(log_url)
+        control = []
+        in_package_contents = 0
+        in_package_control = False
+        for line in log.decode('utf-8', 'backslashreplace').rstrip().split('\n'):
+            if line.startswith("RUN: /usr/share/launchpad-buildd/bin/in-target override-sources-list"):
+                pockets = set()
+                bits = shlex.split(line)
+                for bit in bits:
+                    if bit.startswith("deb http://ftpmaster.internal/ubuntu "):
+                        suite = bit.split()[2]
+                        if "-" in suite:
+                            pocket = suite.split("-")[-1]
+                        else:
+                            pocket = "release"
+                        pockets.add(pocket)
+
+            elif line.startswith("| Package contents"):
+                in_package_contents = 2
+
+            elif line.startswith("+-----------------"):
+                in_package_contents -= 1
+
+            if in_package_contents > 0:
+                if line.startswith(" Package:"):
+                    in_package_control = True
+                elif line.startswith(" Installed-Size:"):
+                    continue
+                elif len(line) == 0:
+                    if in_package_control:
+                        control.append("")
+                    in_package_control = False
+                if in_package_control:
+                    line = line.replace(version, "VERSION")
+                    if abi is not None:
+                        line = line.replace(abi, "ABI")
+                    control.append(line)
+
+        return pockets, control
+
     def binary_new(self, args, against="updates"):
         logger.info("New Review:")
         existing_group = PackageGroup("control")
@@ -413,6 +473,8 @@ class ReviewRequest:
             if pocket_ref is None:
                 continue
             some = False
+            if args.existing_suite is not None:
+                pocket_suite = args.existing_suite
             for package in self.package_group.packages:
                 if package.generate:
                     continue
@@ -433,6 +495,7 @@ class ReviewRequest:
                         src,
                         signing=package.signing,
                         main=package.main,
+                        order=package.order,
                     )
                 )
             if some:
@@ -443,6 +506,7 @@ class ReviewRequest:
         else:
             logger.info("  No binaries found in updates/release, New source packages?")
 
+        logger.info("  published-binaries:")
         old_bins = self.binary_list(existing_group, args, ignore_failures=True)
         new_bins = self.binary_list(self.package_group, args)
         if old_bins is not None and new_bins is not None:
@@ -452,17 +516,60 @@ class ReviewRequest:
                 if line.rstrip() in ("+++", "---"):
                     continue
                 some = True
-                logger.info("  " + line)
+                logger.info("    " + line)
             if not some:
-                logger.info("  No Changes ({} binaries)".format(len(new_bins)))
+                logger.info("    No Changes ({} binaries)".format(len(new_bins)))
 
         elif self.have_autonew:
-            logger.info("  binary builds not complete but automated new-review available")
+            logger.info("    binary builds not complete but automated new-review available")
             self.need_autonew = True
 
         else:
             raise ReviewError("Binary Builds incomplete, cannot review")
 
+        logger.info("  built-against:")
+        against = None
+        against_detail = []
+        against_dump = False
+        new_control = []
+        for package in self.package_group.packages:
+            for build in package.spph.getBuilds():
+                pockets, control = self.build_analyse(build)
+                if pockets is not None and not package.generate:
+                    if against is None:
+                        against = pockets
+                    if against != pockets:
+                        against_dump = True
+                    against_detail.append("    {}/{} ubuntu/primary {}".format(package.name, build.arch_tag, ",".join(sorted(pockets))))
+                    new_control.append("=== {}/{} ===".format(package.name, build.arch_tag))
+                    new_control += control
+
+        if against_dump is True:
+            for against in against_detail:
+                logger.info(against)
+        else:
+            logger.info("    all ubuntu/primary {}".format(",".join(sorted(against))))
+
+        if old_bins is not None and new_bins is not None:
+            logger.info("  control-delta:")
+            old_control = []
+            for package in existing_group.packages:
+                for build in package.spph.getBuilds():
+                    pockets, control = self.build_analyse(build)
+                    if pockets is not None:
+                        old_control.append("=== {}/{} ===".format(package.name, build.arch_tag))
+                        old_control += control
+
+            delta = unified_diff(old_control, new_control, lineterm="")
+            some = False
+            for line in delta:
+                if line.rstrip() in ("+++", "---"):
+                    continue
+                some = True
+                logger.info("    " + line)
+            if not some:
+                logger.info("    No Changes")
+
     def lp_find_tracker_task(self, tracker_id, name):
         try:
             lp_tracker = self.svcs.launchpad.bugs[tracker_id]
@@ -586,113 +693,136 @@ class ReviewRequestKernel(ReviewRequest):
             if package.main:
                 ucs_srcs.append(package.spph)
 
-        if (
-            self.signing_present
-            and self.signing_ref is not None
-            and self.signing_ref.startswith("signing:")
-        ):
+        if self.signing_present:
             logger.info("Signing Review:")
-            logger.info("  key {} selected".format(self.signing_ref[8:]))
-
-            # VALIDATE signing routing.
-            compatibility = []
-            if len(ucs_srcs) == 0:
-                # Go find main...
-                main_version_re = re.compile("(.*)\+[0-9]")
-                main_name = self.ks_source.lookup_package(type="main").name
-                for package in self.package_group.packages:
-                    if not package.signing:
-                        continue
-                    match = main_version_re.match(package.version)
-                    if match:
-                        main_version = match.group(1)
-                    else:
-                        main_version = package_version
-
-                    for ucs_ref, ucs_pocket in self.proposed_routes:
-                        lp_archive = lp.archives.getByReference(reference=ucs_ref)
-                        if lp_archive is None:
-                            raise ReviewError("{}: archive '{}' not found".format(tracker_id, ucs_ref))
-                        srcs = lp_archive.getPublishedSources(exact_match=True, order_by_date=True, distro_series='/ubuntu/' + self.series, source_name=main_name, version=main_version)
-                        if len(srcs) == 0:
+            if (
+                self.signing_ref is not None
+                and self.signing_ref.startswith("signing:")
+            ):
+                signing_config = self.svcs.signing_config
+                signing_descriptor = signing_config.lookup_stream(self.signing_ref[8:])
+                signing_reference = signing_descriptor.archive_reference
+
+                # VALIDATE signing routing.
+                compatibility = []
+                if len(ucs_srcs) == 0:
+                    # Go find main...
+                    main_version_re = re.compile("(.*)\+[0-9]")
+                    main_name = self.ks_source.lookup_package(type="main").name
+                    for package in self.package_group.packages:
+                        if not package.signing:
                             continue
-                        src = srcs[0]
-                        logger.info("  using {}:{} {} (main package) from {} for Ubuntu-Signing-Compatible".format(self.series, main_name, main_version, ucs_ref))
-                        ucs_srcs.append(src)
-                        break
-            for package_spph in ucs_srcs:
-                changes_url = package_spph.changesFileUrl()
-                if changes_url is None:
-                    raise ReviewError("{}: no changes file for {} not found".format(args.tracker, ks_package.type or "main"))
-
-                # If we managed to find a changes file then we can extract the list.
-                changes_url = changes_url.replace('https://launchpad.net/', 'https://api.launchpad.net/devel/')
-                changes = lp._browser.get(changes_url)
-                compatibility = None
-                for line in changes.decode('utf-8').rstrip().split('\n'):
-                    if line.startswith("Ubuntu-Compatible-Signing:"):
-                        compatibility = line.split()[1:]
-                logger.info("    Ubuntu-Compatible-Signing {} source".format(signing_compat(compatibility)))
-
-                # If there are builds available check they are matching compatibility.
-                compatibility_match = True
-                for build in package_spph.getBuilds():
-                    # If we managed to find a changes file then we can extract the list.
-                    changes_url = build.changesfile_url
+                        match = main_version_re.match(package.version)
+                        if match:
+                            main_version = match.group(1)
+                        else:
+                            main_version = package_version
+
+                        for ucs_ref, ucs_pocket in self.proposed_routes:
+                            lp_archive = lp.archives.getByReference(reference=ucs_ref)
+                            if lp_archive is None:
+                                raise ReviewError("{}: archive '{}' not found".format(tracker_id, ucs_ref))
+                            srcs = lp_archive.getPublishedSources(exact_match=True, order_by_date=True, distro_series='/ubuntu/' + self.series, source_name=main_name, version=main_version)
+                            if len(srcs) == 0:
+                                continue
+                            src = srcs[0]
+                            logger.info("  using {}:{} {} (main package) from {} for Ubuntu-Signing-Compatible".format(self.series, main_name, main_version, ucs_ref))
+                            ucs_srcs.append(src)
+                            break
+                for package_spph in ucs_srcs:
+                    changes_url = package_spph.changesFileUrl()
                     if changes_url is None:
-                        logger.warn("Ubuntu-Compatible-Signing - {} build not available".format(build.arch_tag))
-                        continue
-                    changes_url = build.changesfile_url.replace('https://launchpad.net/', 'https://api.launchpad.net/devel/')
+                        raise ReviewError("{}: no changes file for {} not found".format(args.tracker, ks_package.type or "main"))
+
+                    # If we managed to find a changes file then we can extract the list.
+                    changes_url = changes_url.replace('https://launchpad.net/', 'https://api.launchpad.net/devel/')
                     changes = lp._browser.get(changes_url)
-                    build_compat = None
+                    compatibility = None
                     for line in changes.decode('utf-8').rstrip().split('\n'):
                         if line.startswith("Ubuntu-Compatible-Signing:"):
-                            build_compat = line.split()[1:]
-                            if compatibility != build_compat:
-                                compatibility_match = False
-                    logger.info("    Ubuntu-Compatible-Signing {} {}".format(signing_compat(build_compat), build.arch_tag))
-
-                # Report compatibility error.
-                if not compatibility_match:
-                    raise ReviewError("compatibility missmatch".format(ks_package.type or "main"))
-
-            signing_config = self.svcs.signing_config
-            signing_descriptor = signing_config.lookup_stream(self.signing_ref[8:])
-
-            if compatibility is None:
-                compatibility = ["ubuntu/2", "pro/2", "kariya/2"]
-                logger.info("    Ubuntu-Compatible-Signing implied as {}".format(signing_compat(compatibility)))
-
-            compatible = None
-            for stream_level in compatibility:
-                compatible_descriptor = signing_config.lookup_stream(stream_level)
-                if signing_descriptor.stream != compatible_descriptor.stream:
-                    continue
-                compatible = signing_descriptor.level == compatible_descriptor.level
-                break
+                            compatibility = line.split()[1:]
+                    ucs_detail = [(compatibility, "source")]
+
+                    # If there are builds available check they are matching compatibility.
+                    compatibility_match = True
+                    for build in package_spph.getBuilds():
+                        # If we managed to find a changes file then we can extract the list.
+                        changes_url = build.changesfile_url
+                        if changes_url is None:
+                            logger.warn("Ubuntu-Compatible-Signing - {} build not available".format(build.arch_tag))
+                            continue
+                        changes_url = build.changesfile_url.replace('https://launchpad.net/', 'https://api.launchpad.net/devel/')
+                        changes = lp._browser.get(changes_url)
+                        build_compat = None
+                        for line in changes.decode('utf-8').rstrip().split('\n'):
+                            if line.startswith("Ubuntu-Compatible-Signing:"):
+                                build_compat = line.split()[1:]
+                                if compatibility != build_compat:
+                                    compatibility_match = False
+                        ucs_detail.append((build_compat, build.arch_tag))
+
+                    # Report compatibility error.
+                    logger.info("  Ubuntu-Compatible-Signing:")
+                    if not compatibility_match:
+                        for detail_compatibility, detail_thing in ucs_detail:
+                            logger.info("    {} {}".format(detail_thing, signing_compat(detail_compatibility)))
+                        raise ReviewError("compatibility missmatch".format(ks_package.type or "main"))
+                    logger.info("    all {}".format(signing_compat(compatibility)))
+
+                if compatibility is None:
+                    compatibility = ["ubuntu/2", "pro/2", "kariya/2"]
+                    logger.info("    Ubuntu-Compatible-Signing implied as {}".format(signing_compat(compatibility)))
+
+                compatible = None
+                for stream_level in compatibility:
+                    compatible_descriptor = signing_config.lookup_stream(stream_level)
+                    if signing_descriptor.stream != compatible_descriptor.stream:
+                        continue
+                    compatible = signing_descriptor.level == compatible_descriptor.level
+                    break
+
+                # XXX: this really permits us to use the wrong keys, it would be better
+                # if everything used like ibm-gt/1 but ...
+                compatibility.append("*/1")
+                #print("APW", signing_descriptor.level, type(signing_descriptor.level))
+                if compatible is None or signing_descriptor.level == 1:
+                    compatible = 1
+
+                #print(compatibility, signing_descriptor.stream, signing_descriptor.level, compatible)
+                if not compatible:
+                    raise ReviewError("{}: signing-level {}/{} not compatible with package {}".format(args.tracker, signing_descriptor.stream, signing_descriptor.level, " ".join(compatibility)))
+                logger.info("  key {}:".format(self.signing_ref[8:]))
+                logger.info("    selected via {}".format(self.signing_via))
+                logger.info("    compatible with package {}".format(" ".join(compatibility)))
 
-            # XXX: this really permits us to use the wrong keys, it would be better
-            # if everything used like ibm-gt/1 but ...
-            compatibility.append("*/1")
-            #print("APW", signing_descriptor.level, type(signing_descriptor.level))
-            if compatible is None or signing_descriptor.level == 1:
-                compatible = 1
+            else:
+                signing_reference = self.signing_ref
 
-            #print(compatibility, signing_descriptor.stream, signing_descriptor.level, compatible)
-            if not compatible:
-                raise ReviewError("{}: signing-level {}/{} not compatible with package {}".format(args.tracker, signing_descriptor.stream, signing_descriptor.level, " ".join(compatibility)))
-            logger.info("  key {}/{} deemed compatible with package {}".format(signing_descriptor.stream, signing_descriptor.level, " ".join(compatibility)))
+            logger.info("  signing-ppa {}".format(signing_reference))
 
-        elif self.signing_present:
-            logger.info("Signing Review:")
-            logger.info("  using signing PPA {}".format(self.signing_ref))
+            some = False
+            ptypes = set()
+            for package in self.package_group.packages:
+                ptypes.add(package.type or "main")
+            for package in self.package_group.packages:
+                ptype = package.type or "main"
+                signing_from = self.ks_source.lookup_package(type=ptype).signing_from
+                if signing_from is None:
+                    continue
+                if not some:
+                    some = True
+                    logger.info("  pairings:")
+                sf_ptype = signing_from.type or "main"
+                logger.info("    {} with {}".format(ptype, sf_ptype))
+                if sf_ptype not in ptypes:
+                    raise ReviewError("{}: {} in request without {}, request inconsistent ...".format(tracker_id, ptype, sf_ptype))
 
     def set_routing(self, args, ks_routing, from_route_name, from_route_entry):
         if self.signing_present:
             signing_routes = ks_routing.lookup_route('signing')
             if len(signing_routes) > 0:
                 signing_route = signing_routes[0]
-                self.signing_reference_suite(signing_route.raw_reference, signing_route.suite)
+                self.signing_reference_suite(signing_route.raw_reference, signing_route.suite, via="kernel-series")
 
         if args.to_route is not None:
             to_route_name, to_route_entry = self.route_split(args.to_route_name)
@@ -811,22 +941,6 @@ class ReviewRequestKernelTracker(ReviewRequestKernel):
         if tb_promote is None:
             raise ReviewError("{}: {} promotion data not present".format(tracker_id, task))
 
-        # Sanity check ...
-        # XXX: this should be done in validate based on the package names and types, so that
-        # it applied equally to handles and could then be extended to Packages.
-        some = False
-        for ptype in tb_promote:
-            signing_from = ks_source.lookup_package(type=ptype).signing_from
-            if signing_from is None:
-                continue
-            if not some:
-                some = True
-                logger.info("  pairings:")
-            sf_ptype = signing_from.type or "main"
-            logger.info("    {} with {}".format(ptype, sf_ptype))
-            if sf_ptype not in tb_promote:
-                raise ReviewError("{}: {} in request without {}, request inconsistent ...".format(tracker_id, ptype, sf_ptype))
-
         packages = PackageGroup()
         for package_type in tb_promote:
             package_name = tb_packages.get(package_type)
@@ -859,6 +973,7 @@ class ReviewRequestKernelTracker(ReviewRequestKernel):
                     generate=ks_package.signing_to is not None,
                     signing=ks_package.signing_from is not None,
                     main=package_type == "main",
+                    type=package_type,
                     order=self.ks_package_order(ks_package),
                 )
             )
@@ -953,6 +1068,7 @@ class ReviewRequestKernelHandle(ReviewRequestKernel):
                     generate=ks_package.signing_to is not None,
                     signing=ks_package.signing_from is not None,
                     main=package_type == "main",
+                    type=package_type,
                     order=self.ks_package_order(ks_package),
                 )
             )
@@ -1115,7 +1231,7 @@ class ReviewRequestPackages(ReviewRequest):
             logger.info("  key is not driver selected key {}".format(default_key))
 
         signing_ref = "signing:" + key
-        self.signing_reference_suite(signing_ref, Package.series_pocket2suite(series_name, "Release"))
+        self.signing_reference_suite(signing_ref, Package.series_pocket2suite(series_name, "Release"), via=key_source)
 
         proposed_ref, proposed_suite = args.to_route, args.to_suite or series_name
         if proposed_ref is None: