Merge lp:~cjwatson/launchpad/pcj-reupload-fix into lp:launchpad

[Colin Watson]

Unembargo direct-copied files after calling notify, so that changelog-based notification is possible.

== Summary ==

https://code.launchpad.net/~cjwatson/launchpad/pcj-reupload/+merge/111124 didn't quite work, as indicated in the QA notes on bug 334858.

== Proposed fix ==

Add the missing transaction.commit. We didn't notice this in tests because the test in question didn't set SPR.changelog, so fix that too.

== LOC Rationale ==

+6. Same rationale as https://code.launchpad.net/~cjwatson/launchpad/pcj-reupload/+merge/111124.

== Tests ==

bin/test -vvct test_packagecopyjob

== Demo and Q/A ==

Same as https://code.launchpad.net/~cjwatson/launchpad/pcj-reupload/+merge/111124.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py	2012-06-22 23:55:02 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py	2012-06-25 10:51:19 +0000
@@ -677,13 +677,28 @@
                 include_binaries, override, close_bugs=close_bugs,
                 create_dsd_job=create_dsd_job,
                 close_bugs_since_version=old_version, creator=creator,
-                sponsor=sponsor, packageupload=packageupload, logger=logger)
+                sponsor=sponsor, packageupload=packageupload)
             if send_email:
                 notify(
                     person, source.sourcepackagerelease, [], [], archive,
                     destination_series, pocket, action='accepted',
                     announce_from_person=announce_from_person,
                     previous_version=old_version)
+            if not archive.private and has_restricted_files(source):
+                # Fix copies by overriding them according to the current
+                # ancestry and re-upload files with privacy mismatch.  We
+                # must do this *after* calling notify (which only actually
+                # sends mail on commit), because otherwise the new changelog
+                # LFA won't be visible without a commit, which may not be
+                # safe here.
+                for pub_record in sub_copies:
+                    pub_record.overrideFromAncestry()
+                    for new_file in update_files_privacy(pub_record):
+                        if logger is not None:
+                            logger.info(
+                                "Re-uploaded %s to librarian" %
+                                new_file.filename)
+
 
         overrides_index += 1
         copies.extend(sub_copies)
@@ -694,7 +709,7 @@
 def _do_direct_copy(source, archive, series, pocket, include_binaries,
                     override=None, close_bugs=True, create_dsd_job=True,
                     close_bugs_since_version=None, creator=None,
-                    sponsor=None, packageupload=None, logger=None):
+                    sponsor=None, packageupload=None):
     """Copy publishing records to another location.
 
     Copy each item of the given list of `SourcePackagePublishingHistory`
@@ -724,7 +739,6 @@
     :param sponsor: the sponsor `IPerson`, if this copy is being sponsored.
     :param packageupload: The `IPackageUpload` that caused this publication
         to be created.
-    :param logger: An optional logger.
 
     :return: a list of `ISourcePackagePublishingHistory` and
         `BinaryPackagePublishingHistory` corresponding to the copied
@@ -781,16 +795,6 @@
     # XXX cjwatson 2012-06-22 bug=869308: Fails to honour P-a-s.
     source_copy.createMissingBuilds()
 
-    if not archive.private and has_restricted_files(source):
-        # Fix copies by overriding them according to the current ancestry
-        # and re-upload files with privacy mismatch.
-        for pub_record in copies:
-            pub_record.overrideFromAncestry()
-            for new_file in update_files_privacy(pub_record):
-                if logger is not None:
-                    logger.info(
-                        "Re-uploaded %s to librarian" % new_file.filename)
-
     return copies
 
 

=== modified file 'lib/lp/soyuz/tests/test_packagecopyjob.py'
--- lib/lp/soyuz/tests/test_packagecopyjob.py	2012-06-19 23:49:20 +0000
+++ lib/lp/soyuz/tests/test_packagecopyjob.py	2012-06-25 10:51:19 +0000
@@ -1118,8 +1118,10 @@
             version="2.8-1", status=PackagePublishingStatus.PUBLISHED,
             component='multiverse', section='web',
             archive=source_archive)
-        for source_file in spph.sourcepackagerelease.files:
+        spr = spph.sourcepackagerelease
+        for source_file in spr.files:
             self.assertTrue(source_file.libraryfile.restricted)
+        spr.changelog = self.factory.makeLibraryFileAlias(restricted=True)
 
         # Now, run the copy job.
         source = getUtility(IPlainPackageCopyJobSource)