Merge lp:~julian-edwards/launchpad/sync-close-bugs-bug-833736 into lp:launchpad

[Julian Edwards]

[r=gmb][bug=833736] When syncing packages to a distribution, make sure any bugs referenced in the package's changelog are closed.

This branch fixes bug 833736 by ensuring that bugs mentioned in Debian package changelogs are closed when the package is synced to Ubuntu.

The fix is mainly in 2 places:
 1. The packagecopier determines the most recently published version of the package being copied and passes that to the bug closing code.
 2. The bug closing code is fixed to parse changelogs (it only used to parse the changes file) and grabs as many version chunks as necessary to complete the missing history.

I also refactored the regexes that the sync-source script uses to scan changelogs.

[Graham Binns (gmb)]

review: Approve (code)

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-08-19 16:03:54 +0000
+++ database/schema/security.cfg	2011-08-31 12:04:12 +0000
@@ -1014,7 +1014,10 @@
 type=user
 
 [sync_packages]
-groups=script
+# sync_packages does a lot of the same work that process-accepted.py (queued)
+# does, so it's easier to inherit its permissions than try and work them all
+# out from scratch again.
+groups=script,queued
 public.account                                = SELECT
 public.archive                                = SELECT
 public.archivearch                            = SELECT

=== modified file 'lib/lp/soyuz/enums.py'
--- lib/lp/soyuz/enums.py	2011-06-02 11:04:56 +0000
+++ lib/lp/soyuz/enums.py	2011-08-31 12:04:12 +0000
@@ -20,15 +20,27 @@
     'PackagePublishingStatus',
     'PackageUploadCustomFormat',
     'PackageUploadStatus',
+    're_bug_numbers',
+    're_closes',
+    're_lp_closes',
     'SourcePackageFormat',
     ]
 
+import re
+
 from lazr.enum import (
     DBEnumeratedType,
     DBItem,
     )
 
 
+# Regexes that match bug numbers for closing in change logs.
+re_closes = re.compile(
+    r"closes:\s*(?:bug)?\#?\s?\d+(?:,\s*(?:bug)?\#?\s?\d+)*", re.I)
+re_lp_closes = re.compile(r"lp:\s+\#\d+(?:,\s*\#\d+)*", re.I)
+re_bug_numbers = re.compile(r"\#?\s?(\d+)")
+
+
 class ArchiveJobType(DBEnumeratedType):
     """Values that IArchiveJob.job_type can take."""
 

=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py	2011-08-26 16:28:04 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py	2011-08-31 12:04:12 +0000
@@ -628,21 +628,21 @@
             override = None
             if overrides:
                 override = overrides[overrides_index]
