launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-base-none into lp:launchpad.

Commit message:
Honour "base: none" and "build-base" when requesting snap builds.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1819196 in Launchpad itself: "support for base: none and build-base"
  https://bugs.launchpad.net/launchpad/+bug/1819196

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-base-none/+merge/369251

snapcraft doesn't support these yet, but it seems harmless enough to have LP do so in advance.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-base-none into lp:launchpad.

=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2019-05-16 10:21:14 +0000
+++ lib/lp/snappy/model/snap.py	2019-06-24 14:07:58 +0000
@@ -666,21 +666,26 @@
     def _findBase(snapcraft_data):
         """Find a suitable base for a build."""
         snap_base_set = getUtility(ISnapBaseSet)
-        if "base" in snapcraft_data:
-            snap_base_name = snapcraft_data["base"]
+        snap_base_name = snapcraft_data.get("base")
+        if isinstance(snap_base_name, bytes):
+            snap_base_name = snap_base_name.decode("UTF-8")
+        if snap_base_name == "none":
+            snap_base_name = snapcraft_data.get("build-base")
             if isinstance(snap_base_name, bytes):
                 snap_base_name = snap_base_name.decode("UTF-8")
-            return snap_base_set.getByName(snap_base_name)
+        if snap_base_name is not None:
+            return snap_base_set.getByName(snap_base_name), snap_base_name
         else:
-            return snap_base_set.getDefault()
+            return snap_base_set.getDefault(), snap_base_name
 
-    def _pickDistroSeries(self, snap_base, snapcraft_data):
+    def _pickDistroSeries(self, snap_base, snap_base_name):
         """Pick a suitable `IDistroSeries` for a build."""
         if snap_base is not None:
             return self.distro_series or snap_base.distro_series
         elif self.distro_series is None:
             # A base is mandatory if there's no configured distro series.
-            raise NoSuchSnapBase(snapcraft_data.get("base", "<default>"))
+            raise NoSuchSnapBase(
+                snap_base_name if snap_base_name is not None else "<default>")
         else:
             return self.distro_series
 
@@ -721,8 +726,8 @@
             # Find a suitable SnapBase, and combine it with other
             # configuration to find a suitable distro series and suitable
             # channels.
-            snap_base = self._findBase(snapcraft_data)
-            distro_series = self._pickDistroSeries(snap_base, snapcraft_data)
+            snap_base, snap_base_name = self._findBase(snapcraft_data)
+            distro_series = self._pickDistroSeries(snap_base, snap_base_name)
             channels = self._pickChannels(snap_base, channels=channels)
 
             # Sort by Processor.id for determinism.  This is chosen to be

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2019-06-20 13:16:35 +0000
+++ lib/lp/snappy/tests/test_snap.py	2019-06-24 14:07:58 +0000
@@ -513,28 +513,46 @@
             pocket=Equals(PackagePublishingPocket.UPDATES),
             channels=Equals({"snapcraft": "edge"})))
 
-    def test__findBase(self):
-        snap_base_set = getUtility(ISnapBaseSet)
-        with admin_logged_in():
-            snap_bases = [self.factory.makeSnapBase() for _ in range(2)]
-        for snap_base in snap_bases:
-            self.assertEqual(
-                snap_base,
-                Snap._findBase({"base": snap_base.name}))
-        self.assertRaises(
-            NoSuchSnapBase, Snap._findBase,
-            {"base": "nonexistent"})
-        self.assertIsNone(Snap._findBase({}))
-        with admin_logged_in():
-            snap_base_set.setDefault(snap_bases[0])
-        for snap_base in snap_bases:
-            self.assertEqual(
-                snap_base,
-                Snap._findBase({"base": snap_base.name}))
-        self.assertRaises(
-            NoSuchSnapBase, Snap._findBase,
-            {"base": "nonexistent"})
-        self.assertEqual(snap_bases[0], Snap._findBase({}))
+    def test__findBase_without_default(self):
+        with admin_logged_in():
+            snap_bases = [self.factory.makeSnapBase() for _ in range(2)]
+        for snap_base in snap_bases:
+            self.assertEqual(
+                (snap_base, snap_base.name),
+                Snap._findBase({"base": snap_base.name}))
+            self.assertEqual(
+                (snap_base, snap_base.name),
+                Snap._findBase({"base": "none", "build-base": snap_base.name}))
+        self.assertRaises(
+            NoSuchSnapBase, Snap._findBase,
+            {"base": "nonexistent"})
+        self.assertRaises(
+            NoSuchSnapBase, Snap._findBase,
+            {"base": "none", "build-base": "nonexistent"})
+        self.assertEqual((None, None), Snap._findBase({}))
+        self.assertEqual((None, None), Snap._findBase({"base": "none"}))
+
+    def test__findBase_with_default(self):
+        with admin_logged_in():
+            snap_bases = [self.factory.makeSnapBase() for _ in range(2)]
+        with admin_logged_in():
+            getUtility(ISnapBaseSet).setDefault(snap_bases[0])
+        for snap_base in snap_bases:
+            self.assertEqual(
+                (snap_base, snap_base.name),
+                Snap._findBase({"base": snap_base.name}))
+            self.assertEqual(
+                (snap_base, snap_base.name),
+                Snap._findBase({"base": "none", "build-base": snap_base.name}))
+        self.assertRaises(
+            NoSuchSnapBase, Snap._findBase,
+            {"base": "nonexistent"})
+        self.assertRaises(
+            NoSuchSnapBase, Snap._findBase,
+            {"base": "none", "build-base": "nonexistent"})
+        self.assertEqual((snap_bases[0], None), Snap._findBase({}))
+        self.assertEqual(
+            (snap_bases[0], None), Snap._findBase({"base": "none"}))
 
     def makeRequestBuildsJob(self, arch_tags, git_ref=None):
         distro = self.factory.makeDistribution()