launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> 
> === modified file 'lib/lp/snappy/interfaces/snap.py'
> --- lib/lp/snappy/interfaces/snap.py	2018-11-07 18:12:08 +0000
> +++ lib/lp/snappy/interfaces/snap.py	2019-02-05 12:46:08 +0000
> @@ -632,9 +639,11 @@
>          description=_("The owner of this snap package.")))
>  
>      distro_series = exported(Reference(
> -        IDistroSeries, title=_("Distro Series"), required=True, readonly=False,
> +        IDistroSeries, title=_("Distro Series"),
> +        required=False, readonly=False,
>          description=_(
> -            "The series for which the snap package should be built.")))
> +            "The series for which the snap package should be built.  If not "
> +            "set, Launchpad will guess at an appropriate series.")))

"[...] based on snapcraft.yaml"?

>  
>      name = exported(TextLine(
>          title=_("Name"), required=True, readonly=False,
> 
> === modified file 'lib/lp/snappy/model/snap.py'
> --- lib/lp/snappy/model/snap.py	2019-02-05 12:46:08 +0000
> +++ lib/lp/snappy/model/snap.py	2019-02-05 12:46:08 +0000
> @@ -393,13 +399,21 @@
>      @property
>      def available_processors(self):
>          """See `ISnap`."""
> -        processors = Store.of(self).find(
> -            Processor,
> -            Processor.id == DistroArchSeries.processor_id,
> -            DistroArchSeries.id.is_in(
> +        clauses = [Processor.id == DistroArchSeries.processor_id]
> +        if self.distro_series is not None:
> +            clauses.append(DistroArchSeries.id.is_in(
>                  self.distro_series.enabled_architectures.get_select_expr(
>                      DistroArchSeries.id)))
> -        return processors.config(distinct=True)
> +        else:
> +            # We don't know the series until we've looked at snapcraft.yaml
> +            # to dispatch a build, so enabled architectures for any active
> +            # series will do.
> +            clauses.extend([
> +                DistroArchSeries.enabled == True,

Can ditch the "== True".

> +                DistroArchSeries.distroseriesID == DistroSeries.id,
> +                DistroSeries.status.is_in(ACTIVE_STATUSES),
> +                ])
> +        return Store.of(self).find(Processor, *clauses).config(distinct=True)
>  
>      def _getProcessors(self):
>          return list(Store.of(self).find(
> @@ -606,12 +640,38 @@
>                      snapcraft_data = {}
>              else:
>                  snapcraft_data = {}
> +
> +            # Find a suitable base snap.
> +            base_snap_set = getUtility(IBaseSnapSet)
> +            if "base" in snapcraft_data:
> +                base_snap_name = snapcraft_data["base"]
> +                if isinstance(base_snap_name, bytes):
> +                    base_snap_name = base_snap_name.decode("UTF-8")
> +                base_snap = base_snap_set.getByName(base_snap_name)
> +            else:
> +                base_snap = base_snap_set.getDefault()
> +
> +            # Combine the base snap with other configuration to find a
> +            # suitable distro series and suitable channels.
> +            distro_series = self.distro_series
> +            if base_snap is not None:
> +                if distro_series is None:
> +                    distro_series = base_snap.distro_series
> +                new_channels = dict(base_snap.channels)
> +                if channels is not None:
> +                    new_channels.update(channels)
> +                channels = new_channels
> +            elif distro_series is None:
> +                # A base snap is mandatory if there's no configured distro
> +                # series.
> +                raise NoSuchBaseSnap(snapcraft_data.get("base", "<default>"))

These two new blocks seem like good candidates for a helper method or function.

> +
>              # Sort by Processor.id for determinism.  This is chosen to be
>              # the same order as in BinaryPackageBuildSet.createForSource, to
>              # minimise confusion.
>              supported_arches = OrderedDict(
>                  (das.architecturetag, das) for das in sorted(
> -                    self.getAllowedArchitectures(),
> +                    self.getAllowedArchitectures(distro_series),
>                      key=attrgetter("processor.id")))
>              architectures_to_build = determine_architectures_to_build(
>                  snapcraft_data, supported_arches.keys())


-- 
https://code.launchpad.net/~cjwatson/launchpad/snap-without-distro-series/+merge/362730
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-without-distro-series into lp:launchpad.