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

[Julian Edwards]

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.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-08-19 16:03:54 +0000
+++ database/schema/security.cfg	2011-08-30 15:05:02 +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/adapters/notification.py'
--- lib/lp/soyuz/adapters/notification.py	2011-08-26 14:57:39 +0000
+++ lib/lp/soyuz/adapters/notification.py	2011-08-30 15:05:02 +0000
@@ -127,7 +127,7 @@
 def notify(blamer, spr, bprs, customfiles, archive, distroseries, pocket,
            summary_text=None, changes=None, changesfile_content=None,
            changesfile_object=None, action=None, dry_run=False,
-           logger=None, announce_from_person=None):
+           logger=None, announce_from_person=None, previous_version=None):
     """Notify about
 
     :param blamer: The `IPerson` who is to blame for this notification.
@@ -149,6 +149,9 @@
     :param announce_from_person: If passed, use this `IPerson` as the From: in
         announcement emails.  If the person has no preferred email address,
         the person is ignored and the default From: is used instead.
+    :param previous_version: If specified, the change log on the email will
+        include all of the source package's change logs after that version
+        up to and including the passed spr's version.
     """
     # If this is a binary or mixed upload, we don't send *any* emails
     # provided it's not a rejection or a security upload:
@@ -213,12 +216,13 @@
 
     attach_changes = not archive.is_ppa
 
-    def build_and_send_mail(action, recipients, from_addr=None, bcc=None):
+    def build_and_send_mail(action, recipients, from_addr=None, bcc=None,
+                            previous_version=None):
         subject = calculate_subject(
             spr, bprs, customfiles, archive, distroseries, pocket, action)
         body = assemble_body(
             blamer, spr, bprs, archive, distroseries, summarystring, changes,
-            action)
+            action, previous_version=previous_version)
         body = body.encode("utf8")
         send_mail(
             spr, archive, recipients, subject, body, dry_run,
@@ -226,7 +230,8 @@
             attach_changes=attach_changes, from_addr=from_addr, bcc=bcc,
             logger=logger)
 
-    build_and_send_mail(action, recipients)
+    build_and_send_mail(
+        action, recipients, previous_version=previous_version)
 
     info = fetch_information(spr, bprs, changes)
     from_addr = info['changedby']
@@ -254,20 +259,21 @@
 
         build_and_send_mail(
             'announcement', [str(distroseries.changeslist)], from_addr,
-            bcc_addr)
+            bcc_addr, previous_version=previous_version)
 
 
 def assemble_body(blamer, spr, bprs, archive, distroseries, summary, changes,
-                  action):
+                  action, previous_version=None):
     """Assemble the e-mail notification body."""
     if changes is None:
         changes = {}
-    info = fetch_information(spr, bprs, changes)
+    info = fetch_information(
+        spr, bprs, changes, previous_version=previous_version)
     information = {
         'STATUS': ACTION_DESCRIPTIONS[action],
         'SUMMARY': summary,
         'DATE': 'Date: %s' % info['date'],
-        'CHANGESFILE': info['changesfile'],
+        'CHANGESFILE': info['changelog'],
         'DISTRO': distroseries.distribution.title,
         'ANNOUNCE': 'No announcement sent',
         'CHANGEDBY': '',
@@ -596,7 +602,7 @@
         pocket != PackagePublishingPocket.SECURITY)
 
 
-def fetch_information(spr, bprs, changes):
+def fetch_information(spr, bprs, changes, previous_version=None):
     changedby = None
     changedby_displayname = None
     maintainer = None
@@ -613,7 +619,7 @@
     elif spr or bprs:
         if not spr and bprs:
             spr = bprs[0].build.source_package_release
-        changesfile = spr.changelog_entry
+        changesfile = spr.aggregate_changelog(previous_version)
         date = spr.dateuploaded
         changedby = person_to_email(spr.creator)
         maintainer = person_to_email(spr.maintainer)
@@ -629,7 +635,7 @@
         changesfile = date = None
 
     return {
-        'changesfile': changesfile,
+        'changelog': changesfile,
         'date': date,
         'changedby': changedby,
         'changedby_displayname': changedby_displayname,

=== modified file 'lib/lp/soyuz/adapters/tests/test_notification.py'
--- lib/lp/soyuz/adapters/tests/test_notification.py	2011-08-26 14:57:39 +0000
+++ lib/lp/soyuz/adapters/tests/test_notification.py	2011-08-30 15:05:02 +0000
@@ -7,6 +7,7 @@
 
 from email.utils import formataddr
 from storm.store import Store
+from textwrap import dedent
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
@@ -132,6 +133,60 @@
             "=?utf-8?q?Lo=C3=AFc_Mot=C3=B6rhead?= <[e-mail]>",
             notifications[1]["From"])
 
