launchpad-reviewers team mailing list archive

[Andrey Fedoseev <mp+428596@xxxxxxxxxxxxxxxxxx>]

Andrey Fedoseev has proposed merging ~andrey-fedoseev/launchpad:snap-arch-multi-build-on into launchpad:master.

Commit message:
Allow duplicate `build-on` for `core22`


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~andrey-fedoseev/launchpad/+git/launchpad/+merge/428596

This allows the following configuration in `snapcraft.yaml` which is legit only for `core22`

architectures:
  - build-on: amd64
  - build-on: amd64
    build-for: [arm64]
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~andrey-fedoseev/launchpad:snap-arch-multi-build-on into launchpad:master.

diff --git a/lib/lp/snappy/adapters/buildarch.py b/lib/lp/snappy/adapters/buildarch.py
index e4e92f7..9193038 100644
--- a/lib/lp/snappy/adapters/buildarch.py
+++ b/lib/lp/snappy/adapters/buildarch.py
@@ -146,11 +146,13 @@ class SnapBuildInstance:
 
 
 def determine_architectures_to_build(
+    snap_base: Optional[str],
     snapcraft_data: Dict[str, Any],
     supported_arches: List[str],
 ) -> List[SnapBuildInstance]:
     """Return a list of architectures to build based on snapcraft.yaml.
 
+    :param snap_base: Name of the snap base.
     :param snapcraft_data: A parsed snapcraft.yaml.
     :param supported_arches: An ordered list of all architecture tags that
         we can create builds for.
@@ -182,16 +184,17 @@ def determine_architectures_to_build(
             SnapArchitecture(build_on=a) for a in supported_arches
         ]
 
-    # Ensure that multiple `build-on` items don't include the same
-    # architecture; this is ambiguous and forbidden by snapcraft.  Checking
-    # this here means that we don't get duplicate supported_arch results
-    # below.
-    build_ons = Counter()
-    for arch in architectures:
-        build_ons.update(arch.build_on)
-    duplicates = {arch for arch, count in build_ons.items() if count > 1}
-    if duplicates:
-        raise DuplicateBuildOnError(duplicates)
+    if snap_base not in {"core22"}:
+        # Ensure that multiple `build-on` items don't include the same
+        # architecture; this is ambiguous and forbidden by snapcraft prior
+        # to core22. Checking this here means that we don't get duplicate
+        # supported_arch results below.
+        build_ons = Counter()
+        for arch in architectures:
+            build_ons.update(arch.build_on)
+        duplicates = {arch for arch, count in build_ons.items() if count > 1}
+        if duplicates:
+            raise DuplicateBuildOnError(duplicates)
 
     architectures_to_build = []
     for arch in architectures:
diff --git a/lib/lp/snappy/adapters/tests/test_buildarch.py b/lib/lp/snappy/adapters/tests/test_buildarch.py
index 81f3c87..db52baf 100644
--- a/lib/lp/snappy/adapters/tests/test_buildarch.py
+++ b/lib/lp/snappy/adapters/tests/test_buildarch.py
@@ -5,6 +5,7 @@ from testscenarios import WithScenarios, load_tests_apply_scenarios
 from testtools.matchers import HasLength
 
 from lp.snappy.adapters.buildarch import (
+    DuplicateBuildOnError,
     SnapArchitecture,
     SnapBuildInstance,
     UnsupportedBuildOnError,
@@ -392,16 +393,60 @@ class TestDetermineArchitecturesToBuild(WithScenarios, TestCase):
                 ],
             },
         ),
+        (
+            "multiple build-for for the same build-on",
+            {
+                "architectures": [
+                    {"build-on": "amd64", "build-for": ["amd64"]},
+                    {"build-on": "amd64", "build-for": ["i386"]},
+                ],
+                "supported_architectures": ["amd64", "i386", "armhf"],
+                "expected": [
+                    {
+                        "architecture": "amd64",
+                        "target_architectures": ["amd64"],
+                        "required": True,
+                    },
+                    {
+                        "architecture": "amd64",
+                        "target_architectures": ["i386"],
+                        "required": True,
+                    },
+                ],
+            },
+        ),
+        (
+            "multiple build-for for the same build-on: old base",
+            {
+                "snap_base": "core20",
+                "architectures": [
+                    {"build-on": "amd64", "build-for": ["amd64"]},
+                    {"build-on": "amd64", "build-for": ["i386"]},
+                ],
+                "supported_architectures": ["amd64", "i386", "armhf"],
+                "expected_exception": DuplicateBuildOnError,
+            },
+        ),
     ]
 
     def test_parser(self):
         snapcraft_data = {"architectures": self.architectures}
-        build_instances = determine_architectures_to_build(
-            snapcraft_data, self.supported_architectures
-        )
-        self.assertThat(build_instances, HasLength(len(self.expected)))
-        for instance in build_instances:
-            self.assertIn(instance.__dict__, self.expected)
+        snap_base = getattr(self, "snap_base", "core22")
+        if hasattr(self, "expected_exception"):
+            self.assertRaises(
+                self.expected_exception,
+                determine_architectures_to_build,
+                snap_base,
+                snapcraft_data,
+                self.supported_architectures,
+            )
+        else:
+            build_instances = determine_architectures_to_build(
+                snap_base, snapcraft_data, self.supported_architectures
+            )
+            self.assertThat(build_instances, HasLength(len(self.expected)))
+            for instance in build_instances:
+                self.assertIn(instance.__dict__, self.expected)
 
 
 load_tests = load_tests_apply_scenarios
diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
index 28e1d28..e5fef93 100644
--- a/lib/lp/snappy/model/snap.py
+++ b/lib/lp/snappy/model/snap.py
@@ -175,6 +175,7 @@ from lp.snappy.interfaces.snapbuild import ISnapBuild, ISnapBuildSet
 from lp.snappy.interfaces.snapjob import ISnapRequestBuildsJobSource
 from lp.snappy.interfaces.snappyseries import ISnappyDistroSeriesSet
 from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient
+from lp.snappy.model.snapbase import SnapBase
 from lp.snappy.model.snapbuild import SnapBuild
 from lp.snappy.model.snapjob import SnapJob
 from lp.snappy.model.snapsubscription import SnapSubscription
@@ -898,7 +899,9 @@ class Snap(Storm, WebhookTargetMixin):
         return self.getBuildRequest(job.job_id)
 
     @staticmethod
-    def _findBase(snapcraft_data):
+    def _findBase(
+        snapcraft_data: t.Dict[str, t.Any]
+    ) -> t.Tuple[SnapBase, t.Optional[str]]:
         """Find a suitable base for a build."""
         base = snapcraft_data.get("base")
         build_base = snapcraft_data.get("build-base")
@@ -1006,7 +1009,9 @@ class Snap(Storm, WebhookTargetMixin):
                 )
             )
             architectures_to_build = determine_architectures_to_build(
-                snapcraft_data, list(supported_arches.keys())
+                snap_base.name if snap_base is not None else None,
+                snapcraft_data,
+                list(supported_arches.keys()),
             )
         except Exception as e:
             if not allow_failures: