launchpad-reviewers team mailing list archive

[Colin Watson <mp+440778@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:charmrecipe-build-path into launchpad:master.

Commit message:
Allow editing build_path in charm recipe UI

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

The charm recipe build protocol supports `build_path`, which changes to a subdirectory of the checked-out branch before running `charmcraft`.  This is quite useful: for instance, we use it in charms of Launchpad itself so that we can have multiple charm recipes in the same source tree that deploy the same project in different ways.

However, it's currently only possible to set this field using the webservice API, which I think was an oversight from when I first put together the charm recipe UI.  Fix this oversight.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:charmrecipe-build-path into launchpad:master.

diff --git a/lib/lp/charms/browser/charmrecipe.py b/lib/lp/charms/browser/charmrecipe.py
index 8486c2c..db3f387 100644
--- a/lib/lp/charms/browser/charmrecipe.py
+++ b/lib/lp/charms/browser/charmrecipe.py
@@ -278,6 +278,7 @@ class ICharmRecipeEditSchema(Interface):
             "name",
             "project",
             "require_virtualized",
+            "build_path",
             "auto_build",
             "auto_build_channels",
             "store_upload",
@@ -333,6 +334,7 @@ class CharmRecipeAddView(CharmRecipeAuthorizeMixin, LaunchpadFormView):
         else:
             fields += ["project"]
         return fields + [
+            "build_path",
             "auto_build",
             "auto_build_channels",
             "store_upload",
@@ -386,6 +388,7 @@ class CharmRecipeAddView(CharmRecipeAuthorizeMixin, LaunchpadFormView):
             project,
             data["name"],
             git_ref=git_ref,
+            build_path=data["build_path"],
             auto_build=data["auto_build"],
             auto_build_channels=data["auto_build_channels"],
             store_upload=data["store_upload"],
@@ -515,6 +518,7 @@ class CharmRecipeEditView(BaseCharmRecipeEditView):
         "name",
         "project",
         "git_ref",
+        "build_path",
         "auto_build",
         "auto_build_channels",
         "store_upload",
diff --git a/lib/lp/charms/browser/tests/test_charmrecipe.py b/lib/lp/charms/browser/tests/test_charmrecipe.py
index 6153622..78bb21a 100644
--- a/lib/lp/charms/browser/tests/test_charmrecipe.py
+++ b/lib/lp/charms/browser/tests/test_charmrecipe.py
@@ -128,7 +128,7 @@ class TestCharmRecipeAddView(BaseTestCharmRecipeView):
         )
 
     def test_create_new_recipe_git(self):
-        self.factory.makeProduct(
+        project = self.factory.makeProduct(
             name="test-project", displayname="Test Project"
         )
         [git_ref] = self.factory.makeGitRefs(
@@ -156,6 +156,7 @@ class TestCharmRecipeAddView(BaseTestCharmRecipeView):
             "Source:\n%s\nEdit charm recipe" % source_display,
             MatchesTagText(content, "source"),
         )
+        self.assertIsNone(find_tag_by_id(content, "build_path"))
         self.assertThat(
             "Build schedule:\n(?)\nBuilt on request\nEdit charm recipe\n",
             MatchesTagText(content, "auto_build"),
@@ -167,6 +168,24 @@ class TestCharmRecipeAddView(BaseTestCharmRecipeView):
             MatchesTagText(content, "store_upload"),
         )
 
+        login_person(self.person)
+        recipe = getUtility(ICharmRecipeSet).getByName(
+            self.person, project, "charm-name"
+        )
+        self.assertThat(
+            recipe,
+            MatchesStructure(
+                owner=Equals(self.person),
+                project=Equals(project),
+                name=Equals("charm-name"),
+                source=Equals(git_ref),
+                build_path=Is(None),
+                auto_build=Is(False),
+                auto_build_channels=Equals({}),
+                store_upload=Is(False),
+            ),
+        )
+
     def test_create_new_recipe_git_project_namespace(self):
         # If the Git repository is already in a project namespace, then that
         # project is the default for the new recipe.
@@ -263,6 +282,29 @@ class TestCharmRecipeAddView(BaseTestCharmRecipeView):
             sorted(str(option) for option in options),
         )
 
+    def test_create_new_recipe_build_path(self):
+        project = self.factory.makeProduct(name="test-project")
+        [git_ref] = self.factory.makeGitRefs()
+        browser = self.getViewBrowser(
+            git_ref, view_name="+new-charm-recipe", user=self.person
+        )
+        browser.getControl(name="field.name").value = "charm-name"
+        browser.getControl(name="field.project").value = "test-project"
+        browser.getControl("Build path").value = "charm/foo"
+        browser.getControl("Create charm recipe").click()
+
+        content = find_main_content(browser.contents)
+        self.assertThat(
+            "Build path:\ncharm/foo\nEdit charm recipe\n",
+            MatchesTagText(content, "build-path"),
+        )
+
+        login_person(self.person)
+        recipe = getUtility(ICharmRecipeSet).getByName(
+            self.person, project, "charm-name"
+        )
+        self.assertEqual("charm/foo", recipe.build_path)
+
     def test_create_new_recipe_auto_build(self):
         # Creating a new recipe and asking for it to be automatically built
         # sets all the appropriate fields.
@@ -626,6 +668,7 @@ class TestCharmRecipeEditView(BaseTestCharmRecipeView):
             name="field.git_ref.repository"
         ).value = new_git_ref_identity
         browser.getControl(name="field.git_ref.path").value = new_git_ref_path
