launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/git-recipe-stale into lp:launchpad with lp:~cjwatson/launchpad/git-recipe-delete as a prerequisite.

Commit message:
Mark Git recipes as stale when their branches change.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1453022 in Launchpad itself: "Please support daily builds via git"
  https://bugs.launchpad.net/launchpad/+bug/1453022

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/git-recipe-stale/+merge/282261

Mark Git recipes as stale when their branches change.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-recipe-stale into lp:launchpad.

=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py	2016-01-12 04:34:46 +0000
+++ lib/lp/code/interfaces/gitrepository.py	2016-01-12 04:34:46 +0000
@@ -516,6 +516,14 @@
             diffs updated.
         """
 
+    def markRecipesStale(paths):
+        """Mark recipes associated with this repository as stale.
+
+        :param paths: A list of reference paths.  Any recipes that include
+            an entry that points to this repository and that has a `revspec`
+            that is one of these paths will be marked as stale.
+        """
+
     def detectMerges(paths, logger=None):
         """Detect merges of landing candidates.
 

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2016-01-12 04:34:46 +0000
+++ lib/lp/code/model/gitrepository.py	2016-01-12 04:34:46 +0000
@@ -977,6 +977,11 @@
         hook = SourcePackageRecipe.preLoadDataForSourcePackageRecipes
         return DecoratedResultSet(self._getRecipes(), pre_iter_hook=hook)
 
+    def markRecipesStale(self, paths):
+        """See `IGitRepository`."""
+        for recipe in self._getRecipes(paths):
+            recipe.is_stale = True
+
     def _markProposalMerged(self, proposal, merged_revision_id, logger=None):
         if logger is not None:
             logger.info(
@@ -1047,6 +1052,11 @@
             prerequisite_git_repository=self):
             alteration_operations.append(
                 ClearPrerequisiteRepository(merge_proposal))
