launchpad-reviewers team mailing list archive

[Raphaël Victor Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/init-series-not-close-bugs into lp:launchpad with lp:~rvb/launchpad/perm-distributionjob-bug-808680 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #808680 in Launchpad itself: "The initializeddistroseries job fails with "permission denied for relation distributionjob""
  https://bugs.launchpad.net/launchpad/+bug/808680

For more details, see:
https://code.launchpad.net/~rvb/launchpad/init-series-not-close-bugs/+merge/68075

The packagecopier closes the bugs of the copied publications. We use the packagecopier when initializing but we don't want any bug to be closed in this case. This branch adds a parameter to the packagecopier method do_copy to be able to prevent the bugs from being closed when using this method in the initializing code.

= Test =

./bin/test -vvc test_initderiveddistroseries test_multiple_parents_close_bugs

= QA = 

Initializing a series with multiple parents and bugs present in the second parent should be successful (as opposed to failing with a permission denied error because the db user used by the initialization script does not have access to the bug table).
-- 
https://code.launchpad.net/~rvb/launchpad/init-series-not-close-bugs/+merge/68075
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/init-series-not-close-bugs into lp:launchpad.

=== modified file 'lib/lp/registry/tests/test_initderiveddistroseries.py'
--- lib/lp/registry/tests/test_initderiveddistroseries.py	2011-07-15 11:17:42 +0000
+++ lib/lp/registry/tests/test_initderiveddistroseries.py	2011-07-15 11:17:48 +0000
@@ -17,6 +17,7 @@
     )
 from lp.registry.interfaces.distroseries import DerivationError
 from lp.services.features.testing import FeatureFixture
+from lp.soyuz.enums import PackageUploadStatus
 from lp.soyuz.interfaces.distributionjob import (
     IInitializeDistroSeriesJobSource,
     )
@@ -128,3 +129,37 @@
         # Switch back to launchpad_main to be able to cleanup the
         # feature flags.
         self.layer.switchDbUser('launchpad_main')
+
+    def test_multiple_parents_close_bugs(self):
+        # Even when bugs are present in the second parent, the initialization
+        # does not close the bugs on the copied publications (and thus
+        # does not try to access the bug table).
+        self.useFixture(FeatureFixture({FEATURE_FLAG_ENABLE_MODULE: u'on'}))
+        parent1, parent2 = self.setUpParents(
+            packages1={'p1': '0.1-1'}, packages2={'p2': '2.1'})
+        source = parent2.main_archive.getPublishedSources(
+            distroseries=parent2)[0]
+        # Setup a bug and populate
+        # source.sourcepackagerelease.upload_changesfile.
+        bug = self.factory.makeBug(series=parent2)
+        changes_file_content = (
+            "Format: 1.7\nLaunchpad-bugs-fixed: %s\n"
+            % bug.id)
+        pu = self.factory.makePackageUpload(
+            archive=parent2.main_archive, distroseries=parent2,
+            changes_file_content=changes_file_content,
+            status=PackageUploadStatus.DONE)
+        pu.addSource(source.sourcepackagerelease)
+        child = self.factory.makeDistroSeries()
+        transaction.commit()
+        self.layer.switchDbUser('initializedistroseries')
+
+        child = self._fullInitialize(
+            [parent1, parent2], child=child)
+        # Make sure the initialization was successful.
+        self.assertBinPackagesAndVersions(
+            child,
+            [(u'p1', u'0.1-1'), (u'p2', u'2.1')])
+        # Switch back to launchpad_main to be able to cleanup the
+        # feature flags.
+        self.layer.switchDbUser('launchpad_main')

=== modified file 'lib/lp/soyuz/scripts/initialize_distroseries.py'
--- lib/lp/soyuz/scripts/initialize_distroseries.py	2011-07-15 11:17:42 +0000
+++ lib/lp/soyuz/scripts/initialize_distroseries.py	2011-07-15 11:17:48 +0000
@@ -383,7 +383,7 @@
                             sources, target_archive, self.distroseries,
                             pocket, include_binaries=not self.rebuild,
                             check_permissions=False, strict_binaries=False,
-                            create_dsd_job=False)
+                            create_dsd_job=False, close_bugs=False)
                         if self.rebuild:
                             for pubrec in sources_published:
                                 pubrec.createMissingBuilds()

=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py	2011-07-15 11:17:42 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py	2011-07-15 11:17:48 +0000
@@ -526,7 +526,7 @@
 def do_copy(sources, archive, series, pocket, include_binaries=False,
             allow_delayed_copies=True, person=None, check_permissions=True,
             overrides=None, send_email=False, strict_binaries=True,
-            create_dsd_job=True):
+            create_dsd_job=True, close_bugs=True):
     """Perform the complete copy of the given sources incrementally.
 
     Verifies if each copy can be performed using `CopyChecker` and
@@ -562,6 +562,9 @@
         to True will make the copy fail if binaries cannot be also copied.
     :param create_dsd_job: A boolean indicating whether or not dsd jobs
         should be created for the copied source publications.
+    :param close_bugs: A boolean indicating whether or not bugs on the
+        copied publications should be closed.
+
 
     :raise CannotCopy when one or more copies were not allowed. The error
         will contain the reason why each copy was denied.
@@ -608,7 +611,8 @@
                 override = overrides[overrides_index]
             sub_copies = _do_direct_copy(
                 source, archive, destination_series, pocket,
-                include_binaries, override, create_dsd_job=create_dsd_job)
+                include_binaries, override, create_dsd_job=create_dsd_job,
+                close_bugs=close_bugs)
             if send_email:
                 notify(
                     person, source.sourcepackagerelease, [], [], archive,
@@ -622,7 +626,7 @@
 
 
 def _do_direct_copy(source, archive, series, pocket, include_binaries,
-                    override=None, create_dsd_job=True):
+                    override=None, create_dsd_job=True, close_bugs=True):
     """Copy publishing records to another location.
 
     Copy each item of the given list of `SourcePackagePublishingHistory`
@@ -643,6 +647,8 @@
     :param override: An `IOverride` as per do_copy().
     :param create_dsd_job: A boolean indicating whether or not a dsd job
         should be created for the copied source publication.
+    :param close_bugs: A boolean indicating whether or not bugs on the
+        copied publication should be closed.
 
     :return: a list of `ISourcePackagePublishingHistory` and
         `BinaryPackagePublishingHistory` corresponding to the copied
@@ -673,7 +679,8 @@
             override = overrides[0]
         source_copy = source.copyTo(
             series, pocket, archive, override, create_dsd_job)
-        close_bugs_for_sourcepublication(source_copy)
+        if close_bugs:
+            close_bugs_for_sourcepublication(source_copy)
         copies.append(source_copy)
     else:
         source_copy = source_in_destination.first()