Merge lp:~cjwatson/launchpad/snap-parse-architectures into lp:launchpad

[Colin Watson]

Implement determine_architectures_to_build for snaps.

This is based on https://github.com/kyrofa/snapcraft-architectures-parser, and is the first step towards full build-on architectures support. I'm still working on the code to actually use this, but it's far enough along for me to be confident that the interface is the right shape.

The code is quite significantly refactored from Kyle's version, to make it easier to fit into Launchpad.

[William Grant (wgrant)]

review: Approve (code)

=== added directory 'lib/lp/snappy/adapters'
=== added file 'lib/lp/snappy/adapters/__init__.py'
=== added file 'lib/lp/snappy/adapters/buildarch.py'
--- lib/lp/snappy/adapters/buildarch.py	1970-01-01 00:00:00 +0000
+++ lib/lp/snappy/adapters/buildarch.py	2018-06-14 17:14:47 +0000
@@ -0,0 +1,176 @@
+# Copyright 2018 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'determine_architectures_to_build',
+    ]
+
+from collections import Counter
+
+import six
+
+from lp.services.helpers import english_list
+
+
+class SnapArchitecturesParserError(Exception):
+    """Base class for all exceptions in this module."""
+
+
+class MissingPropertyError(SnapArchitecturesParserError):
+    """Error for when an expected property is not present in the YAML."""
+
+    def __init__(self, prop):
+        super(MissingPropertyError, self).__init__(
+            "Architecture specification is missing the {!r} property".format(
+                prop))
+        self.property = prop
+
+
+class IncompatibleArchitecturesStyleError(SnapArchitecturesParserError):
+    """Error for when architectures mix incompatible styles."""
+
+    def __init__(self):
+        super(IncompatibleArchitecturesStyleError, self).__init__(
+            "'architectures' must either be a list of strings or dicts, not "
+            "both")
+
+
+class DuplicateBuildOnError(SnapArchitecturesParserError):
+    """Error for when multiple `build-on`s include the same architecture."""
+
+    def __init__(self, duplicates):
+        super(DuplicateBuildOnError, self).__init__(
+            "{} {} present in the 'build-on' of multiple items".format(
+                english_list(duplicates),
+                "is" if len(duplicates) == 1 else "are"))
+
+
+class UnsupportedBuildOnError(SnapArchitecturesParserError):
+    """Error for when a requested architecture is not supported."""
+
+    def __init__(self, build_on):
+        super(UnsupportedBuildOnError, self).__init__(
+            "build-on specifies no supported architectures: {!r}".format(
+                build_on))
+        self.build_on = build_on
+
+
+class SnapArchitecture:
+    """A single entry in the snapcraft.yaml 'architectures' list."""
+
+    def __init__(self, build_on, run_on=None, build_error=None):
+        """Create a new architecture entry.
+
+        :param build_on: string or list; build-on property from
+            snapcraft.yaml.
+        :param run_on: string or list; run-on property from snapcraft.yaml
+            (defaults to build_on).
+        :param build_error: string; build-error property from
+            snapcraft.yaml.
+        """
+        self.build_on = (
+            [build_on] if isinstance(build_on, six.string_types) else build_on)
+        if run_on:
+            self.run_on = (
+                [run_on] if isinstance(run_on, six.string_types) else run_on)
+        else:
+            self.run_on = self.build_on
+        self.build_error = build_error
+
+    @classmethod
+    def from_dict(cls, properties):
+        """Create a new architecture entry from a dict."""
+        try:
+            build_on = properties["build-on"]
+        except KeyError:
+            raise MissingPropertyError("build-on")
+
+        return cls(
+            build_on=build_on, run_on=properties.get("run-on"),
+            build_error=properties.get("build-error"))
+
+
+class SnapBuildInstance:
+    """A single instance of a snap that should be built.
+
+    It has two useful attributes:
+
+      - architecture: The architecture tag that should be used to build the
+            snap.
+      - required: Whether or not failure to build should cause the entire
+            set to fail.
+    """
+
+    def __init__(self, architecture, supported_architectures):
+        """Construct a new `SnapBuildInstance`.
+
+        :param architecture: `SnapArchitecture` instance.
+        :param supported_architectures: List of supported architectures,
+            sorted by priority.
+        """
+        try:
+            self.architecture = next(
+                arch for arch in supported_architectures
+                if arch in architecture.build_on)
+        except StopIteration:
+            raise UnsupportedBuildOnError(architecture.build_on)
+
+        self.required = architecture.build_error != "ignore"
+
+
+def determine_architectures_to_build(snapcraft_data, supported_arches):
+    """Return a set of architectures to build based on snapcraft.yaml.
+
+    :param snapcraft_data: A parsed snapcraft.yaml.
+    :param supported_arches: An ordered list of all architecture tags that
+        we can create builds for.
+    :return: a set of `SnapBuildInstance`s.
+    """
+    architectures_list = snapcraft_data.get("architectures")
+
+    if architectures_list:
+        # First, determine what style we're parsing.  Is it a list of
+        # strings or a list of dicts?
+        if all(isinstance(a, six.string_types) for a in architectures_list):
+            # If a list of strings (old style), then that's only a single
+            # item.
+            architectures = [SnapArchitecture(build_on=architectures_list)]
+        elif all(isinstance(arch, dict) for arch in architectures_list):
+            # If a list of dicts (new style), then that's multiple items.
+            architectures = [
+                SnapArchitecture.from_dict(a) for a in architectures_list]
+        else:
+            # If a mix of both, bail.  We can't reasonably handle it.
+            raise IncompatibleArchitecturesStyleError()
+    else:
+        # If no architectures are specified, build one for each supported
+        # architecture.
+        architectures = [
+            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)
+
+    architectures_to_build = set()
+    for arch in architectures:
+        try:
+            architectures_to_build.add(
+                SnapBuildInstance(arch, supported_arches))
+        except UnsupportedBuildOnError:
+            # Snaps are allowed to declare that they build on architectures
+            # that Launchpad doesn't currently support (perhaps they're
+            # upcoming, or perhaps they used to be supported).  We just
+            # ignore those.
+            pass
+    return architectures_to_build