+        deletion_operations.extend(
+            DeletionCallable(
+                recipe, msg("This recipe uses this repository."),
+                recipe.destroySelf)
+            for recipe in self.recipes)
         if not getUtility(ISnapSet).findByGitRepository(self).is_empty():
             alteration_operations.append(DeletionCallable(
                 None, msg("Some snap packages build from this repository."),

=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2015-12-10 00:05:41 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2016-01-12 04:34:46 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for Git repositories."""
@@ -452,6 +452,19 @@
             [ReclaimGitRepositorySpaceJob(job).repository_path
              for job in jobs])
 
+    def test_destroySelf_with_SourcePackageRecipe(self):
+        # If repository is a base_git_repository in a recipe, it is deleted.
+        recipe = self.factory.makeSourcePackageRecipe(
+            branches=self.factory.makeGitRefs(owner=self.user))
+        recipe.base_git_repository.destroySelf(break_references=True)
+
+    def test_destroySelf_with_SourcePackageRecipe_as_non_base(self):
+        # If repository is referred to by a recipe, it is deleted.
+        [ref1] = self.factory.makeGitRefs(owner=self.user)
+        [ref2] = self.factory.makeGitRefs(owner=self.user)
+        self.factory.makeSourcePackageRecipe(branches=[ref1, ref2])
+        ref2.repository.destroySelf(break_references=True)
+
     def test_destroySelf_with_inline_comments_draft(self):
         # Draft inline comments related to a deleted repository (source or
         # target MP repository) also get removed.
@@ -683,6 +696,14 @@
         self.assertRaises(
             SQLObjectNotFound, BranchMergeProposal.get, merge_proposal_id)
 
+    def test_deletionRequirements_with_SourcePackageRecipe(self):
+        # Recipes are listed as deletion requirements.
+        recipe = self.factory.makeSourcePackageRecipe(
+            branches=self.factory.makeGitRefs())
+        self.assertEqual(
+            {recipe: ("delete", "This recipe uses this repository.")},
+            recipe.base_git_repository.getDeletionRequirements())
+
 
 class TestGitRepositoryModifications(TestCaseWithFactory):
     """Tests for Git repository modifications."""
@@ -1820,6 +1841,64 @@
         self.assertEqual(0, len(jobs))
 
 
+class TestGitRepositoryMarkRecipesStale(TestCaseWithFactory):
+
+    layer = ZopelessDatabaseLayer
+
+    def test_base_repository_recipe(self):
+        # On ref changes, recipes where this ref is the base become stale.
+        [ref] = self.factory.makeGitRefs()
+        recipe = self.factory.makeSourcePackageRecipe(branches=[ref])
+        removeSecurityProxy(recipe).is_stale = False
+        ref.repository.createOrUpdateRefs(
+            {ref.path: {u"sha1": u"0" * 40, u"type": GitObjectType.COMMIT}})
+        self.assertTrue(recipe.is_stale)
+
+    def test_base_repository_different_ref_recipe(self):
+        # On ref changes, recipes where a different ref in the same
+        # repository is the base are left alone.
+        ref1, ref2 = self.factory.makeGitRefs(
+            paths=[u"refs/heads/a", u"refs/heads/b"])
+        recipe = self.factory.makeSourcePackageRecipe(branches=[ref1])
+        removeSecurityProxy(recipe).is_stale = False
+        ref1.repository.createOrUpdateRefs(
+            {ref2.path: {u"sha1": u"0" * 40, u"type": GitObjectType.COMMIT}})
+        self.assertFalse(recipe.is_stale)
+
+    def test_instruction_repository_recipe(self):
+        # On ref changes, recipes including this repository become stale.
+        [base_ref] = self.factory.makeGitRefs()
+        [ref] = self.factory.makeGitRefs()
+        recipe = self.factory.makeSourcePackageRecipe(branches=[base_ref, ref])
+        removeSecurityProxy(recipe).is_stale = False
+        ref.repository.createOrUpdateRefs(
+            {ref.path: {u"sha1": u"0" * 40, u"type": GitObjectType.COMMIT}})
+        self.assertTrue(recipe.is_stale)
+
+    def test_instruction_repository_different_ref_recipe(self):
+        # On ref changes, recipes including a different ref in the same
+        # repository are left alone.
+        [base_ref] = self.factory.makeGitRefs()
+        ref1, ref2 = self.factory.makeGitRefs(
+            paths=[u"refs/heads/a", u"refs/heads/b"])
+        recipe = self.factory.makeSourcePackageRecipe(
+            branches=[base_ref, ref1])
+        removeSecurityProxy(recipe).is_stale = False
+        ref1.repository.createOrUpdateRefs(
+            {ref2.path: {u"sha1": u"0" * 40, u"type": GitObjectType.COMMIT}})
+        self.assertFalse(recipe.is_stale)
+
+    def test_unrelated_repository_recipe(self):
+        # On ref changes, unrelated recipes are left alone.
+        [ref] = self.factory.makeGitRefs()
+        recipe = self.factory.makeSourcePackageRecipe(
+            branches=self.factory.makeGitRefs())
+        removeSecurityProxy(recipe).is_stale = False
+        ref.repository.createOrUpdateRefs(
+            {ref.path: {u"sha1": u"0" * 40, u"type": GitObjectType.COMMIT}})
+        self.assertFalse(recipe.is_stale)
+
+
 class TestGitRepositoryDetectMerges(TestCaseWithFactory):
 
     layer = LaunchpadZopelessLayer

=== modified file 'lib/lp/code/subscribers/git.py'
--- lib/lp/code/subscribers/git.py	2015-06-12 17:55:28 +0000
+++ lib/lp/code/subscribers/git.py	2016-01-12 04:34:46 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Event subscribers for Git repositories."""
@@ -10,4 +10,5 @@
     """Some references in a Git repository have been updated."""
     repository.updateMergeCommitIDs(event.paths)
     repository.scheduleDiffUpdates(event.paths)
+    repository.markRecipesStale(event.paths)
     repository.detectMerges(event.paths, logger=event.logger)