launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:charm-do-not-require-charmcraft-yaml into launchpad:master.

Commit message:
charms: Don't require charmcraft.yaml to build

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Confusingly, "charmcraft build" doesn't seem to actually require charmcraft.yaml, and we only use it to determine the DASes to build on and have reasonable defaults for that.  Allow dispatching builds without charmcraft.yaml, thereby allowing us to build more actually-existing Operator Framework charms.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:charm-do-not-require-charmcraft-yaml into launchpad:master.

diff --git a/lib/lp/charms/interfaces/charmrecipe.py b/lib/lp/charms/interfaces/charmrecipe.py
index 80e4572..c7b7dd6 100644
--- a/lib/lp/charms/interfaces/charmrecipe.py
+++ b/lib/lp/charms/interfaces/charmrecipe.py
@@ -408,23 +408,20 @@ class ICharmRecipeEditableAttributes(Interface):
         schema=IGitRepository, vocabulary="GitRepository",
         required=False, readonly=True,
         description=_(
-            "A Git repository with a branch containing a charmcraft.yaml "
-            "recipe."))
+            "A Git repository with a branch containing a charm recipe."))
 
     git_path = TextLine(
         title=_("Git branch path"), required=False, readonly=True,
-        description=_(
-            "The path of the Git branch containing a charmcraft.yaml "
-            "recipe."))
+        description=_("The path of the Git branch containing a charm recipe."))
 
     git_ref = Reference(
         IGitRef, title=_("Git branch"), required=False, readonly=False,
-        description=_("The Git branch containing a charmcraft.yaml recipe."))
+        description=_("The Git branch containing a charm recipe."))
 
     build_path = TextLine(
         title=_("Build path"),
         description=_(
-            "Subdirectory within the branch containing charmcraft.yaml."),
+            "Subdirectory within the branch containing metadata.yaml."),
         constraint=path_does_not_escape, required=False, readonly=False)
 
     information_type = Choice(
@@ -437,8 +434,8 @@ class ICharmRecipeEditableAttributes(Interface):
         title=_("Automatically build when branch changes"),
         required=True, readonly=False,
         description=_(
-            "Whether this charm recipe is built automatically when the branch "
-            "containing its charmcraft.yaml recipe changes."))
+            "Whether this charm recipe is built automatically when its branch "
+            "changes."))
 
     auto_build_channels = Dict(
         title=_("Source snap channels for automatic builds"),
diff --git a/lib/lp/charms/model/charmrecipe.py b/lib/lp/charms/model/charmrecipe.py
index f2066d4..40bdd2a 100644
--- a/lib/lp/charms/model/charmrecipe.py
+++ b/lib/lp/charms/model/charmrecipe.py
@@ -514,8 +514,13 @@ class CharmRecipe(StormBase):
                              logger=None):
         """See `ICharmRecipe`."""
         try:
-            charmcraft_data = removeSecurityProxy(
-                getUtility(ICharmRecipeSet).getCharmcraftYaml(self))
+            try:
+                charmcraft_data = removeSecurityProxy(
+                    getUtility(ICharmRecipeSet).getCharmcraftYaml(self))
+            except MissingCharmcraftYaml:
+                # charmcraft doesn't currently require charmcraft.yaml, and
+                # we have reasonable defaults without it.
+                charmcraft_data = {}
 
             # Sort by (Distribution.id, DistroSeries.id, Processor.id) for
             # determinism.  This is chosen to be a similar order as in
diff --git a/lib/lp/charms/templates/charmrecipe-new.pt b/lib/lp/charms/templates/charmrecipe-new.pt
index bde870d..f5e9a0a 100644
--- a/lib/lp/charms/templates/charmrecipe-new.pt
+++ b/lib/lp/charms/templates/charmrecipe-new.pt
@@ -15,7 +15,8 @@
       know-how it takes to operate it, such as how to combine and work with
       other related applications or how to upgrade it.  Launchpad can build
       charms using <a href="https://juju.is/docs/sdk";>charmcraft</a>, from
-      any Git branch on Launchpad that has a <tt>charmcraft.yaml</tt> file.
+      any Git branch on Launchpad that has <tt>metadata.yaml</tt>,
+      <tt>requirements.txt</tt>, and <tt>src/charm.py</tt> files.
     </p>
   </div>
 
diff --git a/lib/lp/charms/tests/test_charmrecipe.py b/lib/lp/charms/tests/test_charmrecipe.py
index edd34d2..5dabfe9 100644
--- a/lib/lp/charms/tests/test_charmrecipe.py
+++ b/lib/lp/charms/tests/test_charmrecipe.py
@@ -56,13 +56,14 @@ from lp.charms.interfaces.charmrecipejob import (
     )
 from lp.charms.model.charmrecipebuild import CharmFile
 from lp.charms.model.charmrecipejob import CharmRecipeJob
+from lp.code.errors import GitRepositoryBlobNotFound
 from lp.code.tests.helpers import GitHostingFixture
 from lp.registry.interfaces.series import SeriesStatus
+from lp.services.config import config
 from lp.services.database.constants import (
     ONE_DAY_AGO,
     UTC_NOW,
     )
-from lp.services.config import config
 from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import (
     flush_database_caches,
@@ -490,6 +491,34 @@ class TestCharmRecipe(TestCaseWithFactory):
         self.assertRequestedBuildsMatch(
             builds, job, "20.04", ["mips64el", "riscv64"], job.channels)
 
+    def test_requestBuildsFromJob_no_charmcraft_yaml(self):
+        # If the recipe has no charmcraft.yaml file, requestBuildsFromJob
+        # treats this as equivalent to a charmcraft.yaml file that doesn't
+        # specify any bases: that is, it requests builds for all configured
+        # architectures for the default series.
+        self.useFixture(FeatureFixture({
+            CHARM_RECIPE_ALLOW_CREATE: "on",
+            CHARM_RECIPE_BUILD_DISTRIBUTION: "ubuntu",
+            "charm.default_build_series.ubuntu": "20.04",
+            }))
+        self.useFixture(GitHostingFixture()).getBlob.failure = (
+            GitRepositoryBlobNotFound("placeholder", "charmcraft.yaml"))
+        old_distro_series = self.factory.makeDistroSeries(
+            distribution=getUtility(ILaunchpadCelebrities).ubuntu,
+            version="18.04")
+        for arch_tag in ("mips64el", "riscv64"):
+            self.makeBuildableDistroArchSeries(
+                distroseries=old_distro_series, architecturetag=arch_tag)
+        job = self.makeRequestBuildsJob("20.04", ["mips64el", "riscv64"])
+        self.assertEqual(
+            get_transaction_timestamp(IStore(job.recipe)), job.date_created)
+        transaction.commit()
+        with person_logged_in(job.requester):
+            builds = job.recipe.requestBuildsFromJob(
+                job.build_request, channels=removeSecurityProxy(job.channels))
+        self.assertRequestedBuildsMatch(
+            builds, job, "20.04", ["mips64el", "riscv64"], job.channels)
+
     def test_requestBuildsFromJob_architectures_parameter(self):
         # If an explicit set of architectures was given as a parameter,
         # requestBuildsFromJob intersects those with any other constraints