launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+474016@xxxxxxxxxxxxxxxxxx>]

I am not really happy about manually maintaining a list of distroseries - let's have a chat before merging.

Diff comments:

> diff --git a/lib/lp/crafts/adapters/buildarch.py b/lib/lp/crafts/adapters/buildarch.py
> new file mode 100644
> index 0000000..3f1ffde
> --- /dev/null
> +++ b/lib/lp/crafts/adapters/buildarch.py
> @@ -0,0 +1,127 @@
> +# Copyright 2024 Canonical Ltd.  This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +__all__ = [
> +    "determine_instances_to_build",
> +]
> +
> +from collections import OrderedDict

OrderedDicts are not longer necessary since Python 3.6/3.7, as insertion order is stable since then - and yes, you adapted that from my code :-)

> +
> +
> +class CraftBasesParserError(Exception):
> +    """Base class for all exceptions in this module."""
> +
> +
> +class MissingPropertyError(CraftBasesParserError):
> +    """Error for when an expected property is not present in the YAML."""
> +
> +    def __init__(self, prop, msg=None):
> +        if msg is None:
> +            msg = f"Craft specification is missing the {prop!r} property"
> +        super().__init__(msg)
> +        self.property = prop
> +
> +
> +class BadPropertyError(CraftBasesParserError):
> +    """Error for when a YAML property is malformed in some way."""
> +
> +
> +class CraftBase:
> +    """A single base in sourcecraft.yaml."""
> +
> +    VALID_BASES = {"ubuntu@20.04", "ubuntu@22.04", "ubuntu@24.04", "bare"}

This means, every time a new LTS gets released, we need to think to update that code here. I am not entirely sure, but I think if you tried to schedule a build for ubuntu@30.04 right now, this would error out automatically.

For rocks we use something like this..

```
        results = store.using(*origin).find(
            (DistroArchSeries, DistroSeries, Distribution),
            DistroSeries.status.is_in(ACTIVE_STATUSES),
        )
        all_buildable_dases = DecoratedResultSet(results, itemgetter(0))
```

> +    VALID_BUILD_BASES = VALID_BASES | {"devel"}
> +
> +    def __init__(self, base_string, is_build_base=False):
> +        if is_build_base:
> +            if base_string not in self.VALID_BUILD_BASES:
> +                raise BadPropertyError(
> +                    f"Invalid build-base: {base_string!r}. "
> +                    f"Must be one of "
> +                    f"{', '.join(sorted(self.VALID_BUILD_BASES))}"
> +                )
> +        else:
> +            if base_string not in self.VALID_BASES:
> +                raise BadPropertyError(
> +                    f"Invalid base: {base_string!r}. "
> +                    f"Must be one of {', '.join(sorted(self.VALID_BASES))}"
> +                )
> +
> +        self.base_string = base_string
> +        if base_string == "bare":
> +            self.name = "bare"
> +            self.channel = None
> +        elif base_string == "devel":
> +            raise BadPropertyError("The 'devel' base is not supported for now")
> +        else:
> +            self.name, self.channel = base_string.split("@")
> +
> +    def __eq__(self, other):
> +        if not isinstance(other, CraftBase):
> +            return NotImplemented
> +        return self.name == other.name and self.channel == other.channel
> +
> +    def __hash__(self):
> +        return hash((self.name, self.channel))
> +
> +    def __str__(self):
> +        return f"{self.name}@{self.channel}"
> +
> +
> +def determine_instances_to_build(
> +    sourcecraft_data, supported_arches, default_distro_series
> +):
> +    """Return a list of instances to build based on sourcecraft.yaml.
> +
> +    :param sourcecraft_data: A parsed sourcecraft.yaml.
> +    :param supported_arches: An ordered list of all `DistroArchSeries` that
> +        we can create builds for. Note that these may span multiple
> +        `DistroSeries`.
> +    :param default_distro_series: The default `DistroSeries` to use if
> +        sourcecraft.yaml does not explicitly declare a base.
> +    :return: A list of `DistroArchSeries`.
> +    """
> +    base = sourcecraft_data.get("base")
> +    if not base:
> +        raise MissingPropertyError("base")
> +
> +    craft_base = CraftBase(base)
> +
> +    if craft_base.base_string == "bare":
> +        build_base = sourcecraft_data.get("build-base")
> +        if not build_base:
> +            raise MissingPropertyError(
> +                "build-base",
> +                "When 'base' is 'bare', 'build-base' must be specified",
> +            )
> +        craft_base = CraftBase(build_base, is_build_base=True)
> +    elif "build-base" in sourcecraft_data:
> +        craft_base = CraftBase(
> +            sourcecraft_data["build-base"], is_build_base=True
> +        )
> +
> +    platforms = sourcecraft_data.get("platforms")
> +    if not platforms:
> +        raise MissingPropertyError("platforms")
> +
> +    instances = OrderedDict()
> +    for platform, config in platforms.items():
> +        build_on = config.get("build-on", [platform])
> +        build_for = config.get("build-for", [platform])
> +
> +        if isinstance(build_on, str):
> +            build_on = [build_on]
> +        if isinstance(build_for, str):
> +            build_for = [build_for]
> +
> +        for arch in build_on:
> +            for das in supported_arches:
> +                if (
> +                    das.distroseries.distribution.name == craft_base.name
> +                    and das.distroseries.version == craft_base.channel
> +                    and das.architecturetag == arch
> +                ):
> +                    instances[das] = None
> +                    break
> +
> +    return list(instances)


-- 
https://code.launchpad.net/~ruinedyourlife/launchpad/+git/launchpad/+merge/474016
Your team Launchpad code reviewers is requested to review the proposed merge of ~ruinedyourlife/launchpad:add-sourcecraft-yaml-parser into launchpad:master.