Merge lp:~sil2100/ubuntu-archive-tools/common-sru-process-bug into lp:ubuntu-archive-tools

[Łukasz Zemczak]

Extract the process_bug() function to a common module so that both sru-review and sru-accept use the same mechanics for updating SRU bugs after acceptance. This also slightly changes the behavior of sru-accept for more than one series.

Extract the process_bug() function to a common module so that both sru-review and sru-accept use the same mechanics for updating SRU bugs after acceptance. This also slightly changes the behavior of sru-accept for more than one series.

Currently we had two versions of process_bug and the sru-accept one was outdated. Having to maintain two of those was crazy.

Also, with this change, sru-accept when called with multiple targets now just runs process_bug() multiple times (e.g. adding one comment per series) instead of once with multiple series in the bug comment. I changed this because it was easier to do and mimics what happens when you use the sru-review tool.

[Brian Murray (brian-murray)]

This looks good to me thanks for working on it - I think a docstring in sru_workflow.py would be good though. Additionally, a change of mine was recently merged to ubuntu-archive-tools which changed the content of the comment added to bug reports requesting feed back (lines 121-132) so please be sure not to loose that.

Thanks!

review: Approve

[lp:~sil2100/ubuntu-archive-tools/common-sru-process-bug]

1127. By Łukasz Zemczak on 2017-12-07

Merge trunk, resolve conflicts.

1128. By Łukasz Zemczak on 2017-12-07

Dumb docstring for the sru_workflow.py file.

=== modified file 'sru-accept'
--- sru-accept	2016-06-07 22:36:28 +0000
+++ sru-accept	2017-12-07 16:28:26 +0000
@@ -24,126 +24,11 @@
 
 import launchpadlib.errors
 from launchpadlib.launchpad import Launchpad
+from sru_workflow import process_bug
 
 
 CONSUMER_KEY = "sru-accept"
 
-bug_target_re = re.compile(
-    r'/ubuntu/(?:(?P<suite>[^/]+)/)?\+source/(?P<source>[^/]+)$')
-
-
-def join_english_list(words):
-    num_words = len(words)
-    assert num_words > 0
-    if num_words > 2:
-        # Yes, I use the Oxford comma.  Deal with it.
-        return '%s, and %s' % (', '.join(words[:-1]), words[-1])
-    elif num_words == 2:
-        return '%s and %s' % (words[0], words[1])
-    else:
-        return words[0]
-
-
-def process_bug(launchpad, options, num):
-    bug = launchpad.bugs[num]
-    sourcepkg_match = False
-    distroseries_match = False
-
-    for task in bug.bug_tasks:
-        # Ugly; we have to do URL-parsing to figure this out.
-        # /ubuntu/+source/foo can be fed to launchpad.load() to get a
-        # distribution_source_package, but /ubuntu/hardy/+source/foo can't.
-        match = bug_target_re.search(task.target.self_link)
-        if (not match or
-            (options.package and
-             match.group('source') != options.package)):
-            print("Ignoring task %s in bug %s" % (task.web_link, num))
-            continue
-        sourcepkg_match = True
-        if (match.group('suite') in options.targets and
-            task.status not in ("Invalid", "Won't Fix",
-                                "Fix Released")):
-            task.status = "Fix Committed"
-            task.lp_save()
-            print("Success: task %s in bug %s" % (task.web_link, num))
-            distroseries_match = True
-
-    if sourcepkg_match and not distroseries_match:
-        # add a release task
-        lp_url = launchpad._root_uri
-        for series in options.targets:
-            series_task_url = '%subuntu/%s/+source/%s' % \
-                              (lp_url, series, options.package)
-            sourcepkg_target = launchpad.load(series_task_url)
-            new_task = bug.addTask(target=sourcepkg_target)
-            new_task.status = "Fix Committed"
-            new_task.lp_save()
-            print("LP: #%s added task for %s %s" %
-                  (num, options.package, series))
-    if not sourcepkg_match:
-        # warn that the bug has no source package tasks
-        print("LP: #%s has no %s tasks!" % (num, options.package))
-
-    bug.subscribe(person=launchpad.people['ubuntu-sru'])
-    bug.subscribe(person=launchpad.people['sru-verification'])
-
-    if not options.package or 'linux' not in options.package:
-        for t in ('verification-failed', 'verification-done'):
-            if t in bug.tags:
-                # this dance is needed due to
-                # https://bugs.launchpad.net/launchpadlib/+bug/254901
-                tags = bug.tags
-                tags.remove(t)
-                bug.tags = tags
-        bug.lp_save()
-
-        if 'verification-needed' not in bug.tags:
-            # this dance is needed due to
-            # https://bugs.launchpad.net/launchpadlib/+bug/254901
-            tags = bug.tags
-            tags.append('verification-needed')
-            bug.tags = tags
-            bug.lp_save()
-
-    text = ('Hello %s, or anyone else affected,\n\n' %
-            re.split(r'[,\s]', bug.owner.display_name)[0])
-
-    if options.package:
-        text += 'Accepted %s into ' % options.package
-    else:
-        text += 'Accepted into '
-    if options.package and options.version:
-        text += ('%s. The package will build now and be available at '
-                 'https://launchpad.net/ubuntu/+source/%s/%s in a few hours, '
-                 'and then in the -proposed repository.\n\n' % (
-                     join_english_list([
-                         '%s-proposed' % target
-                         for target in options.targets]),
-                     options.package, options.version))
-    else:
-        text += ('%s. The package will build now and be available in a few '
-                 'hours in the -proposed repository.\n\n' % (
-                     join_english_list([
-                         '%s-proposed' % target
-                         for target in options.targets])))
-
-    text += ('Please help us by testing this new package.  See '
-             'https://wiki.ubuntu.com/Testing/EnableProposed for '
-             'documentation how to enable and use -proposed.  Your feedback '
-             'will aid us getting this update out to other Ubuntu users.\n\n'
-             'If this package fixes the bug for you, please add a comment to '
-             'this bug, mentioning the version of the package you tested, and '
-             'change the tag from verification-needed to verification-done. '
-             'If it does not fix the bug for you, please add a comment '
-             'stating that, and change the tag to verification-failed.  In '
-             'either case, details of your testing will help us make a better '
-             'decision.\n\n'
-             'Further information regarding the verification process can be '
-             'found at '
-             'https://wiki.ubuntu.com/QATeam/PerformingSRUVerification .  '
-             'Thank you in advance!')
-    bug.newMessage(content=text, subject='Please test proposed package')
-
 
 def append_series(option, opt_str, value, parser):
     if value.endswith('-proposed'):