+    def test_fetch_information_spr_multiple_changelogs(self):
+        # If previous_version is passed the "changelog" entry in the
+        # returned dict should contain the changelogs for all SPRs *since*
+        # that version and up to and including the passed SPR.
+        changelog = self.factory.makeChangelog(
+            spn="foo", versions=["1.2",  "1.1",  "1.0"])
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagename="foo", version="1.3", changelog=changelog)
+        self.layer.txn.commit()  # Yay, librarian.
+
+        spr = spph.sourcepackagerelease
+        info = fetch_information(spr, None, None, previous_version="1.0")
+
+        self.assertIn("foo (1.1)", info['changelog'])
+        self.assertIn("foo (1.2)", info['changelog'])
+
+    def test_notify_bpr_rejected(self):
+        # If we notify about a rejected bpr with no source, a notification is
+        # sent.
+        bpr = self.factory.makeBinaryPackageRelease()
+        changelog = self.factory.makeChangelog(spn="foo", versions=["1.1"])
+        removeSecurityProxy(
+            bpr.build.source_package_release).changelog = changelog
+        self.layer.txn.commit()
+        archive = self.factory.makeArchive()
+        pocket = self.factory.getAnyPocket()
+        distroseries = self.factory.makeDistroSeries()
+        person = self.factory.makePerson()
+        notify(
+            person, None, [bpr], [], archive, distroseries, pocket,
+            action='rejected')
+        [notification] = pop_notifications()
+        body = notification.get_payload()[0].get_payload()
+        self.assertEqual(person_to_email(person), notification['To'])
+        expected_body = dedent("""\
+            Rejected:
+            Rejected by archive administrator.
+
+            foo (1.1) unstable; urgency=3Dlow
+
+              * 1.1.
+
+            =3D=3D=3D
+
+            If you don't understand why your files were rejected please send an email
+            to [e-mail] for help (requires membership).
+
+            -- =
+
+            You are receiving this email because you are the uploader of the above
+            PPA package.
+            """)
+        self.assertEqual(expected_body, body)
+
 
 class TestNotification(TestCaseWithFactory):
 
@@ -147,7 +202,7 @@
         info = fetch_information(
             None, None, changes)
         self.assertEqual('2001-01-01', info['date'])
-        self.assertEqual(' * Foo!', info['changesfile'])
+        self.assertEqual(' * Foo!', info['changelog'])
         fields = [
             info['changedby'],
             info['maintainer'],
@@ -164,7 +219,7 @@
             creator=creator, maintainer=maintainer)
         info = fetch_information(spr, None, None)
         self.assertEqual(info['date'], spr.dateuploaded)
