launchpad-reviewers team mailing list archive

[Colin Watson <mp+411464@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:mark-charm-recipes-stale into launchpad:master.

Commit message:
Mark charm recipes stale when their source branches changed

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1950005 in Launchpad itself: "Not getting automated charm builds"
  https://bugs.launchpad.net/launchpad/+bug/1950005

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/411464
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:mark-charm-recipes-stale into launchpad:master.

diff --git a/lib/lp/code/interfaces/gitrepository.py b/lib/lp/code/interfaces/gitrepository.py
index 363356d..cb12a5d 100644
--- a/lib/lp/code/interfaces/gitrepository.py
+++ b/lib/lp/code/interfaces/gitrepository.py
@@ -644,6 +644,14 @@ class IGitRepositoryView(IHasRecipes):
             based on one of these paths will be marked as stale.
         """
 
+    def markCharmRecipesStale(paths):
+        """Mark charm recipes associated with this repository as stale.
+
+        :param paths: A list of reference paths.  Any charm recipes that
+            include an entry that points to this repository and that are
+            based on one of these paths will be marked as stale.
+        """
+
     def detectMerges(paths, logger=None):
         """Detect merges of landing candidates.
 
diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py
index 99d451b..796ab80 100644
--- a/lib/lp/code/model/gitrepository.py
+++ b/lib/lp/code/model/gitrepository.py
@@ -1290,6 +1290,16 @@ class GitRepository(StormBase, WebhookTargetMixin, AccessTokenTargetMixin,
             # this.
             removeSecurityProxy(snap).is_stale = True
 
+    def markCharmRecipesStale(self, paths):
+        """See `IGitRepository`."""
+        recipes = getUtility(ICharmRecipeSet).findByGitRepository(
+            self, paths=paths, check_permissions=False)
+        for recipe in recipes:
+            # ICharmRecipeSet.findByGitRepository returns security-proxied
+            # CharmRecipe objects on which the is_stale attribute is
+            # read-only.  Bypass this.
+            removeSecurityProxy(recipe).is_stale = True
+
     def _markProposalMerged(self, proposal, merged_revision_id, logger=None):
         if logger is not None:
             logger.info(
diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py
index e1579ee..dbdf8d9 100644
--- a/lib/lp/code/model/tests/test_gitrepository.py
+++ b/lib/lp/code/model/tests/test_gitrepository.py
@@ -55,7 +55,10 @@ from lp.app.enums import (
     )
 from lp.app.errors import NotFoundError
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
-from lp.charms.interfaces.charmrecipe import CHARM_RECIPE_ALLOW_CREATE
+from lp.charms.interfaces.charmrecipe import (
+    CHARM_RECIPE_ALLOW_CREATE,
+    CHARM_RECIPE_PRIVATE_FEATURE_FLAG,
+    )
 from lp.code.enums import (
     BranchMergeProposalStatus,
     BranchSubscriptionDiffSize,
@@ -2796,6 +2799,48 @@ class TestGitRepositoryMarkSnapsStale(TestCaseWithFactory):
         self.assertTrue(snap.is_stale)
 
 
+class TestGitRepositoryMarkCharmRecipesStale(TestCaseWithFactory):
+
+    layer = ZopelessDatabaseLayer
+
+    def setUp(self):
+        super().setUp()
+        self.useFixture(FeatureFixture({
+            CHARM_RECIPE_ALLOW_CREATE: "on",
+            CHARM_RECIPE_PRIVATE_FEATURE_FLAG: "on",
+            }))
+
+    def test_same_repository(self):
+        # On ref changes, charm recipes using this ref become stale.
+        [ref] = self.factory.makeGitRefs()
+        recipe = self.factory.makeCharmRecipe(git_ref=ref)
+        removeSecurityProxy(recipe).is_stale = False
+        ref.repository.createOrUpdateRefs(
+            {ref.path: {"sha1": "0" * 40, "type": GitObjectType.COMMIT}})
+        self.assertTrue(recipe.is_stale)
+
+    def test_same_repository_different_ref(self):
+        # On ref changes, charm recipes using a different ref in the same
+        # repository are left alone.
+        ref1, ref2 = self.factory.makeGitRefs(
+            paths=["refs/heads/a", "refs/heads/b"])
+        recipe = self.factory.makeCharmRecipe(git_ref=ref1)
+        removeSecurityProxy(recipe).is_stale = False
+        ref1.repository.createOrUpdateRefs(
+            {ref2.path: {"sha1": "0" * 40, "type": GitObjectType.COMMIT}})
+        self.assertFalse(recipe.is_stale)
+
+    def test_different_repository(self):
+        # On ref changes, unrelated charm recipes are left alone.
+        [ref] = self.factory.makeGitRefs()
+        recipe = self.factory.makeCharmRecipe(
+            git_ref=self.factory.makeGitRefs()[0])
+        removeSecurityProxy(recipe).is_stale = False
+        ref.repository.createOrUpdateRefs(
+            {ref.path: {"sha1": "0" * 40, "type": GitObjectType.COMMIT}})
+        self.assertFalse(recipe.is_stale)
+
+
 class TestGitRepositoryFork(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer
diff --git a/lib/lp/code/subscribers/git.py b/lib/lp/code/subscribers/git.py
index 5a4ad04..30b432e 100644
--- a/lib/lp/code/subscribers/git.py
+++ b/lib/lp/code/subscribers/git.py
@@ -9,4 +9,5 @@ def refs_updated(repository, event):
     repository.updateLandingTargets(event.paths)
     repository.markRecipesStale(event.paths)
     repository.markSnapsStale(event.paths)
+    repository.markCharmRecipesStale(event.paths)
     repository.detectMerges(event.paths, logger=event.logger)