@@ -184,7 +69,9 @@
             if series.status == "Current Stable Release"][0]]
     try:
         for num in args:
-            process_bug(launchpad, options, num)
+            for series in options.targets:
+                process_bug(
+                    launchpad, options.package, options.version, series, num)
     except launchpadlib.errors.HTTPError as err:
         print("There was an error:")
         print(err.content)
 
=== modified file 'sru-review' (properties changed: +x to -x)
--- sru-review	2017-12-04 21:56:46 +0000
+++ sru-review	2017-12-07 16:28:26 +0000
@@ -43,6 +43,7 @@
 import webbrowser
 
 from launchpadlib.launchpad import Launchpad
+from sru_workflow import process_bug
 from time import sleep
 
 
@@ -245,122 +246,6 @@
                    subject='Proposed package upload rejected')
 
 
-def process_bug(launchpad, upload, num):
-    bug_target_re = re.compile(
-        r'/ubuntu/(?:(?P<suite>[^/]+)/)?\+source/(?P<source>[^/]+)$')
-    bug = launchpad.bugs[num]
-    sourcepkg = upload.package_name
-    release = upload.distroseries.name
-    sourcepkg_match = False
-    distroseries_match = False
-    for task in bug.bug_tasks:
-        # Ugly; we have to do URL-parsing to figure this out.
-        # /ubuntu/+source/foo can be fed to launchpad.load() to get a
-        # distribution_source_package, but /ubuntu/hardy/+source/foo can't.
-        match = bug_target_re.search(task.target.self_link)
-        if (not match or
-            (sourcepkg and
-             match.group('source') != sourcepkg)):
-            print("Ignoring task %s in bug %s" % (task.web_link, num))
-            continue
-        sourcepkg_match = True
-        if (match.group('suite') == release and
-            task.status not in ("Invalid", "Won't Fix",
-                                "Fix Released")):
-            task.status = "Fix Committed"
-            task.lp_save()
-            print("Success: task %s in bug %s" % (task.web_link, num))
-            distroseries_match = True
-
-    if sourcepkg_match and not distroseries_match:
-        # add a release task
-        lp_url = launchpad._root_uri
-        series_task_url = '%subuntu/%s/+source/%s' % \
-                          (lp_url, release, sourcepkg)
-        sourcepkg_target = launchpad.load(series_task_url)
-        new_task = bug.addTask(target=sourcepkg_target)
-        new_task.status = "Fix Committed"
-        new_task.lp_save()
-        print("LP: #%s added task for %s %s" % (num, sourcepkg, release))
-    if not sourcepkg_match:
-        # warn that the bug has no source package tasks
-        print("LP: #%s has no %s tasks!" % (num, sourcepkg))
-
-    # XXX: it might be useful if the package signer/sponsor was
-    #   subscribed to the bug report
-    bug.subscribe(person=launchpad.people['ubuntu-sru'])
-    bug.subscribe(person=launchpad.people['sru-verification'])
-
-    # there may be something else to sponsor so just warn
-    subscribers = [sub.person for sub in bug.subscriptions]
-    if launchpad.people['ubuntu-sponsors'] in subscribers:
-        print('ubuntu-sponsors is still subscribed to LP: #%s. '
-              'Is there anything left to sponsor?' % num)
-
-    if not sourcepkg or 'linux' not in sourcepkg:
-        # this dance is needed due to
-        # https://bugs.launchpad.net/launchpadlib/+bug/254901
-        btags = bug.tags
-        for t in ('verification-failed', 'verification-failed-%s' % release,
-                  'verification-done', 'verification-done-%s' % release):
-            if t in btags:
-                tags = btags
-                tags.remove(t)
-                bug.tags = tags
-
-        if 'verification-needed' not in btags:
-            btags.append('verification-needed')
-            bug.tags = btags
-
-        needed_tag = 'verification-needed-%s' % release
-        if needed_tag not in btags:
-            btags.append(needed_tag)
-            bug.tags = btags
-
-        bug.lp_save()
-
-
-    text = ('Hello %s, or anyone else affected,\n\n' %
-            re.split(r'[,\s]', bug.owner.display_name)[0])
-
-    if sourcepkg:
-        text += 'Accepted %s into ' % sourcepkg
-    else:
-        text += 'Accepted into '
-    if sourcepkg and release:
-        text += ('%s-proposed. The package will build now and be available at '
-                 'https://launchpad.net/ubuntu/+source/%s/%s in a few hours, '
-                 'and then in the -proposed repository.\n\n' % (
-                     release, sourcepkg, upload.package_version))
-    else:
-        text += ('%s-proposed. The package will build now and be available in '
-                 'a few hours in the -proposed repository.\n\n' % (
-                     release))
-
-    text += ('Please help us by testing this new package.  ')
-
-    if sourcepkg == 'casper':
-        text += ('To properly test it you will need to obtain and boot '
-                 'a daily build of a Live CD for %s.' % (release))
-    else:
-        text += ('See https://wiki.ubuntu.com/Testing/EnableProposed for '
-                 'documentation on how to enable and use -proposed.')
-
-    text += ('Your feedback will aid us getting this update out to other '
-             'Ubuntu users.\n\nIf this package fixes the bug for you, '
-             'please add a comment to this bug, mentioning the version of the '
-             'package you tested and change the tag from '
-             'verification-needed-%s to verification-done-%s. '
-             'If it does not fix the bug for you, please add a comment '
-             'stating that, and change the tag to verification-failed-%s. '
-             'In either case, without details of your testing we will not '
-             'be able to proceed.\n\nFurther information regarding the '
-             'verification process can be found at '
-             'https://wiki.ubuntu.com/QATeam/PerformingSRUVerification .  '
-             'Thank you in advance!' % (release, release, release))
-    bug.newMessage(content=text, subject='Please test proposed package')
-
-
 if __name__ == '__main__':
 
     default_release = 'artful'