-        self.assertEqual(info['changesfile'], spr.changelog_entry)
+        self.assertEqual(info['changelog'], spr.changelog_entry)
         self.assertEqual(
             info['changedby'], format_address_for_person(spr.creator))
         self.assertEqual(
@@ -181,7 +236,7 @@
         info = fetch_information(None, [bpr], None)
         spr = bpr.build.source_package_release
         self.assertEqual(info['date'], spr.dateuploaded)
-        self.assertEqual(info['changesfile'], spr.changelog_entry)
+        self.assertEqual(info['changelog'], spr.changelog_entry)
         self.assertEqual(
             info['changedby'], format_address_for_person(spr.creator))
         self.assertEqual(
@@ -234,24 +289,6 @@
         notifications = pop_notifications()
         self.assertEqual(0, len(notifications))
 
-    def test_notify_bpr_rejected(self):
-        # If we notify about a rejected bpr with no source, a notification is
-        # sent.
-        bpr = self.factory.makeBinaryPackageRelease()
-        removeSecurityProxy(
-            bpr.build.source_package_release).changelog_entry = '* Foo!'
-        archive = self.factory.makeArchive()
-        pocket = self.factory.getAnyPocket()
-        distroseries = self.factory.makeDistroSeries()
-        person = self.factory.makePerson()
-        notify(
-            person, None, [bpr], [], archive, distroseries, pocket,
-            action='rejected')
-        [notification] = pop_notifications()
-        body = notification.as_string()
-        self.assertEqual(person_to_email(person), notification['To'])
-        self.assertIn('Rejected by archive administrator.\n\n* Foo!\n', body)
-
     def test_reject_changes_file_no_email(self):
         # If we are rejecting a mail, and the person to notify has no
         # preferred email, we should return early.

=== 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-30 15:05:02 +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/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py	2011-08-28 08:36:14 +0000
+++ lib/lp/soyuz/interfaces/archive.py	2011-08-30 15:05:02 +0000
@@ -974,7 +974,9 @@
         :param created_since_date: Only return results whose `date_created`
             is greater than or equal to this date.
 
-        :return: SelectResults containing `ISourcePackagePublishingHistory`.
+        :return: SelectResults containing `ISourcePackagePublishingHistory`,
+            ordered by name. If there are multiple results for the same
+            name then they are sub-ordered newest first.
         """
 
     @rename_parameters_as(

=== modified file 'lib/lp/soyuz/interfaces/sourcepackagerelease.py'
--- lib/lp/soyuz/interfaces/sourcepackagerelease.py	2011-08-26 14:57:39 +0000
+++ lib/lp/soyuz/interfaces/sourcepackagerelease.py	2011-08-30 15:05:02 +0000
@@ -240,6 +240,17 @@
         :return: total size (in KB) of this package
         """
 
+    def aggregate_changelog(since_version):
+        """Get all the changelogs since the version specified.
+
+        :param since_version: Return changelogs of all versions
+            after since_version up to and including the version of the
+            sourcepackagerelease for this publication.
+        :return: A concatenated set of changelogs of all the required
+            versions, with a blank line between each.  If there is no
+            changelog, or there is an error parsing it, None is returned.
+        """
+
 
 class PackageDiffAlreadyRequestedError(Exception):
     """Raised when an `IPackageDiff` request already exists."""

=== modified file 'lib/lp/soyuz/model/sourcepackagerelease.py'
--- lib/lp/soyuz/model/sourcepackagerelease.py	2011-08-26 14:57:39 +0000
+++ lib/lp/soyuz/model/sourcepackagerelease.py	2011-08-30 15:05:02 +0000
@@ -10,7 +10,13 @@
     ]
 
 
+import apt_pkg
 import datetime
+from debian.changelog import (
+    Changelog,
+    ChangelogCreateError,
+    ChangelogParseError,
+    )
 import operator
 import re
 from StringIO import StringIO
@@ -603,3 +609,40 @@
         return PackageDiff(
             from_source=self, to_source=to_sourcepackagerelease,
             requester=requester, status=status)
+
+    def aggregate_changelog(self, since_version):
+        """See `ISourcePackagePublishingHistory`."""
+        if self.changelog is None:
+            return None
+
+        apt_pkg.InitSystem()
+        chunks = []
+        changelog = self.changelog
+        # The python-debian API for parsing changelogs is pretty awful. The
+        # only useful way of extracting info is to use the iterator on
+        # Changelog and then compare versions.
+        try:
+            for block in Changelog(changelog.read()):
+                version = block._raw_version
+                if (since_version and
+                    apt_pkg.VersionCompare(version, since_version) <= 0):
+                    break
+                # Poking in private attributes is not nice but again the
+                # API is terrible.  We want to ensure that the name/date
+                # line is omitted from these composite changelogs.
+                block._no_trailer = True
+                try:
+                    # python-debian adds an extra blank line to the chunks
+                    # so we'll have to sort this out.
+                    chunks.append(str(block).rstrip())
+                except ChangelogCreateError:
+                    continue
+                if not since_version:
+                    # If a particular version was not requested we just
+                    # return the most recent changelog entry.
+                    break
+        except ChangelogParseError:
+            return None
+
+        output = "\n\n".join(chunks)
+        return output.decode("utf-8", "replace")

=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py	2011-08-26 14:57:39 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py	2011-08-30 15:05:02 +0000
@@ -628,16 +628,28 @@
             override = None
             if overrides:
                 override = overrides[overrides_index]
+            old_version = None
+            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
             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,
                     destination_series, pocket, changes=None,
                     action='accepted',
-                    announce_from_person=announce_from_person)
+                    announce_from_person=announce_from_person,
+                    previous_version=old_version)
 
         overrides_index += 1
         copies.extend(sub_copies)
