launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:charm-recipe-distroseries into launchpad:master with ~cjwatson/launchpad:charm-recipe-job as a prerequisite.

Commit message:
Add CharmRecipe.distribution and CharmRecipe.distro_series

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Similarly to OCI recipes, it doesn't seem to make much difference which distribution series context we dispatch a charm recipe build to: charmcraft is a non-classic snap and does all its work within the snap environment anyway.  Unless and until we hear otherwise, use a fixed series set by a pair of feature rules for this.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:charm-recipe-distroseries into launchpad:master.

diff --git a/lib/lp/charms/interfaces/charmrecipe.py b/lib/lp/charms/interfaces/charmrecipe.py
index 0422e18..56ab189 100644
--- a/lib/lp/charms/interfaces/charmrecipe.py
+++ b/lib/lp/charms/interfaces/charmrecipe.py
@@ -10,6 +10,7 @@ __all__ = [
     "BadCharmRecipeSource",
     "BadCharmRecipeSearchContext",
     "CHARM_RECIPE_ALLOW_CREATE",
+    "CHARM_RECIPE_BUILD_DISTRIBUTION",
     "CHARM_RECIPE_PRIVATE_FEATURE_FLAG",
     "CharmRecipeBuildRequestStatus",
     "CharmRecipeFeatureDisabled",
@@ -57,6 +58,8 @@ from lp.app.validators.name import name_validator
 from lp.app.validators.path import path_does_not_escape
 from lp.code.interfaces.gitref import IGitRef
 from lp.code.interfaces.gitrepository import IGitRepository
+from lp.registry.interfaces.distribution import IDistribution
+from lp.registry.interfaces.distroseries import IDistroSeries
 from lp.registry.interfaces.product import IProduct
 from lp.services.fields import (
     PersonChoice,
@@ -67,6 +70,7 @@ from lp.snappy.validators.channels import channels_validator
 
 CHARM_RECIPE_ALLOW_CREATE = "charm.recipe.create.enabled"
 CHARM_RECIPE_PRIVATE_FEATURE_FLAG = "charm.recipe.allow_private"
+CHARM_RECIPE_BUILD_DISTRIBUTION = "charm.default_build_distribution"
 
 
 @error_status(http_client.UNAUTHORIZED)
@@ -210,6 +214,16 @@ class ICharmRecipeView(Interface):
         title=_("Private"), required=False, readonly=False,
         description=_("Whether this charm recipe is private."))
 
+    distribution = Reference(
+        IDistribution, title=_("Distribution"),
+        required=True, readonly=True,
+        description=_("The distribution that this recipe is associated with."))
+
+    distro_series = Reference(
+        IDistroSeries, title=_("Distro series"),
+        required=True, readonly=True,
+        description=_("The series for which the recipe should be built."))
+
     def getAllowedInformationTypes(user):
         """Get a list of acceptable `InformationType`s for this charm recipe.
 
diff --git a/lib/lp/charms/model/charmrecipe.py b/lib/lp/charms/model/charmrecipe.py
index 65cfbc7..1fac03e 100644
--- a/lib/lp/charms/model/charmrecipe.py
+++ b/lib/lp/charms/model/charmrecipe.py
@@ -28,8 +28,10 @@ from lp.app.enums import (
     InformationType,
     PUBLIC_INFORMATION_TYPES,
     )
+from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.charms.interfaces.charmrecipe import (
     CHARM_RECIPE_ALLOW_CREATE,
+    CHARM_RECIPE_BUILD_DISTRIBUTION,
     CHARM_RECIPE_PRIVATE_FEATURE_FLAG,
     CharmRecipeBuildRequestStatus,
     CharmRecipeFeatureDisabled,
@@ -47,6 +49,7 @@ from lp.charms.interfaces.charmrecipejob import (
     )
 from lp.code.model.gitrepository import GitRepository
 from lp.registry.errors import PrivatePersonLinkageError
+from lp.registry.interfaces.distribution import IDistributionSet
 from lp.registry.interfaces.person import validate_public_person
 from lp.services.database.constants import (
     DEFAULT,
@@ -275,6 +278,33 @@ class CharmRecipe(StormBase):
         """See `ICharmRecipe`."""
         self._store_channels = value or None
 
+    @cachedproperty
+    def distribution(self):
+        """See `ICharmRecipe`."""
+        # Use the default distribution set by this feature rule, or Ubuntu
+        # if none is set.
+        distro_name = getFeatureFlag(CHARM_RECIPE_BUILD_DISTRIBUTION)
+        if not distro_name:
+            return getUtility(ILaunchpadCelebrities).ubuntu
+        distro = getUtility(IDistributionSet).getByName(distro_name)
+        if not distro:
+            raise ValueError(
+                "'%s' is not a valid value for feature rule '%s'" % (
+                    distro_name, CHARM_RECIPE_BUILD_DISTRIBUTION))
+        return distro
+
+    @cachedproperty
+    def distro_series(self):
+        """See `ICharmRecipe`."""
+        # Use the series set by this feature rule, or the current series of
+        # the recipe's distribution if the feature rule is not set.
+        series_name = getFeatureFlag(
+            "charm.build_series.%s" % self.distribution.name)
+        if series_name:
+            return self.distribution.getSeries(series_name)
+        else:
+            return self.distribution.currentseries
+
     def getAllowedInformationTypes(self, user):
         """See `ICharmRecipe`."""
         # XXX cjwatson 2021-05-26: Only allow free information types until
diff --git a/lib/lp/charms/tests/test_charmrecipe.py b/lib/lp/charms/tests/test_charmrecipe.py
index bf88045..9dc8e39 100644
--- a/lib/lp/charms/tests/test_charmrecipe.py
+++ b/lib/lp/charms/tests/test_charmrecipe.py
@@ -20,6 +20,7 @@ from zope.security.proxy import removeSecurityProxy
 from lp.app.enums import InformationType
 from lp.charms.interfaces.charmrecipe import (
     CHARM_RECIPE_ALLOW_CREATE,
+    CHARM_RECIPE_BUILD_DISTRIBUTION,
     CharmRecipeBuildRequestStatus,
     CharmRecipeFeatureDisabled,
     CharmRecipePrivateFeatureDisabled,
@@ -30,6 +31,7 @@ from lp.charms.interfaces.charmrecipe import (
 from lp.charms.interfaces.charmrecipejob import (
     ICharmRecipeRequestBuildsJobSource,
     )
+from lp.registry.interfaces.series import SeriesStatus
 from lp.services.database.constants import (
     ONE_DAY_AGO,
     UTC_NOW,
@@ -104,6 +106,61 @@ class TestCharmRecipe(TestCaseWithFactory):
         self.assertSqlAttributeEqualsDate(
             recipe, "date_last_modified", UTC_NOW)
 
+    def test_distribution_default(self):
+        # If the CHARM_RECIPE_BUILD_DISTRIBUTION feature rule is not set, we
+        # use Ubuntu.
+        recipe = self.factory.makeCharmRecipe()
+        self.assertEqual("ubuntu", recipe.distribution.name)
+
+    def test_distribution_feature_rule(self):
+        # If the CHARM_RECIPE_BUILD_DISTRIBUTION feature rule is set, we use
+        # the distribution with the given name.
+        distro_name = "mydistro"
+        distribution = self.factory.makeDistribution(name=distro_name)
+        recipe = self.factory.makeCharmRecipe()
+        with FeatureFixture({CHARM_RECIPE_BUILD_DISTRIBUTION: distro_name}):
+            self.assertEqual(distribution, recipe.distribution)
+
+    def test_distribution_feature_rule_nonexistent(self):
+        # If we mistakenly set the rule to a non-existent distribution,
+        # things break explicitly.
+        recipe = self.factory.makeCharmRecipe()
+        with FeatureFixture({CHARM_RECIPE_BUILD_DISTRIBUTION: "nonexistent"}):
+            expected_msg = (
+                "'nonexistent' is not a valid value for feature rule '%s'" %
+                CHARM_RECIPE_BUILD_DISTRIBUTION)
+            self.assertRaisesWithContent(
+                ValueError, expected_msg, getattr, recipe, "distribution")
+
+    def test_distro_series_feature_rule(self):
+        # If the appropriate per-distribution feature rule is set, we use
+        # the named distro series.
+        distro_name = "mydistro"
+        distribution = self.factory.makeDistribution(name=distro_name)
+        distro_series_name = "myseries"
+        distro_series = self.factory.makeDistroSeries(
+            distribution=distribution, name=distro_series_name)
+        self.factory.makeDistroSeries(distribution=distribution)
+        recipe = self.factory.makeCharmRecipe()
+        with FeatureFixture({
+                CHARM_RECIPE_BUILD_DISTRIBUTION: distro_name,
+                "charm.build_series.%s" % distro_name: distro_series_name,
+                }):
+            self.assertEqual(distro_series, recipe.distro_series)
+
+    def test_distro_series_no_feature_rule(self):
+        # If the appropriate per-distribution feature rule is not set, we
+        # use the distribution's current series.
+        distro_name = "mydistro"
+        distribution = self.factory.makeDistribution(name=distro_name)
+        self.factory.makeDistroSeries(
+            distribution=distribution, status=SeriesStatus.SUPPORTED)
+        current_series = self.factory.makeDistroSeries(
+            distribution=distribution, status=SeriesStatus.DEVELOPMENT)
+        recipe = self.factory.makeCharmRecipe()
+        with FeatureFixture({CHARM_RECIPE_BUILD_DISTRIBUTION: distro_name}):
+            self.assertEqual(current_series, recipe.distro_series)
+
     def test_requestBuilds(self):
         # requestBuilds schedules a job and returns a corresponding
         # CharmRecipeBuildRequest.