launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-745542 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #745542 in Launchpad itself: "Limit parent-only DSD[J] generation to packagesets"
  https://bugs.launchpad.net/launchpad/+bug/745542

For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-745542/+merge/56106


-- 
https://code.launchpad.net/~jtv/launchpad/bug-745542/+merge/56106
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-745542 into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2011-04-01 00:47:03 +0000
+++ database/schema/security.cfg	2011-04-04 07:26:34 +0000
@@ -1076,6 +1076,7 @@
 public.job                                      = SELECT, UPDATE
 public.libraryfilealias                         = SELECT
 public.libraryfilecontent                       = SELECT
+public.packageset                               = SELECT
 public.sourcepackagename                        = SELECT
 public.sourcepackagepublishinghistory           = SELECT
 public.sourcepackagerelease                     = SELECT

=== modified file 'lib/lp/soyuz/interfaces/packageset.py'
--- lib/lp/soyuz/interfaces/packageset.py	2011-03-29 09:26:33 +0000
+++ lib/lp/soyuz/interfaces/packageset.py	2011-04-04 07:26:34 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # pylint: disable-msg=E0211,E0213
@@ -235,9 +235,10 @@
     @operation_returns_collection_of(Interface)
     @export_read_operation()
     def relatedSets():
-        """Get all package sets related to this one.
+        """Get all other package sets in this set's `PackagesetGroup`.
 