+        browser.getControl("Build path").value = "some-path"
         browser.getControl(
             "Automatically build when branch changes"
         ).selected = True
@@ -642,6 +685,10 @@ class TestCharmRecipeEditView(BaseTestCharmRecipeView):
             MatchesTagText(content, "source"),
         )
         self.assertThat(
+            "Build path:\nsome-path\nEdit charm recipe\n",
+            MatchesTagText(content, "build-path"),
+        )
+        self.assertThat(
             "Build schedule:\n(?)\nBuilt automatically\nEdit charm recipe\n",
             MatchesTagText(content, "auto_build"),
         )
@@ -1504,6 +1551,12 @@ class TestCharmRecipeView(BaseTestCharmRecipeView):
             self.getMainText(recipe),
         )
 
+    def test_index_build_path(self):
+        recipe = self.makeCharmRecipe(build_path="charm/foo")
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            "Build path: charm/foo", self.getMainText(recipe)
+        )
+
     def setStatus(self, build, status):
         build.updateStatus(
             BuildStatus.BUILDING, date_started=build.date_created
diff --git a/lib/lp/charms/templates/charmrecipe-edit.pt b/lib/lp/charms/templates/charmrecipe-edit.pt
index a68bd40..8e4398d 100644
--- a/lib/lp/charms/templates/charmrecipe-edit.pt
+++ b/lib/lp/charms/templates/charmrecipe-edit.pt
@@ -51,6 +51,10 @@
           <metal:block use-macro="context/@@launchpad_form/widget_row" />
         </tal:widget>
 
+        <tal:widget define="widget nocall:view/widgets/build_path">
+          <metal:block use-macro="context/@@launchpad_form/widget_row" />
+        </tal:widget>
+
         <tal:widget define="widget nocall:view/widgets/auto_build">
           <metal:block use-macro="context/@@launchpad_form/widget_row" />
         </tal:widget>
diff --git a/lib/lp/charms/templates/charmrecipe-index.pt b/lib/lp/charms/templates/charmrecipe-index.pt
index b7da253..9f23a0f 100644
--- a/lib/lp/charms/templates/charmrecipe-index.pt
+++ b/lib/lp/charms/templates/charmrecipe-index.pt
@@ -51,6 +51,13 @@
           <span class="sprite private">&lt;redacted&gt;</span>
         </dd>
       </dl>
+      <dl id="build-path" tal:condition="context/build_path">
+        <dt>Build path:</dt>
+        <dd>
+          <span tal:content="context/build_path"/>
+          <a tal:replace="structure view/menu:overview/edit/fmt:icon"/>
+        </dd>
+      </dl>
 
       <dl id="auto_build">
         <dt>Build schedule:
diff --git a/lib/lp/charms/templates/charmrecipe-new.pt b/lib/lp/charms/templates/charmrecipe-new.pt
index 6946d59..21c5f58 100644
--- a/lib/lp/charms/templates/charmrecipe-new.pt
+++ b/lib/lp/charms/templates/charmrecipe-new.pt
@@ -42,6 +42,10 @@
           </tal:widget>
         </tal:project-context>
 
+        <tal:widget define="widget nocall:view/widgets/build_path">
+          <metal:block use-macro="context/@@launchpad_form/widget_row" />
+        </tal:widget>
+
         <tal:widget define="widget nocall:view/widgets/auto_build">
           <metal:block use-macro="context/@@launchpad_form/widget_row" />
         </tal:widget>