Merge lp:~3v1n0/ubuntu-archive-tools/sru-review-bileto-support into lp:ubuntu-archive-tools

[Marco Trevisan (Treviño)]

The long awaited feature from the SRU team having to deal with bileto sync's is there... :)

[lp:~3v1n0/ubuntu-archive-tools/sru-review-bileto-support]

1210. By Marco Trevisan (Treviño) on 2019-03-08

sur-review: Add support to show diffs coming from bileto PPAs

When handling a source upload that is a sync and is coming from a ppa owned by
~ci-train-ppa-service, we can get the debdiff directly from the ppa description, as
bileto generates it.

So, parse such text looking for the proper uri and support both plain-text and gzipped
diff files when showing them.

[Andy Whitcroft (apw)]

Overall this look ok to me. Some nits in-line.

review: Approve

[lp:~3v1n0/ubuntu-archive-tools/sru-review-bileto-support]

1211. By Marco Trevisan (Treviño) on 2019-12-19

sru-review: Make Diff easier to read

1212. By Marco Trevisan (Treviño) on 2019-12-19

sru-review: Figure out if we have to gunzip a diff from its URI

[Marco Trevisan (Treviño) (3v1n0)]

Sorry for the delay, I forgot about the review :(

Anyways, I've addressed the required changes I hope it's fine.

[lp:~3v1n0/ubuntu-archive-tools/sru-review-bileto-support]

1213. By Marco Trevisan (Treviño) on 2019-12-19

sru-review: Figure out if we have to gunzip a diff from its URI

1214. By Marco Trevisan (Treviño) on 2020-06-05

Merge with trunk

1215. By Marco Trevisan (Treviño) on 2020-06-05

sru-review: Properly handle diff uris for re-created PPAs

In these cases the PPA is named something like bileto-NNN.R where R is the
rebuild number (normally 1).

The bileto name however, keeps being bileto-NNNN and so the regex should be
adapted accordingly

[Brian Murray (brian-murray)]

This is great thanks for working on this and updating it. One thing I noticed today when testing it was that bileto's "diff" includes the output of diffstat (e.g. https://objectstorage.prodstack4-5.canonical.com/v1/AUTH_39a8dbb93caf4ec889f8a1b7f69885db/bileto-4610/2021-07-09_10:06:30/focal_xtables-addons_content.diff) and then sru-review can add additional diffstat. This seemed rather redundant to me so I played with the code a bit and made the following change:

--- old/sru-review 2020-06-05 13:56:50 +0000
+++ new/sru-review 2021-07-13 23:18:49 +0000
@@ -43,6 +43,7 @@
 import webbrowser

 from contextlib import ExitStack
+from itertools import dropwhile
 from launchpadlib.launchpad import Launchpad
 from lazr.restfulclient.errors import ServerError
 from sru_workflow import process_bug
@@ -364,8 +365,11 @@
     if debdiff and opts.view:
         with ExitStack() as resources:
             tfile = resources.enter_context(tempfile.NamedTemporaryFile())
- for line in debdiff:
- tfile.write(line)
+ while True:
+ for line in dropwhile(lambda x:not x.decode('utf-8').startswith('diff '), debdiff):
+ tfile.write(line)
+ else:
+ break
             tfile.flush()
             if opts.diffstat:
                 combinedfile = resources.enter_context(

Could you add that to your changes? Then I'll harass somebody who has commit access to this to get it merged. Thanks again!

review: Needs Fixing

[lp:~3v1n0/ubuntu-archive-tools/sru-review-bileto-support]

1216. By Marco Trevisan (Treviño) on 2021-07-14

Merge with trunk

1217. By Brian Murray on 2021-07-14

sru-review: Remove the bileto redundant diffstat

[Marco Trevisan (Treviño) (3v1n0)]

Ok, thanks... Added!

[Brian Murray (brian-murray)]

review: Approve

[Łukasz Zemczak (sil2100)]

review: Approve

=== modified file 'sru-review'
--- sru-review	2021-04-23 08:44:48 +0000
+++ sru-review	2021-07-14 01:13:21 +0000
@@ -35,13 +35,15 @@
 import sys
 import tempfile
 try:
-    from urllib.parse import quote
     from urllib.request import urlopen, urlretrieve
+    from urllib.parse import quote, urlparse
 except ImportError:
     from urllib import urlopen, urlretrieve, quote
+    from urlparse import urlparse
 import webbrowser
 
 from contextlib import ExitStack
+from itertools import dropwhile
 from launchpadlib.launchpad import Launchpad
 from lazr.restfulclient.errors import ServerError
 from sru_workflow import process_bug, check_additional_series
@@ -142,8 +144,11 @@
         sys.exit(1)
     upload = uploads[0]
 
+    is_bileto = False
     if upload.contains_copy:
         archive = upload.copy_source_archive
+        is_bileto = archive.owner.name == 'ci-train-ppa-service' and \
+                    upload.display_arches == 'sync'
         pubs = archive.getPublishedSources(
             exact_match=True, source_name=upload.package_name,
             version=upload.package_version)
@@ -159,20 +164,30 @@
         changes_url = upload.changes_file_url
 
     if options.diff:
-        oops_re = re.compile('class="oopsid">(OOPS[a-zA-Z0-9-]+)<')
-        debdiff_re = re.compile(
-            'href="(http://launchpadlibrarian.net/'
-            '\d+/%s_[^"_]+_[^_"]+\.diff\.gz)">\s*diff from' %
-            re.escape(sourcepkg))
-
-        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 = debdiff_re.search(queue_html)
+        if is_bileto:
+            archive = upload.copy_source_archive
+            debdiff_page = archive.description
+            debdiff_re = re.compile(
+                '(https:\/\/objectstorage\.[-\w\d_.]+\.canonical\.com\/.*\/'
+                'bileto-%s\/[-\d_:]+\/%s_%s_content\.diff)' %
+                    (archive.name.split('.')[0],
+                     re.escape(options.release),
+                     re.escape(sourcepkg)))
+        else:
+            oops_re = re.compile('class="oopsid">(OOPS[a-zA-Z0-9-]+)<')
+            debdiff_re = re.compile(
+                'href="(http://launchpadlibrarian.net/'
+                '\d+/%s_[^"_]+_[^_"]+\.diff\.gz)">\s*diff from' %
+                re.escape(sourcepkg))
+
+            debdiff_page = urlopen(queue_url).read().decode('utf-8')
+
+            m = oops_re.search(debdiff_page)
+            if m:
+                print('ERROR: Launchpad failure:', m.group(1), file=sys.stderr)
+                sys.exit(1)
+
+        m = debdiff_re.search(debdiff_page)
         if not m:
             print('ERROR: queue does not have a debdiff', file=sys.stderr)
             sys.exit(1)
@@ -312,7 +327,11 @@
     debdiff = None
     if debdiff_url:
         with tempfile.NamedTemporaryFile() as f:
-            debdiff = gzip.open(urlretrieve(debdiff_url, f.name)[0])
+            urlretrieve(debdiff_url, f.name)
+            if urlparse(debdiff_url).path.endswith('.gz'):
+                debdiff = gzip.open(f.name)
+            else:
+                debdiff = open(f.name, 'rb')
     elif opts.diff:
         print('No debdiff available')
 
@@ -346,8 +365,11 @@
     if debdiff and opts.view:
         with ExitStack() as resources:
             tfile = resources.enter_context(tempfile.NamedTemporaryFile())
-            for line in debdiff:
-                tfile.write(line)
+            while True:
+                for line in dropwhile(lambda x:not x.decode('utf-8').startswith('diff '), debdiff):
+                    tfile.write(line)
+                else:
+                    break
             tfile.flush()
             if opts.diffstat:
                 combinedfile = resources.enter_context(