@@ -472,7 +357,9 @@
         print("Accepted")
         if changes['bugs']:
             for bug_num in changes['bugs']:
-                process_bug(launchpad, upload, bug_num)
+                process_bug(
+                    launchpad, upload.package_name, upload.package_version,
+                    upload.distroseries.name, bug_num)
     else:
         print("REJECT the package from -proposed? [yN] ", end="")
         response = sys.stdin.readline()
 
=== added file 'sru_workflow.py'
--- sru_workflow.py	1970-01-01 00:00:00 +0000
+++ sru_workflow.py	2017-12-07 16:28:26 +0000
@@ -0,0 +1,134 @@
+#!/usr/bin/python3
+
+# Copyright (C) 2017  Canonical Ltd.
+# Author: Brian Murray <[e-mail]>
+# Author: Lukasz 'sil2100' Zemczak <[e-mail]>
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 3 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Portions of SRU-related code that is re-used by various SRU tools."""
+
+import re
+
+
+def process_bug(launchpad, sourcepkg, version, release, num):
+    bug_target_re = re.compile(
+        r'/ubuntu/(?:(?P<suite>[^/]+)/)?\+source/(?P<source>[^/]+)$')
+    bug = launchpad.bugs[num]
+    sourcepkg_match = False
+    distroseries_match = False
+    for task in bug.bug_tasks:
+        # Ugly; we have to do URL-parsing to figure this out.
+        # /ubuntu/+source/foo can be fed to launchpad.load() to get a
+        # distribution_source_package, but /ubuntu/hardy/+source/foo can't.
+        match = bug_target_re.search(task.target.self_link)
+        if (not match or
+            (sourcepkg and
+             match.group('source') != sourcepkg)):
+            print("Ignoring task %s in bug %s" % (task.web_link, num))
+            continue
+        sourcepkg_match = True
+        if (match.group('suite') == release and
+            task.status not in ("Invalid", "Won't Fix",
+                                "Fix Released")):
+            task.status = "Fix Committed"
+            task.lp_save()
+            print("Success: task %s in bug %s" % (task.web_link, num))
+            distroseries_match = True
+
+    if sourcepkg_match and not distroseries_match:
+        # add a release task
+        lp_url = launchpad._root_uri
+        series_task_url = '%subuntu/%s/+source/%s' % \
+                          (lp_url, release, sourcepkg)
+        sourcepkg_target = launchpad.load(series_task_url)
+        new_task = bug.addTask(target=sourcepkg_target)
+        new_task.status = "Fix Committed"
+        new_task.lp_save()
+        print("LP: #%s added task for %s %s" % (num, sourcepkg, release))
+    if not sourcepkg_match:
+        # warn that the bug has no source package tasks
+        print("LP: #%s has no %s tasks!" % (num, sourcepkg))
+
+    # XXX: it might be useful if the package signer/sponsor was
+    #   subscribed to the bug report
+    bug.subscribe(person=launchpad.people['ubuntu-sru'])
+    bug.subscribe(person=launchpad.people['sru-verification'])
+
+    # there may be something else to sponsor so just warn
+    subscribers = [sub.person for sub in bug.subscriptions]
+    if launchpad.people['ubuntu-sponsors'] in subscribers:
+        print('ubuntu-sponsors is still subscribed to LP: #%s. '
+              'Is there anything left to sponsor?' % num)
+
+    if not sourcepkg or 'linux' not in sourcepkg:
+        # this dance is needed due to
+        # https://bugs.launchpad.net/launchpadlib/+bug/254901
+        btags = bug.tags
+        for t in ('verification-failed', 'verification-failed-%s' % release,
+                  'verification-done', 'verification-done-%s' % release):
+            if t in btags:
+                tags = btags
+                tags.remove(t)
+                bug.tags = tags
+
+        if 'verification-needed' not in btags:
+            btags.append('verification-needed')
+            bug.tags = btags
+
+        needed_tag = 'verification-needed-%s' % release
+        if needed_tag not in btags:
+            btags.append(needed_tag)
+            bug.tags = btags
+
+        bug.lp_save()
+
+    text = ('Hello %s, or anyone else affected,\n\n' %
+            re.split(r'[,\s]', bug.owner.display_name)[0])
+
+    if sourcepkg:
+        text += 'Accepted %s into ' % sourcepkg
+    else:
+        text += 'Accepted into '
+    if sourcepkg and release:
+        text += ('%s-proposed. The package will build now and be available at '
+                 'https://launchpad.net/ubuntu/+source/%s/%s in a few hours, '
+                 'and then in the -proposed repository.\n\n' % (
+                     release, sourcepkg, version))
+    else:
+        text += ('%s-proposed. The package will build now and be available in '
+                 'a few hours in the -proposed repository.\n\n' % (
+                     release))
+
+    text += ('Please help us by testing this new package.  ')
+
+    if sourcepkg == 'casper':
+        text += ('To properly test it you will need to obtain and boot '
+                 'a daily build of a Live CD for %s.' % (release))
+    else:
+        text += ('See https://wiki.ubuntu.com/Testing/EnableProposed for '
+                 'documentation on how to enable and use -proposed.')
+
+    text += ('Your feedback will aid us getting this update out to other '
+             'Ubuntu users.\n\nIf this package fixes the bug for you, '
+             'please add a comment to this bug, mentioning the version of the '
+             'package you tested and change the tag from '
+             'verification-needed-%s to verification-done-%s. '
+             'If it does not fix the bug for you, please add a comment '
+             'stating that, and change the tag to verification-failed-%s. '
+             'In either case, without details of your testing we will not '
+             'be able to proceed.\n\nFurther information regarding the '
+             'verification process can be found at '
+             'https://wiki.ubuntu.com/QATeam/PerformingSRUVerification .  '
+             'Thank you in advance!' % (release, release, release))
+    bug.newMessage(content=text, subject='Please test proposed package')