launchpad-reviewers team mailing list archive

[Colin Watson <mp+408103@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:charmcraft-no-architectures into launchpad:master.

Commit message:
Fix charmcraft.yaml parsing when "architectures" is absent

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/408103

The spec says this can be omitted, so we shouldn't crash in this case.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:charmcraft-no-architectures into launchpad:master.

diff --git a/lib/lp/charms/adapters/buildarch.py b/lib/lp/charms/adapters/buildarch.py
index ac32d86..d6f96fd 100644
--- a/lib/lp/charms/adapters/buildarch.py
+++ b/lib/lp/charms/adapters/buildarch.py
@@ -82,7 +82,9 @@ class CharmBase:
         return not self == other
 
     def __hash__(self):
-        return hash((self.name, self.channel, tuple(self.architectures)))
+        architectures = (
+            None if self.architectures is None else tuple(self.architectures))
+        return hash((self.name, self.channel, architectures))
 
     def __str__(self):
         return "{} {} {}".format(
@@ -165,11 +167,18 @@ def determine_instances_to_build(charmcraft_data, supported_arches,
         # those.
         for build_on in config.build_on:
             for das in supported_arches:
-                if (das.distroseries.distribution.name == build_on.name and
-                        build_on.channel in (
-                            das.distroseries.name,
-                            das.distroseries.version) and
-                        das.architecturetag in build_on.architectures):
+                if das.distroseries.distribution.name != build_on.name:
+                    continue
+                if build_on.channel not in (
+                        das.distroseries.name, das.distroseries.version):
+                    continue
+                if build_on.architectures is None:
+                    # Build on all supported architectures for the requested
+                    # series.
+                    instances[das] = None
+                elif das.architecturetag in build_on.architectures:
+                    # Build on the first matching supported architecture for
+                    # the requested series.
                     instances[das] = None
                     break
             else:
diff --git a/lib/lp/charms/adapters/tests/test_buildarch.py b/lib/lp/charms/adapters/tests/test_buildarch.py
index 569ad02..0ba3aa3 100644
--- a/lib/lp/charms/adapters/tests/test_buildarch.py
+++ b/lib/lp/charms/adapters/tests/test_buildarch.py
@@ -109,7 +109,7 @@ class TestDetermineInstancesToBuild(WithScenarios, TestCaseWithFactory):
     layer = LaunchpadZopelessLayer
 
     # Scenarios taken from the charmcraft build providers specification:
-    # https://docs.google.com/document/d/1Tix0V2J21hfXj-dukgArbTEN8rg31b7za4VM_KylDkc
+    # https://discourse.charmhub.io/t/charmcraft-bases-provider-support/4713
     scenarios = [
         ("single entry, single arch", {
             "bases": [{
@@ -234,6 +234,28 @@ class TestDetermineInstancesToBuild(WithScenarios, TestCaseWithFactory):
                 }],
             "expected": [("20.04", "amd64")],
             }),
+        ("multiple build-on entries", {
+            "bases": [{
+                "build-on": [
+                    {
+                        "name": "ubuntu",
+                        "channel": "18.04",
+                        "architectures": ["amd64"],
+                        },
+                    {
+                        "name": "ubuntu",
+                        "channel": "20.04",
+                        "architectures": ["amd64"],
+                        },
+                    ],
+                "run-on": [{
+                    "name": "ubuntu",
+                    "channel": "20.04",
+                    "architectures": ["amd64"],
+                    }],
+                }],
+            "expected": [("18.04", "amd64")],
+            }),
         ("redundant outputs", {
             "bases": [
                 {
@@ -271,6 +293,14 @@ class TestDetermineInstancesToBuild(WithScenarios, TestCaseWithFactory):
             "expected": [
                 ("20.04", "amd64"), ("20.04", "arm64"), ("20.04", "riscv64")],
             }),
+        ("abbreviated, no architectures specified", {
+            "bases": [{
+                "name": "ubuntu",
+                "channel": "18.04",
+                }],
+            "expected": [
+                ("18.04", "amd64"), ("18.04", "arm64"), ("18.04", "riscv64")],
+            }),
         ]
 
     def test_parser(self):