Merge lp:~stevenk/launchpad/bpph-supersede into lp:launchpad

[Steve Kowalik]

No longer tolerate supersede() being called on unpublished binary arch-dep publications. This allows us to remove some probable foot-guns from the code, and also brings the source and binary supersede methods into closer alignment.

[lp:~stevenk/launchpad/bpph-supersede]

14656. By Steve Kowalik on 2012-01-09

One assert is not enough, since we need to tolerate SUPERSEDED arch-indep publications.

[Deryck Hodge (deryck)]

As we chatted here in person, I'm not sure about the condition check you do, but I don't understand what you're trying to do here well enough to feel confident voting yay or nay. Do you mind getting someone else to look at it?

review: Abstain

Unmerged revisions

14656. By Steve Kowalik on 2012-01-09

One assert is not enough, since we need to tolerate SUPERSEDED arch-indep publications.

14655. By Steve Kowalik on 2012-01-08

No longer tolerate superseded arch-indep binary publications in IBPPH.supersede()

=== modified file 'lib/lp/soyuz/model/publishing.py'
--- lib/lp/soyuz/model/publishing.py	2011-12-30 06:14:56 +0000
+++ lib/lp/soyuz/model/publishing.py	2012-01-09 14:24:32 +0000
@@ -1134,18 +1134,18 @@
 
     def supersede(self, dominant=None, logger=None):
         """See `IBinaryPackagePublishingHistory`."""
-        # At this point only PUBLISHED (ancient versions) or PENDING (
-        # multiple overrides/copies) publications should be given. We
-        # tolerate SUPERSEDED architecture-independent binaries, because
-        # they are dominated automatically once the first publication is
-        # processed.
+        # If the binary publication is not published, we allow SUPERSEDED
+        # arch-indep publications only.
         if self.status not in active_publishing_status:
-            assert not self.binarypackagerelease.architecturespecific, (
-                "Should not dominate unpublished architecture specific "
-                "binary %s (%s)" % (
-                self.binarypackagerelease.title,
-                self.distroarchseries.architecturetag))
-            return
+            if self.binarypackagerelease.architecturespecific:
+                assert self.status in active_publishing_status + (
+                    PackagePublishingStatus.SUPERSEDED,), (
+                    "Should not dominate unpublished architecture specific "
+                    "binary %s (%s)" % (
+                        self.binarypackagerelease.title,
+                        self.distroarchseries.architecturetag))
+            else:
+                assert "Can not dominate unpublished binary publication."
 
         self.setSuperseded()
 

=== modified file 'lib/lp/soyuz/tests/test_publishing_models.py'
--- lib/lp/soyuz/tests/test_publishing_models.py	2011-12-30 06:14:56 +0000
+++ lib/lp/soyuz/tests/test_publishing_models.py	2012-01-09 14:24:32 +0000
@@ -175,3 +175,12 @@
             binarypackagerelease=bpr, archive=archive)
         expected_urls = self.get_urls_for_binarypackagerelease(bpr, archive)
         self.assertContentEqual(expected_urls, bpph.binaryFileUrls())
+
+    def test_cannot_supersede_superseded_binary_publication(self):
+        # Superseded binary publications cannot be superseded again.
+        bpph = self.factory.makeBinaryPackagePublishingHistory()
+        bpph.supersede()
+        self.assertRaisesWithContent(
+            AssertionError, 
+            "Can not dominate unpublished binary publication.",
+            bpph.supersede)