@@ -646,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`
@@ -669,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
@@ -700,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-30 15:05:02 +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,33 @@
     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 is not None:
+        assert changesfile_object is None, (
+            "Only set since_version if changesfile_object is None")
+
+    if changesfile_object:
+        bugs_to_close = get_bugs_from_changes_file(changesfile_object)
+    else:
+        bugs_to_close = get_bugs_from_changelog_entry(
+            source_release, since_version=since_version)
 
     # No bugs to be closed by this upload, move on.
     if not bugs_to_close:

=== modified file 'lib/lp/soyuz/scripts/tests/test_copypackage.py'
--- lib/lp/soyuz/scripts/tests/test_copypackage.py	2011-08-26 14:57:39 +0000
+++ lib/lp/soyuz/scripts/tests/test_copypackage.py	2011-08-30 15:05:02 +0000
@@ -1413,7 +1413,9 @@
         archive = self.test_publisher.ubuntutest.main_archive
         source = self.test_publisher.getPubSource(
             archive=archive, version='1.0-2', architecturehintlist='any')
-        source.sourcepackagerelease.changelog_entry = '* Foo!'
+        changelog = self.factory.makeChangelog(spn="foo", versions=["1.0-2"])
+        source.sourcepackagerelease.changelog = changelog
+        transaction.commit()  # Librarian.
         nobby = self.createNobby(('i386', 'hppa'))
         getUtility(ISourcePackageFormatSelectionSet).add(
             nobby, SourcePackageFormat.FORMAT_1_0)
@@ -1427,9 +1429,22 @@
         self.assertEquals(
             get_ppa_reference(target_archive),
             notification['X-Launchpad-PPA'])
-        self.assertIn(
-            source.sourcepackagerelease.changelog_entry,
-            notification.as_string())
+        body = notification.get_payload()[0].get_payload()
+        expected = dedent("""\
+            Accepted:
+             OK: foo_1.0-2.dsc
+                 -> Component: main Section: base
+
+            foo (1.0-2) unstable; urgency=3Dlow
+
+              * 1.0-2.
+
+            -- =
+
+            You are receiving this email because you are the uploader of the above
+            PPA package.
+            """)
+        self.assertEqual(expected, body)
 
     def test_copy_generates_notification(self):
         # When a copy into a primary archive is performed, a notification is
@@ -1437,7 +1452,8 @@
         archive = self.test_publisher.ubuntutest.main_archive
         source = self.test_publisher.getPubSource(
             archive=archive, version='1.0-2', architecturehintlist='any')
-        source.sourcepackagerelease.changelog_entry = '* Foo!'
+        changelog = self.factory.makeChangelog(spn="foo", versions=["1.0-2"])
+        source.sourcepackagerelease.changelog = changelog
         # Copying to a primary archive reads the changes to close bugs.
         transaction.commit()
         nobby = self.createNobby(('i386', 'hppa'))
@@ -1455,15 +1471,59 @@
         for mail in (notification, announcement):
             self.assertEquals(
                 '[ubuntutest/nobby] foo 1.0-2 (Accepted)', mail['Subject'])
-        expected_text = dedent("""
-            * Foo!
+        expected_text = dedent("""\
+            foo (1.0-2) unstable; urgency=3Dlow
+
+              * 1.0-2.
 
             Date: %s
             Changed-By: Foo Bar <[e-mail]>
             http://launchpad.dev/ubuntutest/nobby/+source/foo/1.0-2
             """ % source.sourcepackagerelease.dateuploaded)
-        self.assertIn(expected_text, notification.as_string())
-        self.assertIn(expected_text, announcement.as_string())
+        # Spurious newlines are a pain and don't really affect the end
+        # results so stripping is the easiest route here.
+        expected_text.strip()
+        body = mail.get_payload()[0].get_payload()
+        self.assertEqual(expected_text, body)
+        self.assertEqual(expected_text, body)
+
+    def test_copy_notification_contains_aggregate_change_log(self):
+        # When copying a package that generates a notification,
+        # the changelog should contain all of the changelog_entry texts for
+        # all the sourcepackagereleases between the last published version
+        # and the new version.
+        archive = self.test_publisher.ubuntutest.main_archive
+        source3 = self.test_publisher.getPubSource(
+            sourcename="foo", archive=archive, version='1.2',
+            architecturehintlist='any')
+        changelog = self.factory.makeChangelog(
+            spn="foo", versions=["1.2",  "1.1",  "1.0"])
+        source3.sourcepackagerelease.changelog = changelog
+        transaction.commit()
+
+        # Now make a new series, nobby, and publish foo 1.0 in it.
+        nobby = self.createNobby(('i386', 'hppa'))
+        getUtility(ISourcePackageFormatSelectionSet).add(
+            nobby, SourcePackageFormat.FORMAT_1_0)
+        nobby.changeslist = '[e-mail]'
+        source1 = self.factory.makeSourcePackageRelease(
+            sourcepackagename="foo", version="1.0")
+        self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagerelease=source1, distroseries=nobby,
+            status=PackagePublishingStatus.PUBLISHED,
+            pocket=source3.pocket)
+
+        # Now copy foo 1.3 from ubuntutest.
+        [copied_source] = do_copy(
+            [source3], nobby.main_archive, nobby, source3.pocket, False,
+            person=source3.sourcepackagerelease.creator,
+            check_permissions=False, send_email=True)
+
+        [notification, announcement] = pop_notifications()
+        for mail in (notification, announcement):
+            mailtext = mail.as_string()
+            self.assertIn("foo (1.1)", mailtext)
+            self.assertIn("foo (1.2)", mailtext)
 
     def test_copy_generates_rejection_email(self):
         # When a copy into a primary archive fails, we expect a rejection

=== 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-30 15:05:02 +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-30 15:05:02 +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 'lib/lp/soyuz/tests/test_sourcepackagerelease.py'
--- lib/lp/soyuz/tests/test_sourcepackagerelease.py	2011-08-26 14:57:39 +0000
+++ lib/lp/soyuz/tests/test_sourcepackagerelease.py	2011-08-30 15:05:02 +0000
@@ -5,6 +5,7 @@
 
 __metaclass__ = type
 
+from textwrap import dedent
 import transaction
 from zope.component import getUtility
 
@@ -81,6 +82,32 @@
         spr = self.factory.makeSourcePackageRelease(homepage="<invalid<url")
         self.assertEquals("<invalid<url", spr.homepage)
 
+    def test_aggregate_changelog(self):
+        # If since_version is passed the "changelog" entry returned
+        # should contain the changelogs for all releases *since*
+        # that version and up to and including the context SPR.
+        changelog = self.factory.makeChangelog(
+            spn="foo", versions=["1.3",  "1.2",  "1.1",  "1.0"])
+        expected_changelog = dedent(u"""\
+            foo (1.3) unstable; urgency=low
+
+              * 1.3.
+
+            foo (1.2) unstable; urgency=low
+
+              * 1.2.
+
+            foo (1.1) unstable; urgency=low
+
+              * 1.1.""")
+        spph = self.factory.makeSourcePackagePublishingHistory(
+            sourcepackagename="foo", version="1.3", changelog=changelog)
+        transaction.commit()  # Yay, librarian.
+
+        observed = spph.sourcepackagerelease.aggregate_changelog(
+            since_version="1.0")
+        self.assertEqual(expected_changelog, observed)
+
 
 class TestSourcePackageReleaseGetBuildByArch(TestCaseWithFactory):
     """Tests for SourcePackageRelease.getBuildByArch()."""

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2011-08-30 00:07:34 +0000
+++ lib/lp/testing/factory.py	2011-08-30 15:05:02 +0000
@@ -1,3 +1,7 @@
+# -*- coding: utf-8 -*-
+# NOTE: The first line above must stay first; do not move the copyright
+# notice to the top.  See http://www.python.org/dev/peps/pep-0263/.
+#
 # Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
@@ -2181,21 +2185,26 @@
                 review_status=review_status)
 
     def makeChangelog(self, spn=None, versions=[]):
-        """Create and return a LFA of a valid Debian-style changelog."""
+        """Create and return a LFA of a valid Debian-style changelog.
+
+        Note that the changelog returned is unicode - this is deliberate
+        so that code is forced to cope with it as utf-8 changelogs are
+        normal.
+        """
         if spn is None:
             spn = self.getUniqueString()
         changelog = ''
         for version in versions:
-            entry = dedent('''
+            entry = dedent(u'''\
             %s (%s) unstable; urgency=low
 
               * %s.
 
-             -- Foo Bar <[e-mail]>  Tue, 01 Jan 1970 01:50:41 +0000
+             -- Føo Bær <[e-mail]>  Tue, 01 Jan 1970 01:50:41 +0000
 
             ''' % (spn, version, version))
             changelog += entry
-        return self.makeLibraryFileAlias(content=changelog)
+        return self.makeLibraryFileAlias(content=changelog.encode("utf-8"))
 
     def makeCodeImportEvent(self):
         """Create and return a CodeImportEvent."""

=== 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-30 15:05:02 +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