-        Return all package sets that are related to this one.
+        Returns all package sets that are related to this one, but not
+        this one itself.
 
         :return: A (potentially empty) sequence of `IPackageset` instances.
         """

=== modified file 'lib/lp/soyuz/model/distroseriesdifferencejob.py'
--- lib/lp/soyuz/model/distroseriesdifferencejob.py	2011-03-18 03:39:58 +0000
+++ lib/lp/soyuz/model/distroseriesdifferencejob.py	2011-04-04 07:26:34 +0000
@@ -27,6 +27,7 @@
     IDistroSeriesDifferenceJob,
     IDistroSeriesDifferenceJobSource,
     )
+from lp.soyuz.interfaces.packageset import IPackagesetSet
 from lp.soyuz.model.distributionjob import (
     DistributionJob,
     DistributionJobDerived,
@@ -99,6 +100,12 @@
     return find_waiting_jobs(distroseries, sourcepackagename).is_empty()
 
 
+def has_package(distroseries, sourcepackagename):
+    """Does `distroseries` have the given source package?"""
+    return not distroseries.getPublishedSources(
+        sourcepackagename, include_pending=True).is_empty()
+
+
 class DistroSeriesDifferenceJob(DistributionJobDerived):
     """A `Job` type for creating/updating `DistroSeriesDifference`s."""
 
@@ -123,13 +130,42 @@
     def sourcepackagename(self):
         return SourcePackageName.get(self.metadata['sourcepackagename'])
 
+    def passesPackagesetFilter(self):
+        """Is this package of interest as far as packagesets are concerned?
+
+        If the parent series has packagesets, then packages that are
+        missing in the derived series are only of interest if they are
+        in a packageset that the derived series also has.
+        """
+        derived_series = self.distroseries
+        parent_series = derived_series.parent_series
+        if has_package(derived_series, self.sourcepackagename):
+            return True
+        if not has_package(parent_series, self.sourcepackagename):
+            return True
+        packagesetset = getUtility(IPackagesetSet)
+        if packagesetset.getBySeries(parent_series).is_empty():
+            # Parent series does not have packagesets, as would be the
+            # case for e.g. Debian.  In that case, don't filter.
+            return True
+        parent_sets = packagesetset.setsIncludingSource(
+            self.sourcepackagename, distroseries=parent_series)
+        for parent_set in parent_sets:
+            for related_set in parent_set.relatedSets():
+                if related_set.distroseries == derived_series:
+                    return True
+        return False
+
     def run(self):
         """See `IRunnableJob`."""
+        if not self.passesPackagesetFilter():
+            return
+
         store = IMasterStore(DistroSeriesDifference)
         ds_diff = store.find(
-            DistroSeriesDifference, 
+            DistroSeriesDifference,
             DistroSeriesDifference.derived_series == self.distroseries,
-            DistroSeriesDifference.source_package_name == 
+            DistroSeriesDifference.source_package_name ==
             self.sourcepackagename).one()
         if ds_diff is None:
             ds_diff = getUtility(IDistroSeriesDifferenceSource).new(

=== modified file 'lib/lp/soyuz/tests/test_distroseriesdifferencejob.py'
--- lib/lp/soyuz/tests/test_distroseriesdifferencejob.py	2011-03-28 07:29:51 +0000
+++ lib/lp/soyuz/tests/test_distroseriesdifferencejob.py	2011-04-04 07:26:34 +0000
@@ -198,7 +198,7 @@
         self.assertEqual(0, len(jobs))
         store = IMasterStore(DistroSeriesDifference)
         ds_diff = store.find(
-            DistroSeriesDifference, 
+            DistroSeriesDifference,
             DistroSeriesDifference.derived_series == derived_series,
             DistroSeriesDifference.source_package_name == package)
         self.assertEqual(1, ds_diff.count())
@@ -214,7 +214,7 @@
         # The first job would have created a DSD for us.
         store = IMasterStore(DistroSeriesDifference)
         ds_diff = store.find(
-            DistroSeriesDifference, 
+            DistroSeriesDifference,
             DistroSeriesDifference.derived_series == derived_series,
             DistroSeriesDifference.source_package_name == package)
         self.assertEqual(1, ds_diff.count())
@@ -224,11 +224,91 @@
         job[0].start()
         job[0].run()
         ds_diff = store.find(
-            DistroSeriesDifference, 
+            DistroSeriesDifference,
             DistroSeriesDifference.derived_series == derived_series,
             DistroSeriesDifference.source_package_name == package)
         self.assertEqual(1, ds_diff.count())
 
+    def test_packageset_filter_passes_inherited_packages(self):
+        derived_series = self.makeDerivedDistroSeries()
+        parent_series = derived_series.parent_series
+        # Parent must have a packageset or the filter will pass anyway.
+        self.factory.makePackageset(distroseries=parent_series)
+        package = self.factory.makeSourcePackageName()
+        # Package is not in the packageset _but_ both the parent and
+        # derived series both have it.
+        self.factory.makeSourcePackagePublishingHistory(
+            distroseries=parent_series, sourcepackagename=package)
+        self.factory.makeSourcePackagePublishingHistory(
+            distroseries=derived_series, sourcepackagename=package)
+        job = create_job(derived_series, package)
+        self.assertTrue(job.passesPackagesetFilter())
+
+    def test_packageset_filter_passes_packages_unique_to_derived_series(self):
+        derived_series = self.makeDerivedDistroSeries()
+        parent_series = derived_series.parent_series
+        # Parent must have a packageset or the filter will pass anyway.
+        self.factory.makePackageset(distroseries=parent_series)
+        package = self.factory.makeSourcePackageName()
+        # Package exists in the derived series but not in the parent
+        # series.
+        self.factory.makeSourcePackagePublishingHistory(
+            distroseries=derived_series, sourcepackagename=package)
+        job = create_job(derived_series, package)
+        self.assertTrue(job.passesPackagesetFilter())
+
+    def test_packageset_filter_passes_all_if_parent_has_no_packagesets(self):
+        # Debian in particular has no packagesets.  If the parent series
+        # has no packagesets, the packageset filter passes all packages.
+        derived_series = self.makeDerivedDistroSeries()
+        parent_series = derived_series.parent_series
+        package = self.factory.makeSourcePackageName()
+        self.factory.makeSourcePackagePublishingHistory(
+            distroseries=parent_series, sourcepackagename=package)
+        job = create_job(derived_series, package)
+        self.assertTrue(job.passesPackagesetFilter())
+
+    def makeInheritedPackageSet(self, derived_series, packages=()):
+        """Simulate an inherited `Packageset`.
+
+        Creates a packageset in the parent that has an equivalent in
+        `derived_series`.
+        """
+        parent_series = derived_series.parent_series
+        parent_packageset = self.factory.makePackageset(
+            distroseries=parent_series, packages=packages)
+        derived_packageset = self.factory.makePackageset(
+            distroseries=derived_series, packages=packages,
+            name=parent_packageset.name, owner=parent_packageset.owner,
+            related_set=parent_packageset)
+
+    def test_packageset_filter_passes_package_in_inherited_packageset(self):
+        derived_series = self.makeDerivedDistroSeries()
+        parent_series = derived_series.parent_series
+        # Package is in a packageset on the parent that the derived
+        # series also has.
+        package = self.factory.makeSourcePackageName()
+        self.makeInheritedPackageSet(derived_series, [package])
+        # Package is in parent series and in a packageset that the
+        # derived series inherited.
+        self.factory.makeSourcePackagePublishingHistory(
+            distroseries=parent_series, sourcepackagename=package)
+        job = create_job(derived_series, package)
+        self.assertTrue(job.passesPackagesetFilter())
+
+    def test_packageset_filter_blocks_unwanted_parent_package(self):
+        derived_series = self.makeDerivedDistroSeries()
+        parent_series = derived_series.parent_series
+        self.makeInheritedPackageSet(derived_series)
+        package = self.factory.makeSourcePackageName()
+        # Package is in the parent series but not in a packageset shared
+        # between the derived series and the parent series.
+        self.factory.makeSourcePackagePublishingHistory(
+            distroseries=parent_series, sourcepackagename=package)
+        job = create_job(derived_series, package)
+        self.assertFalse(job.passesPackagesetFilter())
+
+
 class TestDistroSeriesDifferenceJobEndToEnd(TestCaseWithFactory):
 
     layer = LaunchpadZopelessLayer
@@ -262,7 +342,7 @@
 
     def findDSD(self, derived_series, source_package_name):
         return self.store.find(
-            DistroSeriesDifference, 
+            DistroSeriesDifference,
             DistroSeriesDifference.derived_series == derived_series,
             DistroSeriesDifference.source_package_name ==
             source_package_name)
@@ -348,7 +428,7 @@
         self.runJob(jobs[0])
         self.assertEqual(
             DistroSeriesDifferenceStatus.RESOLVED, ds_diff[0].status)
-        
+
     def test_only_in_child(self):
         # If a source package only exists in the child distroseries, the DSD
         # is created with the right type.

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2011-03-31 12:53:27 +0000
+++ lib/lp/testing/factory.py	2011-04-04 07:26:34 +0000
@@ -3617,7 +3617,7 @@
         return getUtility(ISectionSet).ensure(name)
 
     def makePackageset(self, name=None, description=None, owner=None,
-                       packages=(), distroseries=None):
+                       packages=(), distroseries=None, related_set=None):
         """Make an `IPackageset`."""
         if name is None:
             name = self.getUniqueString(u'package-set-name')
@@ -3630,7 +3630,8 @@
         ps_set = getUtility(IPackagesetSet)
         package_set = run_with_login(
             techboard.teamowner,
-            lambda: ps_set.new(name, description, owner, distroseries))
+            lambda: ps_set.new(
+                name, description, owner, distroseries, related_set))
         run_with_login(owner, lambda: package_set.add(packages))
         return package_set