=== added directory 'lib/lp/snappy/adapters/tests'
=== added file 'lib/lp/snappy/adapters/tests/__init__.py'
=== added file 'lib/lp/snappy/adapters/tests/test_buildarch.py'
--- lib/lp/snappy/adapters/tests/test_buildarch.py	1970-01-01 00:00:00 +0000
+++ lib/lp/snappy/adapters/tests/test_buildarch.py	2018-06-14 17:14:47 +0000
@@ -0,0 +1,231 @@
+# Copyright 2018 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+from testscenarios import (
+    load_tests_apply_scenarios,
+    WithScenarios,
+    )
+from testtools.matchers import HasLength
+
+from lp.snappy.adapters.buildarch import (
+    determine_architectures_to_build,
+    SnapArchitecture,
+    SnapBuildInstance,
+    UnsupportedBuildOnError,
+    )
+from lp.testing import TestCase
+
+
+class TestSnapArchitecture(WithScenarios, TestCase):
+
+    scenarios = [
+        ("lists", {
+            "architectures": {"build-on": ["amd64"], "run-on": ["amd64"]},
+            "expected_build_on": ["amd64"],
+            "expected_run_on": ["amd64"],
+            "expected_build_error": None,
+            }),
+        ("strings", {
+            "architectures": {"build-on": "amd64", "run-on": "amd64"},
+            "expected_build_on": ["amd64"],
+            "expected_run_on": ["amd64"],
+            "expected_build_error": None,
+            }),
+        ("no run-on", {
+            "architectures": {"build-on": ["amd64"]},
+            "expected_build_on": ["amd64"],
+            "expected_run_on": ["amd64"],
+            "expected_build_error": None,
+            }),
+        ("not required", {
+            "architectures": {
+                "build-on": ["amd64"],
+                "run-on": "amd64",
+                "build-error": "ignore"},
+            "expected_build_on": ["amd64"],
+            "expected_run_on": ["amd64"],
+            "expected_build_error": "ignore",
+            }),
+        ]
+
+    def test_architecture(self):
+        architecture = SnapArchitecture.from_dict(self.architectures)
+        self.assertEqual(self.expected_build_on, architecture.build_on)
+        self.assertEqual(self.expected_run_on, architecture.run_on)
+        self.assertEqual(self.expected_build_error, architecture.build_error)
+
+
+class TestSnapBuildInstance(WithScenarios, TestCase):
+
+    # Single-item scenarios taken from the architectures document:
+    # https://forum.snapcraft.io/t/architectures/4972
+    scenarios = [
+        ("i386", {
+            "architecture": SnapArchitecture(
+                build_on="i386", run_on=["amd64", "i386"]),
+            "supported_architectures": ["amd64", "i386", "armhf"],
+            "expected_architecture": "i386",
+            "expected_required": True,
+            }),
+        ("amd64", {
+            "architecture": SnapArchitecture(build_on="amd64", run_on="all"),
+            "supported_architectures": ["amd64", "i386", "armhf"],
+            "expected_architecture": "amd64",
+            "expected_required": True,
+            }),
+        ("amd64 priority", {
+            "architecture": SnapArchitecture(
+                build_on=["amd64", "i386"], run_on="all"),
+            "supported_architectures": ["amd64", "i386", "armhf"],
+            "expected_architecture": "amd64",
+            "expected_required": True,
+            }),
+        ("i386 priority", {
+            "architecture": SnapArchitecture(
+                build_on=["amd64", "i386"], run_on="all"),
+            "supported_architectures": ["i386", "amd64", "armhf"],
+            "expected_architecture": "i386",
+            "expected_required": True,
+            }),
+        ("optional", {
+            "architecture": SnapArchitecture(
+                build_on="amd64", build_error="ignore"),
+            "supported_architectures": ["amd64", "i386", "armhf"],
+            "expected_architecture": "amd64",
+            "expected_required": False,
+            }),
+        ]
+
+    def test_build_instance(self):
+        instance = SnapBuildInstance(
+            self.architecture, self.supported_architectures)
+        self.assertEqual(self.expected_architecture, instance.architecture)
+        self.assertEqual(self.expected_required, instance.required)
+
+
+class TestSnapBuildInstanceError(TestCase):
+
+    def test_no_matching_arch_raises(self):
+        architecture = SnapArchitecture(build_on="amd64", run_on="amd64")
+        raised = self.assertRaises(
+            UnsupportedBuildOnError, SnapBuildInstance, architecture, ["i386"])
+        self.assertEqual(["amd64"], raised.build_on)
+
+
+class TestDetermineArchitecturesToBuild(WithScenarios, TestCase):
+
+    # Scenarios taken from the architectures document:
+    # https://forum.snapcraft.io/t/architectures/4972
+    scenarios = [
+        ("none", {
+            "architectures": None,
+            "supported_architectures": ["amd64", "i386", "armhf"],
+            "expected": [
+                {"architecture": "amd64", "required": True},
+                {"architecture": "i386", "required": True},
+                {"architecture": "armhf", "required": True},
+                ],
+            }),
+        ("i386", {
+            "architectures": [
+                {"build-on": "i386", "run-on": ["amd64", "i386"]},
+                ],
+            "supported_architectures": ["amd64", "i386", "armhf"],
+            "expected": [{"architecture": "i386", "required": True}],
+            }),
+        ("amd64", {
+            "architectures": [{"build-on": "amd64", "run-on": "all"}],
+            "supported_architectures": ["amd64", "i386", "armhf"],
+            "expected": [{"architecture": "amd64", "required": True}],
+            }),
+        ("amd64 and i386", {
+            "architectures": [
+                {"build-on": "amd64", "run-on": "amd64"},
+                {"build-on": "i386", "run-on": "i386"},
+                ],
+            "supported_architectures": ["amd64", "i386", "armhf"],
+            "expected": [
+                {"architecture": "amd64", "required": True},
+                {"architecture": "i386", "required": True},
+                ],
+            }),
+        ("amd64 and i386 shorthand", {
+            "architectures": [
+                {"build-on": "amd64"},
+                {"build-on": "i386"},
+                ],
+            "supported_architectures": ["amd64", "i386", "armhf"],
+            "expected": [
+                {"architecture": "amd64", "required": True},
+                {"architecture": "i386", "required": True},
+                ],
+            }),
+        ("amd64, i386, and armhf", {
+            "architectures": [
+                {"build-on": "amd64", "run-on": "amd64"},
+                {"build-on": "i386", "run-on": "i386"},
+                {
+                    "build-on": "armhf",
+                    "run-on": "armhf",
+                    "build-error": "ignore",
+                    },
+                ],
+            "supported_architectures": ["amd64", "i386", "armhf"],
+            "expected": [
+                {"architecture": "amd64", "required": True},
+                {"architecture": "i386", "required": True},
+                {"architecture": "armhf", "required": False},
+                ],
+            }),
+        ("amd64 priority", {
+            "architectures": [
+                {"build-on": ["amd64", "i386"], "run-on": "all"},
+                ],
+            "supported_architectures": ["amd64", "i386", "armhf"],
+            "expected": [{"architecture": "amd64", "required": True}],
+            }),
+        ("i386 priority", {
+            "architectures": [
+                {"build-on": ["amd64", "i386"], "run-on": "all"},
+                ],
+            "supported_architectures": ["i386", "amd64", "armhf"],
+            "expected": [{"architecture": "i386", "required": True}],
+            }),
+        ("old style i386 priority", {
+            "architectures": ["amd64", "i386"],
+            "supported_architectures": ["i386", "amd64", "armhf"],
+            "expected": [{"architecture": "i386", "required": True}],
+            }),
+        ("old style amd64 priority", {
+            "architectures": ["amd64", "i386"],
+            "supported_architectures": ["amd64", "i386", "armhf"],
+            "expected": [{"architecture": "amd64", "required": True}],
+            }),
+        ("more architectures listed than are supported", {
+            "architectures": [
+                {"build-on": "amd64"},
+                {"build-on": "i386"},
+                {"build-on": "armhf"},
+                ],
+            "supported_architectures": ["amd64", "i386"],
+            "expected": [
+                {"architecture": "amd64", "required": True},
+                {"architecture": "i386", "required": True},
+                ],
+            })
+    ]
+
+    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)
+
+
+load_tests = load_tests_apply_scenarios