launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-request-build-select-channels into lp:launchpad.

Commit message:
Allow selecting source snap channels when requesting manual snap builds.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1791265 in Launchpad itself: "Manual snap builds don't allow for snapcraft/snapd channel selection"
  https://bugs.launchpad.net/launchpad/+bug/1791265

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-request-build-select-channels/+merge/367518
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-request-build-select-channels into lp:launchpad.

=== modified file 'lib/lp/snappy/browser/snap.py'
--- lib/lp/snappy/browser/snap.py	2019-04-01 09:03:46 +0000
+++ lib/lp/snappy/browser/snap.py	2019-05-16 11:33:34 +0000
@@ -29,6 +29,7 @@
 from zope.interface import Interface
 from zope.schema import (
     Choice,
+    Dict,
     List,
     TextLine,
     )
@@ -294,10 +295,14 @@
             description=(
                 u'The package stream within the source distribution series '
                 u'to use when building the snap package.'))
+        channels = Dict(
+            title=u'Source snap channels', key_type=TextLine(), required=True,
+            description=ISnap['auto_build_channels'].description)
 
     custom_widget_archive = SnapArchiveWidget
     custom_widget_distro_arch_series = LabeledMultiCheckBoxWidget
     custom_widget_pocket = LaunchpadDropdownWidget
+    custom_widget_channels = SnapBuildChannelsWidget
 
     help_links = {
         "pocket": u"/+help-snappy/snap-build-pocket.html",
@@ -320,6 +325,7 @@
                 else self.context.distro_series.main_archive),
             'distro_arch_series': [],
             'pocket': PackagePublishingPocket.UPDATES,
+            'channels': self.context.auto_build_channels,
             }
 
     def requestBuild(self, data):
@@ -336,7 +342,8 @@
         for arch in data['distro_arch_series']:
             try:
                 build = self.context.requestBuild(
-                    self.user, data['archive'], arch, data['pocket'])
+                    self.user, data['archive'], arch, data['pocket'],
+                    channels=data['channels'])
                 builds.append(build)
             except SnapBuildAlreadyPending:
                 already_pending.append(arch)
@@ -356,7 +363,8 @@
             self.request.response.addNotification(notification_text)
         else:
             self.context.requestBuilds(
-                self.user, data['archive'], data['pocket'])
+                self.user, data['archive'], data['pocket'],
+                channels=data['channels'])
             self.request.response.addNotification(
                 _('Builds will be dispatched soon.'))
         self.next_url = self.cancel_url

=== modified file 'lib/lp/snappy/browser/tests/test_snap.py'
--- lib/lp/snappy/browser/tests/test_snap.py	2019-04-01 09:03:46 +0000
+++ lib/lp/snappy/browser/tests/test_snap.py	2019-05-16 11:33:34 +0000
@@ -1668,6 +1668,14 @@
             \(\?\)
             The package stream within the source distribution series to use
             when building the snap package.
+            Source snap channels:
+            core
+            snapcraft
+            The channels to use for build tools when building the snap
+            package.
+            If unset, or if the channel for snapcraft is set to "apt", the
+            default is to install snapcraft from the source archive using
+            apt.
             or
             Cancel
             """,
@@ -1717,6 +1725,21 @@
         builds = self.snap.pending_builds
         self.assertEqual([ppa], [build.archive for build in builds])
 
+    def test_request_builds_with_architectures_channels(self):
+        # Selecting different channels with architectures selected creates
+        # builds using those channels.
+        browser = self.getViewBrowser(
+            self.snap, "+request-builds", user=self.person)
+        browser.getControl(name="field.channels.core").value = "edge"
+        browser.getControl("amd64").selected = True
+        self.assertFalse(browser.getControl("i386").selected)
+        browser.getControl("Request builds").click()
+
+        login_person(self.person)
+        builds = self.snap.pending_builds
+        self.assertEqual(
+            [{"core": "edge"}], [build.channels for build in builds])
+
     def test_request_builds_with_architectures_rejects_duplicate(self):
         # A duplicate build request with architectures selected causes a
         # notification.
@@ -1753,7 +1776,7 @@
             _job=MatchesStructure(
                 requester=Equals(self.person),
                 pocket=Equals(PackagePublishingPocket.UPDATES),
-                channels=Is(None))))
+                channels=Equals({}))))
 
     def test_request_builds_no_architectures_ppa(self):
         # Selecting a different archive with no architectures selected
@@ -1772,6 +1795,20 @@
         [request] = self.snap.pending_build_requests
         self.assertEqual(ppa, request.archive)
 
+    def test_request_builds_no_architectures_channels(self):
+        # Selecting different channels with no architectures selected
+        # creates a build request using those channels.
+        browser = self.getViewBrowser(
+            self.snap, "+request-builds", user=self.person)
+        browser.getControl(name="field.channels.core").value = "edge"
+        self.assertFalse(browser.getControl("amd64").selected)
+        self.assertFalse(browser.getControl("i386").selected)
+        browser.getControl("Request builds").click()
+
+        login_person(self.person)
+        [request] = self.snap.pending_build_requests
+        self.assertEqual({"core": "edge"}, request.channels)
+
     def test_request_builds_no_distro_series(self):
         # Requesting builds of a snap configured to infer an appropriate
         # distro series from snapcraft.yaml creates a pending build request.
@@ -1792,4 +1829,4 @@
             _job=MatchesStructure(
                 requester=Equals(self.person),
                 pocket=Equals(PackagePublishingPocket.UPDATES),
-                channels=Is(None))))
+                channels=Equals({}))))

=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py	2019-04-16 13:05:56 +0000
+++ lib/lp/snappy/interfaces/snap.py	2019-05-16 11:33:34 +0000
@@ -332,6 +332,10 @@
         title=u"The source archive for builds produced by this request",
         required=True, readonly=True)
 
+    channels = Dict(
+        title=_("Source snap channels for builds produced by this request"),
+        key_type=TextLine(), required=False, readonly=True)
+
 
 class ISnapView(Interface):
     """`ISnap` attributes that require launchpad.View permission."""

=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2019-04-16 13:05:56 +0000
+++ lib/lp/snappy/model/snap.py	2019-05-16 11:33:34 +0000
@@ -244,6 +244,11 @@
         """See `ISnapBuildRequest`."""
         return self._job.archive
 
+    @property
+    def channels(self):
+        """See `ISnapBuildRequest`."""
+        return self._job.channels
+
 
 @implementer(ISnap, IHasOwner)
 class Snap(Storm, WebhookTargetMixin):
@@ -627,13 +632,18 @@
         if not self._isArchitectureAllowed(distro_arch_series, pocket):
             raise SnapBuildDisallowedArchitecture(distro_arch_series, pocket)
 
+        if not channels:
+            channels_clause = Or(
+                SnapBuild.channels == None, SnapBuild.channels == {})
+        else:
+            channels_clause = SnapBuild.channels == channels
         pending = IStore(self).find(
             SnapBuild,
             SnapBuild.snap_id == self.id,
             SnapBuild.archive_id == archive.id,
             SnapBuild.distro_arch_series_id == distro_arch_series.id,
             SnapBuild.pocket == pocket,
-            SnapBuild.channels == channels,
+            channels_clause,
             SnapBuild.status == BuildStatus.NEEDSBUILD)
         if pending.any() is not None:
             raise SnapBuildAlreadyPending

=== modified file 'lib/lp/snappy/templates/snap-request-builds.pt'
--- lib/lp/snappy/templates/snap-request-builds.pt	2019-02-07 10:34:20 +0000
+++ lib/lp/snappy/templates/snap-request-builds.pt	2019-05-16 11:33:34 +0000
@@ -28,6 +28,9 @@
         <tal:widget define="widget nocall:view/widgets/pocket">
           <metal:block use-macro="context/@@launchpad_form/widget_row" />
         </tal:widget>
+        <tal:widget define="widget nocall:view/widgets/channels">
+          <metal:block use-macro="context/@@launchpad_form/widget_row" />
+        </tal:widget>
       </table>
     </metal:formbody>
   </div>

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2019-04-16 13:05:56 +0000
+++ lib/lp/snappy/tests/test_snap.py	2019-05-16 11:33:34 +0000
@@ -308,6 +308,19 @@
         snap.requestBuild(
             snap.owner, snap.distro_series.main_archive, arches[1],
             PackagePublishingPocket.UPDATES)
+        # channels=None and channels={} are treated as equivalent, but
+        # anything else allows a new build.
+        self.assertRaises(
+            SnapBuildAlreadyPending, snap.requestBuild,
+            snap.owner, snap.distro_series.main_archive, arches[0],
+            PackagePublishingPocket.UPDATES, channels={})
+        snap.requestBuild(
+            snap.owner, snap.distro_series.main_archive, arches[0],
+            PackagePublishingPocket.UPDATES, channels={"core": "edge"})
+        self.assertRaises(
+            SnapBuildAlreadyPending, snap.requestBuild,
+            snap.owner, snap.distro_series.main_archive, arches[0],
+            PackagePublishingPocket.UPDATES, channels={"core": "edge"})
         # Changing the status of the old build allows a new build.
         old_build.updateStatus(BuildStatus.BUILDING)
         old_build.updateStatus(BuildStatus.FULLYBUILT)