Merge ubuntu-archive-tools:sru-review-sync-diffs into ubuntu-archive-tools:main

[Łukasz Zemczak]

sru-review: Support sync uploads in the queue by generating their debdiffs locally

This is an initial try on getting proper support for SRU reviewing sync uploads. Sync uploads are troublesome as they generally come with no debdiffs for reviewing. Everytime this happens, SRU members need to manually fetch and debdiff those packages.

This MP automates that process. Basically it pulls the sources for both the new upload and the latest upload in the target archive/series and does a dumb debdiff on those. We could probably optimize this a bit, but that's a good working start point for sure.

[Steve Langasek (vorlon)]

review: Needs Fixing

[Łukasz Zemczak (sil2100)]

Thank you for the review, Steve!

Ok, fixed the typo + a whitespace thing. As for your inline comment about directory cleanup: no manual cleanup should be needed as the packages are extracted into a temporary directory created using the TemporaryDirectory() context manager. This should ensure that when we go out of context (or the tool closes early), the temporary directory is auto-removed with all its contents. So cleanup is automatic!

[Steve Langasek (vorlon)]

review: Approve

diff --git a/sru-review b/sru-review
index e9e1493..0f55d5d 100755
--- a/sru-review
+++ b/sru-review
@@ -122,10 +122,27 @@ def parse_changes(changes_url):
     return info
 
 
+def fetch_source(pub, dest_dir):
+    '''Downloads source files of a given source publishing.
+
+    Return path to the downloaded source's .dsc file.'''
+    sources = pub.sourceFileUrls()
+
+    dsc_file = None
+    for src in sources:
+        basename = os.path.basename(src)
+        dst = os.path.join(dest_dir, basename)
+        if dst.endswith('.dsc'):
+            dsc_file = dst
+        urlretrieve(src, dst)
+
+    return dsc_file
+
+
 def from_queue(options, archive, sourcepkg, series, version=None):
     '''Get package_upload from LP and debdiff from queue page.
 
-    Return (package_upload, changes URL, debdiff URL) tuple.
+    Return (package_upload, changes URL, debdiff contents) tuple.
     '''
     queues = {'New': 0, 'Unapproved': 1, 'Rejected': 4}
     queue = options.queue.title()
@@ -147,55 +164,87 @@ def from_queue(options, archive, sourcepkg, series, version=None):
     upload = uploads[0]
 
     if upload.contains_copy:
-        archive = upload.copy_source_archive
-        pubs = archive.getPublishedSources(
+        source_archive = upload.copy_source_archive
+        pubs = source_archive.getPublishedSources(
             exact_match=True, source_name=upload.package_name,
             version=upload.package_version)
         if pubs:
             changes_url = pubs[0].changesFileUrl()
         else:
             print("ERROR: Can't find source package %s %s in %s" %
-                  (upload.package_name, upload.package_version,
+                  (source_archive.package_name, upload.package_version,
                    archive.web_link),
                   file=sys.stderr)
             sys.exit(1)
     else:
         changes_url = upload.changes_file_url
 
+    debdiff = None
     if options.diff:
-        oops_re = re.compile('class="oopsid">(OOPS[a-zA-Z0-9-]+)<')
-        if version:
-            re_version = re.escape(version)
+        if upload.contains_copy:
+            # The upload is a sync - we need to create the debdiff ourselves
+            print('This upload is a sync, no autogenerated debdiff present.',
+                  file=sys.stderr)
+            print('''Do you want to generate the debdiff locally? [Yn]''',
+                  end="")
+            sys.stdout.flush()
+            response = sys.stdin.readline()
+            if not response.strip().lower().startswith('n'):
+                with tempfile.TemporaryDirectory() as tempdir:
+                    source_archive = upload.copy_source_archive
+                    pubs = source_archive.getPublishedSources(
+                        exact_match=True, source_name=upload.package_name,
+                        version=upload.package_version)
+                    sru_dsc_file = fetch_source(pubs[0], tempdir)
+
+                    pubs = archive.getPublishedSources(
+                        exact_match=True, source_name=upload.package_name,
+                        order_by_date=True, status='Published',
+                        distro_series=series)
+                    cur_dsc_file = fetch_source(pubs[0], tempdir)
+
+                    ret = subprocess.run(
+                        ["debdiff", cur_dsc_file, sru_dsc_file],
+                        stdout=subprocess.PIPE,
+                        stderr=subprocess.DEVNULL)
+
+                    # ...not a filelike object, but should work
+                    debdiff = ret.stdout.splitlines(keepends=True)
         else:
-            re_version = '[^_"]+'
-        debdiff_re = re.compile(
-            'href="(http://launchpadlibrarian.net/'
-            '\d+/%s_[^"_]+_%s\.diff\.gz)">\s*diff from' %
-            (re.escape(sourcepkg), re_version))
-
-        queue_html = urlopen(queue_url).read().decode('utf-8')
+            oops_re = re.compile('class="oopsid">(OOPS[a-zA-Z0-9-]+)<')
+            if version:
+                re_version = re.escape(version)
+            else:
+                re_version = '[^_"]+'
+            debdiff_re = re.compile(
+                'href="(http://launchpadlibrarian.net/'
+                '\d+/%s_[^"_]+_%s\.diff\.gz)">\s*diff from' %
+                (re.escape(sourcepkg), re_version))
+
+            queue_html = urlopen(queue_url).read().decode('utf-8')
+
+            m = oops_re.search(queue_html)
+            if m:
+                print('ERROR: Launchpad failure:', m.group(1), file=sys.stderr)
+                sys.exit(1)
 
-        m = oops_re.search(queue_html)
-        if m:
-            print('ERROR: Launchpad failure:', m.group(1), file=sys.stderr)
-            sys.exit(1)
+            m = debdiff_re.search(queue_html)
+            if not m:
+                print('ERROR: queue does not have a debdiff', file=sys.stderr)
+                sys.exit(1)
+            debdiff_url = m.group(1)
+            #print('debdiff URL:', debdiff_url, file=sys.stderr)
 
-        m = debdiff_re.search(queue_html)
-        if not m:
-            print('ERROR: queue does not have a debdiff', file=sys.stderr)
-            sys.exit(1)
-        debdiff_url = m.group(1)
-        #print('debdiff URL:', debdiff_url, file=sys.stderr)
-    else:
-        debdiff_url = None
+            with tempfile.NamedTemporaryFile() as f:
+                debdiff = gzip.open(urlretrieve(debdiff_url, f.name)[0])
 
-    return (upload, changes_url, debdiff_url)
+    return (upload, changes_url, debdiff)
 
 
 def from_ppa(options, sourcepkg, user, ppaname):
     '''Get .changes and debdiff from a PPA.
 
-    Return (changes URL, debdiff URL) pair.
+    Return (changes URL, debdiff contents) pair.
     '''
     changes_re = re.compile(
         'href="(https://launchpad.net/[^ "]+/%s_[^"]+_source.changes)"' %
@@ -243,11 +292,12 @@ def from_ppa(options, sourcepkg, user, ppaname):
     #        print('ERROR: PPA does not have a debdiff', file=sys.stderr)
     #        sys.exit(1)
     #    debdiff_url = m.group(1)
+    #    # TODO: we need to download the debdiff now
     #    #print('debdiff URL:', debdiff_url, file=sys.stderr)
     #else:
-    debdiff_url = None
+    debdiff = None
 
-    return (changes_url, debdiff_url)
+    return (changes_url, debdiff)
 
 
 def reject_comment(launchpad, num, package, release, reason):
@@ -284,9 +334,9 @@ if __name__ == '__main__':
 
     if opts.ppa:
         (user, ppaname) = opts.ppa.split('/', 1)
-        (changes_url, debdiff_url) = from_ppa(opts, sourcepkg, user, ppaname)
+        (changes_url, debdiff) = from_ppa(opts, sourcepkg, user, ppaname)
     else:
-        (upload, changes_url, debdiff_url) = from_queue(
+        (upload, changes_url, debdiff) = from_queue(
             opts, archive, sourcepkg, series, version)
 
     # Check for existing version in proposed
@@ -322,11 +372,7 @@ if __name__ == '__main__':
                     print('''Exiting''')
                     sys.exit(1)
 
-    debdiff = None
-    if debdiff_url:
-        with tempfile.NamedTemporaryFile() as f:
-            debdiff = gzip.open(urlretrieve(debdiff_url, f.name)[0])
-    elif opts.diff:
+    if opts.diff and not debdiff:
         print('No debdiff available')
 
     # parse changes and open bugs first since we are using subprocess
@@ -365,6 +411,7 @@ if __name__ == '__main__':
             for line in debdiff:
                 tfile.write(line)
             tfile.flush()
+            debdiff = None
             if opts.diffstat:
                 combinedfile = resources.enter_context(
                     tempfile.NamedTemporaryFile())