launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04898
[Merge] lp:~jtv/launchpad/re-830890 into lp:launchpad
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/re-830890 into lp:launchpad with lp:~jtv/launchpad/bug-832647 as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #830890 in Launchpad itself: "Gina should delete publications if removed from source archive"
https://bugs.launchpad.net/launchpad/+bug/830890
For more details, see:
https://code.launchpad.net/~jtv/launchpad/re-830890/+merge/74552
= Summary =
We want Gina to run full, proper, new-style domination on packages that are no longer found in the Sources lists it imports. This will cause any published versions of these packages in our archive to be marked Deleted.
There is a legacy problem, however.
Gina has never done any domination in the past. This results in lots and lots of Debian source package publications in our database being marked Pending. The domination we have in mind as the permanent mechanism would send all of those straight to Deleted, as if all those versions were kept in Debian Sources lists until they were all suddenly deleted one day.
== Proposed fix ==
Transitional domination: for packages that aren't in the Sources list at all, pretend that the latest version we have in the database is still in the Sources list. This will cause all its older releases to be marked as superseded by the newer one. The newer one will stay active. Then, once this is done for the legacy cases, we can switch the code over to the permanet, deleting style of domination.
== Pre-implementation notes ==
Discussed in detail with William. There are a few points to pay attention to:
* This deals with source publications only. It'll be easier to extend this to binary publications (if needed) once we have the legacy out of the way. We do not import binaries for Debian so the legacy issue does not pertain there.
* You won't see extensive superseded-by chains for deleted packages. The last known version will be considered to have superseded all previously existing versions of the deleted package.
* Only Published publications will be dominated. We still need to update Gina to produce Published publications rather than Pending ones (which was never very appropriate), and we still need to update the existing publications in the database.
== Implementation details ==
The loop that used to go over all packages in the Sources list (thus skipping the deleted packages) now iterates over all published packages in the relevant context. The special case for transitional domination is expressed as a special input to the regular domination method. Once we retire transitional domination, the special case can go and the regular domination method will do the right thing. The “entries = packages_map.src_map.get(package_name)” will become “entries = packages_map.src_map.get(package_name, [])” and removal of the transitional code follows from there.
== Tests ==
{{{
./bin/test -vvc lp.archivepublisher -t dominat
./bin/test -vvc lp.soyuz -t gina
}}}
== Demo and Q/A ==
The publish-ftpmaster script should still dominate packages as before.
Gina is harder. Update SourcePackagePublishingHistory statuses in a test version of Debian from Pending to Published. Gina should now dominate packages, meaning that all obsolete publications will be in Superseded status — except for the last known revision of any package that is no longer in the imported Sources lists.
You could also try creating a Published SPPH for Debian, with a fake and an unrealistically high version number. Gina should now mark it as Deleted.
= Launchpad lint =
Some pre-existing lint remains, but I didn't add any. Actually I didn't even touch the files it's in in this branch; that was all done in a branch that this one depends on (and again, all pre-existing, better state than I found it in etc.).
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/archivepublisher/domination.py
lib/lp/soyuz/scripts/gina/retire.py
lib/lp/soyuz/model/publishing.py
lib/lp/soyuz/interfaces/publishing.py
scripts/gina.py
lib/lp/archivepublisher/tests/test_dominator.py
lib/lp/soyuz/scripts/tests/test_gina.py
./lib/lp/soyuz/interfaces/publishing.py
381: E261 at least two spaces before inline comment
478: E261 at least two spaces before inline comment
511: E261 at least two spaces before inline comment
681: E261 at least two spaces before inline comment
767: E261 at least two spaces before inline comment
./scripts/gina.py
26: '_pythonpath' imported but unused
--
https://code.launchpad.net/~jtv/launchpad/re-830890/+merge/74552
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/re-830890 into lp:launchpad.
=== modified file 'lib/lp/archivepublisher/domination.py'
--- lib/lp/archivepublisher/domination.py 2011-09-08 07:39:24 +0000
+++ lib/lp/archivepublisher/domination.py 2011-09-08 07:39:25 +0000
@@ -495,6 +495,27 @@
self._sortPackages(sources, generalization), generalization)
flush_database_updates()
+ def findPublishedSourcePackageNames(self, distroseries, pocket):
+ """Find names of currently published source packages."""
+ result = IStore(SourcePackageName).find(
+ SourcePackageName.name,
+ join_spph_spr(),
+ join_spr_spn(),
+ self._composeActiveSourcePubsCondition(distroseries, pocket))
+ return result.config(distinct=True)
+
+ def findPublishedSPPHs(self, distroseries, pocket, package_name):
+ """Find currently published source publications for given package."""
+ # Avoid circular imports.
+ from lp.soyuz.model.publishing import SourcePackagePublishingHistory
+
+ return IStore(SourcePackagePublishingHistory).find(
+ SourcePackagePublishingHistory,
+ join_spph_spr(),
+ join_spr_spn(),
+ SourcePackageName.name == package_name,
+ self._composeActiveSourcePubsCondition(distroseries, pocket))
+
def dominateRemovedSourceVersions(self, distroseries, pocket,
package_name, live_versions):
"""Dominate source publications based on a set of "live" versions.
@@ -512,19 +533,9 @@
:param live_versions: Iterable of all version strings that are to
remain active.
"""
- # Avoid circular imports.
- from lp.soyuz.model.publishing import SourcePackagePublishingHistory
-
generalization = GeneralizedPublication(is_source=True)
-
- package_pubs = IStore(SourcePackagePublishingHistory).find(
- SourcePackagePublishingHistory,
- join_spph_spr(),
- join_spr_spn(),
- SourcePackageName.name == package_name,
- self._composeActiveSourcePubsCondition(distroseries, pocket))
-
- self.dominatePackage(package_pubs, live_versions, generalization)
+ pubs = self.findPublishedSPPHs(distroseries, pocket, package_name)
+ self.dominatePackage(pubs, live_versions, generalization)
def judge(self, distroseries, pocket):
"""Judge superseded sources and binaries."""
=== modified file 'lib/lp/archivepublisher/tests/test_dominator.py'
--- lib/lp/archivepublisher/tests/test_dominator.py 2011-09-08 07:39:24 +0000
+++ lib/lp/archivepublisher/tests/test_dominator.py 2011-09-08 07:39:25 +0000
@@ -363,6 +363,7 @@
layer = ZopelessDatabaseLayer
def makeDominator(self, publications):
+ """Create a `Dominator` suitable for `publications`."""
if len(publications) == 0:
archive = self.factory.makeArchive()
else:
@@ -469,3 +470,144 @@
self.makeDominator(pubs).dominateRemovedSourceVersions(
pubs[0].distroseries, pubs[0].pocket, other_package_name, ['1.1'])
self.assertEqual(PackagePublishingStatus.PUBLISHED, pubs[0].status)
+
+ def test_findPublishedSourcePackageNames_finds_package(self):
+ spph = self.factory.makeSourcePackagePublishingHistory(
+ status=PackagePublishingStatus.PUBLISHED)
+ dominator = self.makeDominator([spph])
+ self.assertContentEqual(
+ [spph.sourcepackagerelease.sourcepackagename.name],
+ dominator.findPublishedSourcePackageNames(
+ spph.distroseries, spph.pocket))
+
+ def test_findPublishedSourcePackageNames_ignores_other_states(self):
+ series = self.factory.makeDistroSeries()
+ pocket = PackagePublishingPocket.RELEASE
+ spphs = dict(
+ (status, self.factory.makeSourcePackagePublishingHistory(
+ distroseries=series, archive=series.main_archive,
+ pocket=pocket, status=status))
+ for status in PackagePublishingStatus.items)
+ published_spph = spphs[PackagePublishingStatus.PUBLISHED]
+ dominator = self.makeDominator(spphs.values())
+ self.assertContentEqual(
+ [published_spph.sourcepackagerelease.sourcepackagename.name],
+ dominator.findPublishedSourcePackageNames(series, pocket))
+
+ def test_findPublishedSourcePackageNames_ignores_other_archives(self):
+ spph = self.factory.makeSourcePackagePublishingHistory(
+ status=PackagePublishingStatus.PUBLISHED)
+ dominator = self.makeDominator([spph])
+ dominator.archive = self.factory.makeArchive()
+ self.assertContentEqual(
+ [],
+ dominator.findPublishedSourcePackageNames(
+ spph.distroseries, spph.pocket))
+
+ def test_findPublishedSourcePackageNames_ignores_other_series(self):
+ spph = self.factory.makeSourcePackagePublishingHistory(
+ status=PackagePublishingStatus.PUBLISHED)
+ distro = spph.distroseries.distribution
+ other_series = self.factory.makeDistroSeries(distribution=distro)
+ dominator = self.makeDominator([spph])
+ self.assertContentEqual(
+ [],
+ dominator.findPublishedSourcePackageNames(
+ other_series, spph.pocket))
+
+ def test_findPublishedSourcePackageNames_ignores_other_pockets(self):
+ spph = self.factory.makeSourcePackagePublishingHistory(
+ status=PackagePublishingStatus.PUBLISHED,
+ pocket=PackagePublishingPocket.RELEASE)
+ dominator = self.makeDominator([spph])
+ self.assertContentEqual(
+ [],
+ dominator.findPublishedSourcePackageNames(
+ spph.distroseries, PackagePublishingPocket.SECURITY))
+
+ def test_findPublishedSourcePackageNames_does_not_return_duplicates(self):
+ series = self.factory.makeDistroSeries()
+ pocket = PackagePublishingPocket.RELEASE
+ package = self.factory.makeSourcePackageName()
+ spphs = [
+ self.factory.makeSourcePackagePublishingHistory(
+ distroseries=series, archive=series.main_archive,
+ pocket=pocket, status=PackagePublishingStatus.PUBLISHED,
+ sourcepackagerelease=self.factory.makeSourcePackageRelease(
+ sourcepackagename=package))
+ for counter in xrange(2)]
+ dominator = self.makeDominator(spphs)
+ self.assertEqual(
+ [package.name],
+ list(dominator.findPublishedSourcePackageNames(series, pocket)))
+
+ def test_findPublishedSPPHs_finds_published_SPPH(self):
+ spph = self.factory.makeSourcePackagePublishingHistory(
+ status=PackagePublishingStatus.PUBLISHED)
+ package_name = spph.sourcepackagerelease.sourcepackagename.name
+ dominator = self.makeDominator([spph])
+ self.assertContentEqual(
+ [spph],
+ dominator.findPublishedSPPHs(
+ spph.distroseries, spph.pocket, package_name))
+
+ def test_findPublishedSPPHs_ignores_other_states(self):
+ series = self.factory.makeDistroSeries()
+ package = self.factory.makeSourcePackageName()
+ pocket = PackagePublishingPocket.RELEASE
+ spphs = dict(
+ (status, self.factory.makeSourcePackagePublishingHistory(
+ distroseries=series, archive=series.main_archive,
+ pocket=pocket, status=status,
+ sourcepackagerelease=self.factory.makeSourcePackageRelease(
+ sourcepackagename=package)))
+ for status in PackagePublishingStatus.items)
+ dominator = self.makeDominator(spphs.values())
+ self.assertContentEqual(
+ [spphs[PackagePublishingStatus.PUBLISHED]],
+ dominator.findPublishedSPPHs(series, pocket, package.name))
+
+ def test_findPublishedSPPHs_ignores_other_archives(self):
+ spph = self.factory.makeSourcePackagePublishingHistory(
+ status=PackagePublishingStatus.PUBLISHED)
+ package = spph.sourcepackagerelease.sourcepackagename
+ dominator = self.makeDominator([spph])
+ dominator.archive = self.factory.makeArchive()
+ self.assertContentEqual(
+ [],
+ dominator.findPublishedSPPHs(
+ spph.distroseries, spph.pocket, package.name))
+
+ def test_findPublishedSPPHs_ignores_other_series(self):
+ spph = self.factory.makeSourcePackagePublishingHistory(
+ status=PackagePublishingStatus.PUBLISHED)
+ distro = spph.distroseries.distribution
+ package = spph.sourcepackagerelease.sourcepackagename
+ other_series = self.factory.makeDistroSeries(distribution=distro)
+ dominator = self.makeDominator([spph])
+ self.assertContentEqual(
+ [],
+ dominator.findPublishedSPPHs(
+ other_series, spph.pocket, package.name))
+
+ def test_findPublishedSPPHs_ignores_other_pockets(self):
+ spph = self.factory.makeSourcePackagePublishingHistory(
+ status=PackagePublishingStatus.PUBLISHED,
+ pocket=PackagePublishingPocket.RELEASE)
+ package = spph.sourcepackagerelease.sourcepackagename
+ dominator = self.makeDominator([spph])
+ self.assertContentEqual(
+ [],
+ dominator.findPublishedSPPHs(
+ spph.distroseries, PackagePublishingPocket.SECURITY,
+ package.name))
+
+ def test_findPublishedSPPHs_ignores_other_packages(self):
+ spph = self.factory.makeSourcePackagePublishingHistory(
+ status=PackagePublishingStatus.PUBLISHED)
+ other_package = self.factory.makeSourcePackageName()
+ dominator = self.makeDominator([spph])
+ self.assertContentEqual(
+ [],
+ dominator.findPublishedSPPHs(
+ spph.distroseries, spph.pocket, other_package.name))
=== modified file 'lib/lp/soyuz/scripts/gina/retire.py'
--- lib/lp/soyuz/scripts/gina/retire.py 2011-09-08 07:39:24 +0000
+++ lib/lp/soyuz/scripts/gina/retire.py 2011-09-08 07:39:25 +0000
@@ -10,7 +10,14 @@
from zope.component import getUtility
-from lp.archivepublisher.domination import Dominator
+# XXX JeroenVermeulen 2011-09-08, bug=844550: The GeneralizedPublication
+# import violates import policy and elicits a warning from the test
+# suite. The warning helps remind us to retire this code as soon as
+# possible.
+from lp.archivepublisher.domination import (
+ Dominator,
+ GeneralizedPublication,
+ )
from lp.registry.interfaces.distribution import IDistributionSet
@@ -19,9 +26,39 @@
"""Perform domination."""
series = getUtility(IDistributionSet)[distro_name].getSeries(series_name)
dominator = Dominator(logger, series.main_archive)
- for package_name, entries in packages_map.src_map.iteritems():
- live_versions = [
- entry['Version']
- for entry in entries if 'Version' in entry]
+
+ # Dominate packages found in the Sources list we're importing.
+ package_names = dominator.findPublishedSourcePackageNames(series, pocket)
+ for package_name in package_names:
+ entries = packages_map.src_map.get(package_name)
+
+ if entries is None:
+ # XXX JeroenVermeulen 2011-09-08, bug=844550: This is a
+ # transitional hack. The database is full of "Published"
+ # Debian SPPHs whose packages have actually been deleted.
+ # In the future such publications should simply be marked
+ # Deleted, but for the legacy baggage we currently carry
+ # around we'll just do traditional domination first: pick
+ # the latest Published version, and mark the rest of the
+ # SPPHs as superseded by that version. The latest version
+ # will then, finally, be marked appropriately Deleted once
+ # we remove this transitional hack.
+ # To remove the transitional hack, just let live_versions
+ # default to the empty list instead of doing this:
+ pubs = dominator.findPublishedSPPHs(series, pocket, package_name)
+ generalization = GeneralizedPublication(is_source=True)
+ pubs_dict = dominator._sortPackages(pubs, generalization)
+ sorted_pubs = pubs_dict[package_name]
+ if len(sorted_pubs) <= 1:
+ # If there's only one published SPPH, the transitional
+ # code will just leave it Published. Don't bother; the
+ # migration will be costly enough as it is.
+ continue
+ live_versions = [sorted_pubs[0].sourcepackagerelease.version]
+ else:
+ live_versions = [
+ entry['Version']
+ for entry in entries if 'Version' in entry]
+
dominator.dominateRemovedSourceVersions(
series, pocket, package_name, live_versions)
=== modified file 'lib/lp/soyuz/scripts/tests/test_gina.py'
--- lib/lp/soyuz/scripts/tests/test_gina.py 2011-09-08 07:39:24 +0000
+++ lib/lp/soyuz/scripts/tests/test_gina.py 2011-09-08 07:39:25 +0000
@@ -5,6 +5,7 @@
from unittest import TestLoader
from canonical.testing.layers import ZopelessDatabaseLayer
+from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.services.log.logger import DevNullLogger
from lp.soyuz.enums import PackagePublishingStatus
import lp.soyuz.scripts.gina.handlers
@@ -12,28 +13,62 @@
from lp.testing import TestCaseWithFactory
+class FakePackagesMap:
+ def __init__(self, src_map):
+ self.src_map = src_map
+
+
class TestGina(TestCaseWithFactory):
layer = ZopelessDatabaseLayer
- def test_dominate_imported_source_packages(self):
-
- class SimpleFakePackagesMap:
- def __init__(self, src_map):
- self.src_map = src_map
-
+ def test_dominate_imported_source_packages_dominates_imports(self):
+ # dominate_imported_source_packages dominates the source
+ # packages that Gina imports.
logger = DevNullLogger()
pub = self.factory.makeSourcePackagePublishingHistory(
status=PackagePublishingStatus.PUBLISHED)
series = pub.distroseries
spr = pub.sourcepackagerelease
package = spr.sourcepackagename
- packages_map = SimpleFakePackagesMap({package.name: []})
dominate_imported_source_packages(
logger, series.distribution.name, series.name, pub.pocket,
- packages_map)
+ FakePackagesMap({package.name: []}))
self.assertEqual(PackagePublishingStatus.DELETED, pub.status)
+ def test_dominate_imported_source_packages_dominates_deletions(self):
+ # dominate_imported_source_packages dominates the source
+ # packages that have been deleted from the Sources lists that
+ # Gina imports.
+ series = self.factory.makeDistroSeries()
+ pocket = PackagePublishingPocket.RELEASE
+ package = self.factory.makeSourcePackageName()
+ pubs = [
+ self.factory.makeSourcePackagePublishingHistory(
+ archive=series.main_archive, distroseries=series,
+ pocket=pocket, status=PackagePublishingStatus.PUBLISHED,
+ sourcepackagerelease=self.factory.makeSourcePackageRelease(
+ sourcepackagename=package, version=version))
+ for version in ['1.0', '1.1', '1.1a']]
+ logger = DevNullLogger()
+ dominate_imported_source_packages(
+ logger, series.distribution.name, series.name, pocket,
+ FakePackagesMap({}))
+ # XXX JeroenVermeulen 2011-09-08, bug=844550: This is
+ # "transitional" domination which supersedes older versions of
+ # deleted packages with the last known version. Permanent
+ # domination will then mark the last known version as deleted.
+ # For permanent domination, the expected outcome is that all
+ # these publications will be Deleted (but any pre-existing
+ # Superseded publications for older versions will remain
+ # Superseded).
+ self.assertEqual([
+ PackagePublishingStatus.SUPERSEDED,
+ PackagePublishingStatus.SUPERSEDED,
+ PackagePublishingStatus.PUBLISHED,
+ ],
+ [pub.status for pub in pubs])
+
def test_suite():
suite = TestLoader().loadTestsFromName(__name__)
Follow ups
