launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/dspr-latestbuild-archtag into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1403849 in Launchpad itself: "URLs for latest build of a DistributionSourcePackageRelease on a given architecture"
  https://bugs.launchpad.net/launchpad/+bug/1403849

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/dspr-latestbuild-archtag/+merge/245101

http://people.canonical.com/~ubuntu-archive/proposed-migration/update_excuses.html needs to be able to construct links to builds for individual architectures for its "missing build on <arch>" and "old binaries left on <arch>" excuses.  It would be very helpful if it could do this without having to talk to LP to get the +build URLs, and right now it just links to the DSPR to avoid having to do this.  Furthermore, in some cases there may not actually be a build for the architecture in question (perhaps the problem is that the package no longer builds on that architecture but there are still old binaries in -proposed) and in that case we still need to link to the DSPR.

I think the simplest improvement would be to add DSPR:+latestbuild/<archtag>, redirecting to the latest build on the requested architecture if it can find one, otherwise to the DSPR.  This branch does that.  I initially thought about using just DSPR:+build/<archtag>, but it seemed too weird to have that have two entirely different sets of semantics depending on the path element after +build.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/dspr-latestbuild-archtag into lp:launchpad.

=== modified file 'lib/lp/soyuz/browser/distributionsourcepackagerelease.py'
--- lib/lp/soyuz/browser/distributionsourcepackagerelease.py	2014-05-06 09:11:14 +0000
+++ lib/lp/soyuz/browser/distributionsourcepackagerelease.py	2014-12-18 12:02:47 +0000
@@ -21,6 +21,7 @@
 from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.propertycache import cachedproperty
 from lp.services.webapp import (
+    canonical_url,
     LaunchpadView,
     Navigation,
     stepthrough,
@@ -56,6 +57,13 @@
             return None
         return build
 
+    @stepthrough('+latestbuild')
+    def redirect_latestbuild(self, name):
+        build = self.context.getBuildsByArchTag(name).first()
+        if build is not None:
+            return self.redirectSubTree(canonical_url(build))
+        return self.redirectSubTree(canonical_url(self.context))
+
 
 class DistributionSourcePackageReleaseView(LaunchpadView):
     """View logic for `DistributionSourcePackageRelease` objects. """

=== modified file 'lib/lp/soyuz/browser/tests/test_distributionsourcepackagerelease.py'
--- lib/lp/soyuz/browser/tests/test_distributionsourcepackagerelease.py	2014-11-09 01:07:27 +0000
+++ lib/lp/soyuz/browser/tests/test_distributionsourcepackagerelease.py	2014-12-18 12:02:47 +0000
@@ -10,11 +10,55 @@
 
 from zope.security.proxy import removeSecurityProxy
 
+from lp.services.webapp import canonical_url
 from lp.testing import TestCaseWithFactory
-from lp.testing.layers import LaunchpadFunctionalLayer
+from lp.testing.layers import (
+    DatabaseFunctionalLayer,
+    LaunchpadFunctionalLayer,
+    )
+from lp.testing.publication import test_traverse
 from lp.testing.views import create_initialized_view
 
 
+class TestDistributionSourcePackageReleaseNavigation(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUpLatestBuildTests(self):
+        distribution = self.factory.makeDistribution()
+        dses = [
+            self.factory.makeDistroSeries(distribution=distribution)
+            for i in range(2)]
+        spr = self.factory.makeSourcePackageRelease(distroseries=dses[0])
+        proc = self.factory.makeProcessor()
+        builds = []
+        for i in range(2):
+            self.factory.makeSourcePackagePublishingHistory(
+                distroseries=dses[0], sourcepackagerelease=spr)
+            das = self.factory.makeDistroArchSeries(
+                distroseries=dses[i], architecturetag="arch", processor=proc)
+            builds.append(self.factory.makeBinaryPackageBuild(
+                source_package_release=spr, distroarchseries=das))
+        builds.append(
+            self.factory.makeBinaryPackageBuild(source_package_release=spr))
+        return canonical_url(distribution.getSourcePackageRelease(spr)), builds
+
+    def test_latestbuild_known_arch(self):
+        # +latestbuild traverses to the most recent build for the requested
+        # architecture.
+        dspr_url, builds = self.setUpLatestBuildTests()
+        _, view, _ = test_traverse("%s/+latestbuild/arch" % dspr_url)
+        self.assertEqual(
+            canonical_url(builds[1]), removeSecurityProxy(view).target)
+
+    def test_latestbuild_unknown_arch(self):
+        # If there is no build for the requested architecture, +latestbuild
+        # traverses to the context DSPR.
+        dspr_url, _ = self.setUpLatestBuildTests()
+        obj, _, _ = test_traverse("%s/+latestbuild/unknown" % dspr_url)
+        self.assertEqual(dspr_url, canonical_url(obj))
+
+
 class TestDistributionSourcePackageReleaseFiles(TestCaseWithFactory):
     """Source package release files are rendered correctly."""
 

=== modified file 'lib/lp/soyuz/interfaces/distributionsourcepackagerelease.py'
--- lib/lp/soyuz/interfaces/distributionsourcepackagerelease.py	2014-11-09 23:28:27 +0000
+++ lib/lp/soyuz/interfaces/distributionsourcepackagerelease.py	2014-12-18 12:02:47 +0000
@@ -39,6 +39,9 @@
         "be inherited from a parent distribution, not necessarily built "
         "here, but must be published in a main archive.")
 
+    def getBuildsByArchTag(arch_tag):
+        """Return the builds for this SPR on the given architecture."""
+
     sample_binary_packages = Attribute("A single binary package of each "
         "named package produced from this source package in this "
         "distribution. The are each of form DistroSeriesBinaryPackage.")

=== modified file 'lib/lp/soyuz/model/distributionsourcepackagerelease.py'
--- lib/lp/soyuz/model/distributionsourcepackagerelease.py	2014-11-27 20:52:37 +0000
+++ lib/lp/soyuz/model/distributionsourcepackagerelease.py	2014-12-18 12:02:47 +0000
@@ -30,6 +30,7 @@
 from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
 from lp.soyuz.model.binarypackagename import BinaryPackageName
 from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
+from lp.soyuz.model.distroarchseries import DistroArchSeries
 from lp.soyuz.model.distroseriesbinarypackage import DistroSeriesBinaryPackage
 from lp.soyuz.model.publishing import (
     BinaryPackagePublishingHistory,
@@ -90,9 +91,7 @@
         return self.getPublishingHistories(
             self.distribution, [self.sourcepackagerelease])
 
-    @property
-    def builds(self):
-        """See IDistributionSourcePackageRelease."""
+    def _getBuilds(self, clauses=[]):
         # First, get all the builds built in a main archive (this will
         # include new and failed builds.)
         builds_built_in_main_archives = Store.of(self.distribution).find(
@@ -100,7 +99,8 @@
             BinaryPackageBuild.source_package_release ==
                 self.sourcepackagerelease,
             BinaryPackageBuild.archive_id.is_in(
-                self.distribution.all_distro_archive_ids))
+                self.distribution.all_distro_archive_ids),
+            *clauses)
 
         # Next get all the builds that have a binary published in the
         # main archive... this will include many of those in the above
@@ -114,14 +114,27 @@
             BinaryPackagePublishingHistory.binarypackagerelease ==
                 BinaryPackageRelease.id,
             BinaryPackagePublishingHistory.archiveID.is_in(
-                self.distribution.all_distro_archive_ids)).config(
-                    distinct=True)
+                self.distribution.all_distro_archive_ids),
+            *clauses).config(distinct=True)
 
         return builds_built_in_main_archives.union(
             builds_published_in_main_archives).order_by(
                 Desc(BinaryPackageBuild.id))
 
     @property
+    def builds(self):
+        """See IDistributionSourcePackageRelease."""
+        return self._getBuilds()
+
+    def getBuildsByArchTag(self, arch_tag):
+        """See IDistributionSourcePackageRelease."""
+        clauses = [
+            BinaryPackageBuild.distro_arch_series_id == DistroArchSeries.id,
+            DistroArchSeries.architecturetag == arch_tag,
+            ]
+        return self._getBuilds(clauses)
+
+    @property
     def sample_binary_packages(self):
         """See IDistributionSourcePackageRelease."""
         #avoid circular imports.