-            if send_email:
-                # Make a note of the destination source's version for use
-                # in sending the email notification.
-                existing = archive.getPublishedSources(
-                    name=source.sourcepackagerelease.name, exact_match=True,
-                    status=active_publishing_status,
-                    distroseries=series, pocket=pocket).first()
-                if existing:
-                    old_version = existing.sourcepackagerelease.version
-                else:
-                    old_version = None
+            # Make a note of the destination source's version for use
+            # in sending the email notification and closing bugs.
+            existing = archive.getPublishedSources(
+                name=source.sourcepackagerelease.name, exact_match=True,
+                status=active_publishing_status,
+                distroseries=series, pocket=pocket).first()
+            if existing:
+                old_version = existing.sourcepackagerelease.version
+            else:
+                old_version = None
             sub_copies = _do_direct_copy(
                 source, archive, destination_series, pocket,
                 include_binaries, override, close_bugs=close_bugs,
-                create_dsd_job=create_dsd_job)
+                create_dsd_job=create_dsd_job,
+                close_bugs_since_version=old_version)
             if send_email:
                 notify(
                     person, source.sourcepackagerelease, [], [], archive,
@@ -658,7 +658,8 @@
 
 
 def _do_direct_copy(source, archive, series, pocket, include_binaries,
-                    override=None, close_bugs=True, create_dsd_job=True):
+                    override=None, close_bugs=True, create_dsd_job=True,
+                    close_bugs_since_version=None):
     """Copy publishing records to another location.
 
     Copy each item of the given list of `SourcePackagePublishingHistory`
@@ -681,6 +682,9 @@
         copied publication should be closed.
     :param create_dsd_job: A boolean indicating whether or not a dsd job
          should be created for the new source publication.
+    :param close_bugs_since_version: If close_bugs is True,
+        then this parameter says which changelog entries to parse looking
+        for bugs to close.  See `close_bugs_for_sourcepackagerelease`.
 
     :return: a list of `ISourcePackagePublishingHistory` and
         `BinaryPackagePublishingHistory` corresponding to the copied
@@ -712,7 +716,8 @@
         source_copy = source.copyTo(
             series, pocket, archive, override, create_dsd_job=create_dsd_job)
         if close_bugs:
-            close_bugs_for_sourcepublication(source_copy)
+            close_bugs_for_sourcepublication(
+                source_copy, close_bugs_since_version)
         copies.append(source_copy)
     else:
         source_copy = source_in_destination.first()

=== modified file 'lib/lp/soyuz/scripts/processaccepted.py'
--- lib/lp/soyuz/scripts/processaccepted.py	2011-08-08 07:31:08 +0000
+++ lib/lp/soyuz/scripts/processaccepted.py	2011-08-31 12:04:12 +0000
@@ -37,6 +37,9 @@
 from lp.soyuz.enums import (
     ArchivePurpose,
     PackageUploadStatus,
+    re_bug_numbers,
+    re_closes,
+    re_lp_closes,
     )
 from lp.soyuz.interfaces.archive import IArchiveSet
 from lp.soyuz.interfaces.queue import IPackageUploadSet
@@ -64,6 +67,35 @@
     return bugs
 
 
+def get_bugs_from_changelog_entry(sourcepackagerelease, since_version):
+    """Parse the changelog_entry in the sourcepackagerelease and return a
+    list of `IBug`s referenced by it.
+    """
+    changelog = sourcepackagerelease.aggregate_changelog(since_version)
+    closes = []
+    # There are 2 main regexes to match.  Each match from those can then
+    # have further multiple matches from the 3rd regex:
+    # closes: NNN, NNN
+    # lp: #NNN, #NNN
+    regexes = (
+        re_closes.finditer(changelog), re_lp_closes.finditer(changelog))
+    for regex in regexes:
+        for match in regex:
+            bug_match = re_bug_numbers.findall(match.group(0))
+            closes += map(int, bug_match)
+
+    bugs = []
+    for bug_id in closes:
+        try:
+            bug = getUtility(IBugSet).get(bug_id)
+        except NotFoundError:
+            continue
+        else:
+            bugs.append(bug)
+
+    return bugs
+
+
 def can_close_bugs(target):
     """Whether or not bugs should be closed in the given target.
 
@@ -119,7 +151,7 @@
             source_queue_item.sourcepackagerelease, changesfile_object)
 
 
-def close_bugs_for_sourcepublication(source_publication):
+def close_bugs_for_sourcepublication(source_publication, since_version=None):
     """Close bugs for a given sourcepublication.
 
     Given a `ISourcePackagePublishingHistory` close bugs mentioned in
@@ -131,21 +163,31 @@
     sourcepackagerelease = source_publication.sourcepackagerelease
     changesfile_object = sourcepackagerelease.upload_changesfile
 
-    # No changesfile available, cannot close bugs.
-    if changesfile_object is None:
-        return
-
     close_bugs_for_sourcepackagerelease(
-        sourcepackagerelease, changesfile_object)
-
-
-def close_bugs_for_sourcepackagerelease(source_release, changesfile_object):
+        sourcepackagerelease, changesfile_object, since_version)
+
+
+def close_bugs_for_sourcepackagerelease(source_release, changesfile_object,
+                                        since_version=None):
     """Close bugs for a given source.
 
     Given a `ISourcePackageRelease` and a corresponding changesfile object,
     close bugs mentioned in the changesfile in the context of the source.
+
+    If changesfile_object is None and since_version is supplied,
+    close all the bugs in changelog entries made after that version and up
+    to and including the source_release's version.  It does this by parsing
+    the changelog on the sourcepackagerelease.  This could be extended in
+    the future to deal with the changes file as well but there is no
+    requirement to do so right now.
     """
-    bugs_to_close = get_bugs_from_changes_file(changesfile_object)
+    if since_version and source_release.changelog:
+        bugs_to_close = get_bugs_from_changelog_entry(
+            source_release, since_version=since_version)
+    elif changesfile_object:
+        bugs_to_close = get_bugs_from_changes_file(changesfile_object)
+    else:
+        return
 
     # No bugs to be closed by this upload, move on.
     if not bugs_to_close:

=== added file 'lib/lp/soyuz/scripts/tests/test_processaccepted.py'
--- lib/lp/soyuz/scripts/tests/test_processaccepted.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/scripts/tests/test_processaccepted.py	2011-08-31 12:04:12 +0000
@@ -0,0 +1,91 @@
+# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from textwrap import dedent
+from zope.security.proxy import removeSecurityProxy
+
+from canonical.testing.layers import LaunchpadZopelessLayer
+from lp.bugs.interfaces.bugtask import BugTaskStatus
+from lp.soyuz.scripts.processaccepted import (
+    close_bugs_for_sourcepackagerelease,
+    )
+from lp.testing import TestCaseWithFactory
+
+
+class TestClosingBugs(TestCaseWithFactory):
+    """Test the various bug closing methods in processaccepted.py.
+
+    Tests are currently spread around the codebase; this is an attempt to
+    start a unification in a single file and those other tests need
+    migrating here.
+    See also:
+        * lp/soyuz/scripts/tests/test_queue.py
+        * lib/lp/soyuz/doc/closing-bugs-from-changelogs.txt
+        * lib/lp/archiveuploader/tests/nascentupload-closing-bugs.txt
+    """
+    layer = LaunchpadZopelessLayer
+
+    def test_close_bugs_for_sourcepackagerelease_with_no_changes_file(self):
+        # If there's no changes file it should read the changelog_entry on
+        # the sourcepackagerelease.
+
+        spr = self.factory.makeSourcePackageRelease(changelog_entry="blah")
+
+        # Make 4 bugs and corresponding bugtasks and put them in an array
+        # as tuples.
+        bugs = []
+        for i in range(5):
+            bug = self.factory.makeBug()
+            bugtask = self.factory.makeBugTask(
+                target=spr.sourcepackage, bug=bug)
+            bugs.append((bug, bugtask))
+
+        unfixed_bug = self.factory.makeBug()
+        unfixed_task = self.factory.makeBugTask(
+            target=spr.sourcepackage, bug=unfixed_bug)
+
+        # Make a changelog entry for a package which contains the IDs of
+        # the 5 bugs separated across 2 releases.
+        changelog=dedent("""
+            foo (1.0-3) unstable; urgency=low
+
+              * closes: %s, %s
+              * lp: #%s, #%s
+
+             -- Foo Bar <[e-mail]>  Tue, 01 Jan 1970 01:50:41 +0000
+
+            foo (1.0-2) unstable; urgency=low
+
+              * closes: %s
+
+             -- Foo Bar <[e-mail]>  Tue, 01 Jan 1970 01:50:41 +0000
+
+            foo (1.0-1) unstable; urgency=low
+
+              * closes: %s
+
+             -- Foo Bar <[e-mail]>  Tue, 01 Jan 1970 01:50:41 +0000
+
+            """ % (
+            bugs[0][0].id,
+            bugs[1][0].id,
+            bugs[2][0].id,
+            bugs[3][0].id,
+            bugs[4][0].id,
+            unfixed_bug.id,
+            ))
+        lfa = self.factory.makeLibraryFileAlias(content=changelog)
+
+        removeSecurityProxy(spr).changelog = lfa
+        self.layer.txn.commit()
+
+        # Call the method and test it's closed the bugs.
+        close_bugs_for_sourcepackagerelease(spr, changesfile_object=None,
+                                            since_version="1.0-1")
+        for bug, bugtask in bugs:
+            self.assertEqual(BugTaskStatus.FIXRELEASED, bugtask.status)
+
+        self.assertEqual(BugTaskStatus.NEW, unfixed_task.status)
+

=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py	2011-08-23 14:43:56 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py	2011-08-31 12:04:12 +0000
@@ -8,6 +8,7 @@
 from storm.store import Store
 from testtools.content import text_content
 from testtools.matchers import MatchesStructure
+from textwrap import dedent
 import transaction
 from zope.component import getUtility
 from zope.security.interfaces import Unauthorized
@@ -21,6 +22,7 @@
     LaunchpadZopelessLayer,
     ZopelessDatabaseLayer,
     )
+from lp.bugs.interfaces.bugtask import BugTaskStatus
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.registry.interfaces.series import SeriesStatus
 from lp.registry.model.distroseriesdifferencecomment import (
@@ -919,6 +921,86 @@
             "Nancy Requester <[e-mail]>",
             emails[1]['From'])
 
+    def test_copying_closes_bugs(self):
+        # Copying a package into a primary archive should close any bugs
+        # mentioned in its changelog for versions added since the most
+        # recently published version in the target.
+
+        # Firstly, lots of boring set up.
+        publisher = SoyuzTestPublisher()
+        publisher.prepareBreezyAutotest()
+        distroseries = publisher.breezy_autotest
+        target_archive = self.factory.makeArchive(
+            distroseries.distribution, purpose=ArchivePurpose.PRIMARY)
+        source_archive = self.factory.makeArchive()
+        bug280 = self.factory.makeBug()
+        bug281 = self.factory.makeBug()
+        bug282 = self.factory.makeBug()
+
+        # Publish a package in the source archive and give it a changelog
+        # entry that closes a bug.
+        source_pub = self.factory.makeSourcePackagePublishingHistory(
+            distroseries=distroseries, sourcepackagename="libc",
+            version="2.8-2", status=PackagePublishingStatus.PUBLISHED,
+            archive=source_archive)
+        spr = removeSecurityProxy(source_pub).sourcepackagerelease
+        changelog = dedent("""\
+            libc (2.8-2) unstable; urgency=low
+
+              * closes: %s
+
+             -- Foo Bar <[e-mail]>  Tue, 01 Jan 1970 01:50:41 +0000
+
+            libc (2.8-1) unstable; urgency=low
+
+              * closes: %s
+
+             -- Foo Bar <[e-mail]>  Tue, 01 Jan 1970 01:50:41 +0000
+
+            libc (2.8-0) unstable; urgency=low
+
+              * closes: %s
+
+             -- Foo Bar <[e-mail]>  Tue, 01 Jan 1970 01:50:41 +0000
+            """ % (bug282.id, bug281.id, bug280.id))
+        spr.changelog = self.factory.makeLibraryFileAlias(content=changelog)
+        spr.changelog_entry = "dummy"
+        self.layer.txn.commit()  # Librarian.
+        bugtask280 = self.factory.makeBugTask(
+            target=spr.sourcepackage, bug=bug280)
+        bugtask281 = self.factory.makeBugTask(
+            target=spr.sourcepackage, bug=bug281)
+        bugtask282 = self.factory.makeBugTask(
+            target=spr.sourcepackage, bug=bug282)
+
+        # Now put the same named package in the target archive at the
+        # oldest version in the changelog.
+        publisher.getPubSource(
+            distroseries=distroseries, sourcename="libc",
+            version="2.8-0", status=PackagePublishingStatus.PUBLISHED,
+            archive=target_archive)
+
+        # Run the copy job.
+        source = getUtility(IPlainPackageCopyJobSource)
+        requester = self.factory.makePerson()
+        with person_logged_in(target_archive.owner):
+            target_archive.newComponentUploader(requester, "main")
+        job = source.create(
+            package_name="libc",
+            package_version="2.8-2",
+            source_archive=source_archive,
+            target_archive=target_archive,
+            target_distroseries=distroseries,
+            target_pocket=PackagePublishingPocket.RELEASE,
+            include_binaries=False,
+            requester=requester)
+        self.runJob(job)
+
+        # All the bugs apart from the one for 2.8-0 should be fixed.
+        self.assertEqual(BugTaskStatus.FIXRELEASED, bugtask282.status)
+        self.assertEqual(BugTaskStatus.FIXRELEASED, bugtask281.status)
+        self.assertEqual(BugTaskStatus.NEW, bugtask280.status)
+
     def test_findMatchingDSDs_matches_all_DSDs_for_job(self):
         # findMatchingDSDs finds matching DSDs for any of the packages
         # in the job.

=== modified file 'scripts/ftpmaster-tools/sync-source.py'
--- scripts/ftpmaster-tools/sync-source.py	2011-06-09 10:50:25 +0000
+++ scripts/ftpmaster-tools/sync-source.py	2011-08-31 12:04:12 +0000
@@ -51,7 +51,12 @@
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.person import IPersonSet
 from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.soyuz.enums import PackagePublishingStatus
+from lp.soyuz.enums import (
+    PackagePublishingStatus,
+    re_bug_numbers,
+    re_closes,
+    re_lp_closes,
+    )
 from lp.soyuz.scripts.ftpmaster import (
     generate_changes,
     SyncSource,
@@ -63,10 +68,6 @@
 re_strip_revision = re.compile(r"-([^-]+)$")
 re_changelog_header = re.compile(
     r"^\S+ \((?P<version>.*)\) .*;.*urgency=(?P<urgency>\w+).*")
-re_closes = re.compile(
-    r"closes:\s*(?:bug)?\#?\s?\d+(?:,\s*(?:bug)?\#?\s?\d+)*", re.I)
-re_lp_closes = re.compile(r"lp:\s+\#\d+(?:,\s*\#\d+)*", re.I)
-re_bug_numbers = re.compile(r"\#?\s?(\d+)")
 
 
 Blacklisted = None