← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/launchpad/snap-auto-build-code into lp:launchpad

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-auto-build-code into lp:launchpad with lp:~cjwatson/launchpad/snap-auto-build-model as a prerequisite.

Commit message:
Mark snap packages stale when their source branches changed, and dispatch automatic builds for them.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1593359 in Launchpad itself: "Add option to trigger snap builds when top-level branch changes"
  https://bugs.launchpad.net/launchpad/+bug/1593359

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-auto-build-code/+merge/297957

Mark snap packages stale when their source branches changed, and dispatch automatic builds for them.

It's almost like recipes, only not quite because of model differences and an expected desire to have builds appear a bit more responsively.  But it's close enough that we can reuse most of the infrastructure.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-auto-build-code into lp:launchpad.
=== modified file 'cronscripts/request_daily_builds.py'
--- cronscripts/request_daily_builds.py	2013-01-07 02:40:55 +0000
+++ cronscripts/request_daily_builds.py	2016-06-20 20:40:14 +0000
@@ -1,9 +1,9 @@
 #!/usr/bin/python -S
 #
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""Request builds for stale daily build recipes."""
+"""Request builds for stale daily build recipes and snap packages."""
 
 __metaclass__ = type
 
@@ -18,6 +18,7 @@
 from lp.services.config import config
 from lp.services.scripts.base import LaunchpadCronScript
 from lp.services.webapp.errorlog import globalErrorUtility
+from lp.snappy.interfaces.snap import ISnapSet
 
 
 class RequestDailyBuilds(LaunchpadCronScript):
@@ -32,7 +33,10 @@
         globalErrorUtility.configure(self.name)
         source = getUtility(ISourcePackageRecipeBuildSource)
         builds = source.makeDailyBuilds(self.logger)
-        self.logger.info('Requested %d daily builds.' % len(builds))
+        self.logger.info('Requested %d daily recipe builds.' % len(builds))
+        builds = getUtility(ISnapSet).makeAutoBuilds(self.logger)
+        self.logger.info(
+            'Requested %d automatic snap package builds.' % len(builds))
         transaction.commit()
 
 

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2016-06-14 12:40:30 +0000
+++ database/schema/security.cfg	2016-06-20 20:40:14 +0000
@@ -708,6 +708,8 @@
 public.revisionparent                     = SELECT, INSERT
 public.revisionproperty                   = SELECT, INSERT
 public.seriessourcepackagebranch          = SELECT
+public.snap                               = SELECT, UPDATE
+public.snapbuild                          = SELECT
 public.sourcepackagename                  = SELECT
 public.sourcepackagerecipe                = SELECT, UPDATE
 public.sourcepackagerecipedata            = SELECT
@@ -827,8 +829,13 @@
 public.distroarchseries                         = SELECT
 public.distroseries                             = SELECT
 public.job                                      = SELECT, INSERT
+public.libraryfilealias                         = SELECT
 public.person                                   = SELECT
+public.pocketchroot                             = SELECT
 public.processor                                = SELECT
+public.snap                                     = SELECT, UPDATE
+public.snaparch                                 = SELECT
+public.snapbuild                                = SELECT, INSERT
 public.sourcepackagename                        = SELECT
 public.sourcepackagerecipe                      = SELECT, UPDATE
 public.sourcepackagerecipebuild                 = SELECT, INSERT

=== modified file 'lib/lp/code/configure.zcml'
--- lib/lp/code/configure.zcml	2016-02-28 19:33:42 +0000
+++ lib/lp/code/configure.zcml	2016-06-20 20:40:14 +0000
@@ -423,6 +423,9 @@
       handler="lp.codehosting.scanner.bzrsync.update_recipes"/>
   <subscriber
       for="lp.codehosting.scanner.events.ITipChanged"
+      handler="lp.codehosting.scanner.bzrsync.update_snaps"/>
+  <subscriber
+      for="lp.codehosting.scanner.events.ITipChanged"
       handler="lp.codehosting.scanner.bzrsync.trigger_webhooks"/>
   <subscriber
       for="lp.codehosting.scanner.events.IRevisionsRemoved"

=== modified file 'lib/lp/code/interfaces/branch.py'
--- lib/lp/code/interfaces/branch.py	2015-10-12 12:58:32 +0000
+++ lib/lp/code/interfaces/branch.py	2016-06-20 20:40:14 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Branch interfaces."""
@@ -674,6 +674,9 @@
     def markRecipesStale():
         """Mark all recipes associated with this branch as stale."""
 
+    def markSnapsStale():
+        """Mark all snap packages associated with this branch as stale."""
+
     def getStackedBranches():
         """The branches that are stacked on this one."""
 

=== modified file 'lib/lp/code/interfaces/gitrepository.py'
--- lib/lp/code/interfaces/gitrepository.py	2016-05-14 09:54:32 +0000
+++ lib/lp/code/interfaces/gitrepository.py	2016-06-20 20:40:14 +0000
@@ -520,8 +520,16 @@
         """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.
+            an entry that points to this repository and that have a
+            `revspec` that is one of these paths will be marked as stale.
+        """
+
+    def markSnapsStale(paths):
+        """Mark snap packages associated with this repository as stale.
+
+        :param paths: A list of reference paths.  Any snap packages 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):

=== modified file 'lib/lp/code/model/branch.py'
--- lib/lp/code/model/branch.py	2016-06-01 17:24:35 +0000
+++ lib/lp/code/model/branch.py	2016-06-20 20:40:14 +0000
@@ -184,6 +184,7 @@
 from lp.services.webapp.interfaces import ILaunchBag
 from lp.services.webhooks.interfaces import IWebhookSet
 from lp.services.webhooks.model import WebhookTargetMixin
+from lp.snappy.interfaces.snap import ISnapSet
 
 
 @implementer(IBranch, IPrivacy, IInformationType)
@@ -667,6 +668,11 @@
         for recipe in self._recipes:
             recipe.is_stale = True
 
+    def markSnapsStale(self):
+        """See `IBranch`."""
+        for snap in getUtility(ISnapSet).findByBranch(self):
+            snap.is_stale = True
+
     def addToLaunchBag(self, launchbag):
         """See `IBranch`."""
         launchbag.add(self.product)
@@ -821,8 +827,6 @@
         As well as the dictionaries, this method returns two list of callables
         that may be called to perform the alterations and deletions needed.
         """
-        from lp.snappy.interfaces.snap import ISnapSet
-
         alteration_operations = []
         deletion_operations = []
         # Merge proposals require their source and target branches to exist.

=== modified file 'lib/lp/code/model/gitrepository.py'
--- lib/lp/code/model/gitrepository.py	2016-05-14 09:54:32 +0000
+++ lib/lp/code/model/gitrepository.py	2016-06-20 20:40:14 +0000
@@ -152,6 +152,7 @@
 from lp.services.webapp.authorization import available_with_permission
 from lp.services.webhooks.interfaces import IWebhookSet
 from lp.services.webhooks.model import WebhookTargetMixin
+from lp.snappy.interfaces.snap import ISnapSet
 
 
 object_type_map = {
@@ -1004,6 +1005,12 @@
         for recipe in self._getRecipes(paths):
             recipe.is_stale = True
 
+    def markSnapsStale(self, paths):
+        """See `IGitRepository`."""
+        snap_set = getUtility(ISnapSet)
+        for snap in snap_set.findByGitRepository(self, paths=paths):
+            snap.is_stale = True
+
     def _markProposalMerged(self, proposal, merged_revision_id, logger=None):
         if logger is not None:
             logger.info(
@@ -1058,8 +1065,6 @@
         As well as the dictionaries, this method returns two list of callables
         that may be called to perform the alterations and deletions needed.
         """
-        from lp.snappy.interfaces.snap import ISnapSet
-
         alteration_operations = []
         deletion_operations = []
         # Merge proposals require their source and target repositories to

=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py	2016-05-13 10:58:59 +0000
+++ lib/lp/code/model/tests/test_gitrepository.py	2016-06-20 20:40:14 +0000
@@ -1946,6 +1946,44 @@
         self.assertFalse(recipe.is_stale)
 
 
+class TestGitRepositoryMarkSnapsStale(TestCaseWithFactory):
+
+    layer = ZopelessDatabaseLayer
+
+    def setUp(self):
+        super(TestGitRepositoryMarkSnapsStale, self).setUp()
+        self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
+
+    def test_same_repository(self):
+        # On ref changes, snap packages using this ref become stale.
+        [ref] = self.factory.makeGitRefs()
+        snap = self.factory.makeSnap(git_ref=ref)
+        removeSecurityProxy(snap).is_stale = False
+        ref.repository.createOrUpdateRefs(
+            {ref.path: {u"sha1": u"0" * 40, u"type": GitObjectType.COMMIT}})
+        self.assertTrue(snap.is_stale)
+
+    def test_same_repository_different_ref(self):
+        # On ref changes, snap packages using a different ref in the same
+        # repository are left alone.
+        ref1, ref2 = self.factory.makeGitRefs(
+            paths=[u"refs/heads/a", u"refs/heads/b"])
+        snap = self.factory.makeSnap(git_ref=ref1)
+        removeSecurityProxy(snap).is_stale = False
+        ref1.repository.createOrUpdateRefs(
+            {ref2.path: {u"sha1": u"0" * 40, u"type": GitObjectType.COMMIT}})
+        self.assertFalse(snap.is_stale)
+
+    def test_different_repository(self):
+        # On ref changes, unrelated snap packages are left alone.
+        [ref] = self.factory.makeGitRefs()
+        snap = self.factory.makeSnap(git_ref=self.factory.makeGitRefs()[0])
+        removeSecurityProxy(snap).is_stale = False
+        ref.repository.createOrUpdateRefs(
+            {ref.path: {u"sha1": u"0" * 40, u"type": GitObjectType.COMMIT}})
+        self.assertFalse(snap.is_stale)
+
+
 class TestGitRepositoryDetectMerges(TestCaseWithFactory):
 
     layer = LaunchpadZopelessLayer

=== modified file 'lib/lp/code/scripts/tests/test_request_daily_builds.py'
--- lib/lp/code/scripts/tests/test_request_daily_builds.py	2012-06-29 14:36:44 +0000
+++ lib/lp/code/scripts/tests/test_request_daily_builds.py	2016-06-20 20:40:14 +0000
@@ -1,11 +1,13 @@
-# Copyright 2010-2012 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test the request_daily_builds script"""
 
 import transaction
 
+from lp.services.features.testing import FeatureFixture
 from lp.services.scripts.tests import run_script
+from lp.snappy.interfaces.snap import SNAP_TESTING_FLAGS
 from lp.soyuz.enums import ArchivePurpose
 from lp.testing import TestCaseWithFactory
 from lp.testing.layers import ZopelessAppServerLayer
@@ -15,24 +17,49 @@
 
     layer = ZopelessAppServerLayer
 
+    def setUp(self):
+        super(TestRequestDailyBuilds, self).setUp()
+        self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
+
     def test_request_daily_builds(self):
         """Ensure the request_daily_builds script requests daily builds."""
+        processor = self.factory.makeProcessor(supports_virtualized=True)
+        distroarchseries = self.factory.makeDistroArchSeries(
+            processor=processor)
+        fake_chroot = self.factory.makeLibraryFileAlias(
+            filename="fake_chroot.tar.gz", db_only=True)
+        distroarchseries.addOrUpdateChroot(fake_chroot)
         prod_branch = self.factory.makeProductBranch()
         prod_recipe = self.factory.makeSourcePackageRecipe(
             build_daily=True, is_stale=True, branches=[prod_branch])
+        prod_snap = self.factory.makeSnap(
+            distroseries=distroarchseries.distroseries,
+            processors=[distroarchseries.processor],
+            auto_build=True, is_stale=True, branch=prod_branch)
         pack_branch = self.factory.makePackageBranch()
         pack_recipe = self.factory.makeSourcePackageRecipe(
             build_daily=True, is_stale=True, branches=[pack_branch])
+        pack_snap = self.factory.makeSnap(
+            distroseries=distroarchseries.distroseries,
+            processors=[distroarchseries.processor],
+            auto_build=True, is_stale=True, branch=pack_branch)
         self.assertEqual(0, prod_recipe.pending_builds.count())
+        self.assertEqual(0, prod_snap.pending_builds.count())
         self.assertEqual(0, pack_recipe.pending_builds.count())
+        self.assertEqual(0, pack_snap.pending_builds.count())
         transaction.commit()
         retcode, stdout, stderr = run_script(
             'cronscripts/request_daily_builds.py', [])
-        self.assertIn('Requested 2 daily builds.', stderr)
+        self.assertIn('Requested 2 daily recipe builds.', stderr)
+        self.assertIn('Requested 2 automatic snap package builds.', stderr)
         self.assertEqual(1, prod_recipe.pending_builds.count())
+        self.assertEqual(1, prod_snap.pending_builds.count())
         self.assertEqual(1, pack_recipe.pending_builds.count())
+        self.assertEqual(1, pack_snap.pending_builds.count())
         self.assertFalse(prod_recipe.is_stale)
+        self.assertFalse(prod_snap.is_stale)
         self.assertFalse(pack_recipe.is_stale)
+        self.assertFalse(pack_snap.is_stale)
 
     def test_request_daily_builds_oops(self):
         """Ensure errors are handled cleanly."""
@@ -43,7 +70,8 @@
         retcode, stdout, stderr = run_script(
             'cronscripts/request_daily_builds.py', [])
         self.assertEqual(0, recipe.pending_builds.count())
-        self.assertIn('Requested 0 daily builds.', stderr)
+        self.assertIn('Requested 0 daily recipe builds.', stderr)
+        self.assertIn('Requested 0 automatic snap package builds.', stderr)
         self.oops_capture.sync()
         self.assertEqual('NonPPABuildRequest', self.oopses[0]['type'])
         self.assertEqual(

=== modified file 'lib/lp/code/subscribers/git.py'
--- lib/lp/code/subscribers/git.py	2016-01-12 04:34:09 +0000
+++ lib/lp/code/subscribers/git.py	2016-06-20 20:40:14 +0000
@@ -11,4 +11,5 @@
     repository.updateMergeCommitIDs(event.paths)
     repository.scheduleDiffUpdates(event.paths)
     repository.markRecipesStale(event.paths)
+    repository.markSnapsStale(event.paths)
     repository.detectMerges(event.paths, logger=event.logger)

=== modified file 'lib/lp/codehosting/scanner/bzrsync.py'
--- lib/lp/codehosting/scanner/bzrsync.py	2016-05-24 05:29:48 +0000
+++ lib/lp/codehosting/scanner/bzrsync.py	2016-06-20 20:40:14 +0000
@@ -1,6 +1,6 @@
 #!/usr/bin/python
 #
-# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2016 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Import version control metadata from a Bazaar branch into the database."""
@@ -333,6 +333,10 @@
     tip_changed.db_branch.markRecipesStale()
 
 
+def update_snaps(tip_changed):
+    tip_changed.db_branch.markSnapsStale()
+
+
 def trigger_webhooks(tip_changed):
     old_revid = tip_changed.old_tip_revision_id
     new_revid = tip_changed.new_tip_revision_id

=== modified file 'lib/lp/codehosting/scanner/tests/test_bzrsync.py'
--- lib/lp/codehosting/scanner/tests/test_bzrsync.py	2016-03-02 21:21:26 +0000
+++ lib/lp/codehosting/scanner/tests/test_bzrsync.py	2016-06-20 20:40:14 +0000
@@ -51,6 +51,7 @@
 from lp.services.database.interfaces import IStore
 from lp.services.features.testing import FeatureFixture
 from lp.services.osutils import override_environ
+from lp.snappy.interfaces.snap import SNAP_FEATURE_FLAG
 from lp.testing import TestCaseWithFactory
 from lp.testing.dbuser import (
     dbuser,
@@ -749,6 +750,32 @@
         self.assertEqual(False, recipe.is_stale)
 
 
+class TestMarkSnapsStale(BzrSyncTestCase):
+    """Test that snap packages associated with the branch are marked stale."""
+
+    def setUp(self):
+        super(TestMarkSnapsStale, self).setUp()
+        self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
+
+    @run_as_db_user(config.launchpad.dbuser)
+    def test_same_branch(self):
+        # On tip change, snap packages using this branch become stale.
+        snap = self.factory.makeSnap(branch=self.db_branch)
+        removeSecurityProxy(snap).is_stale = False
+        switch_dbuser("branchscanner")
+        self.makeBzrSync(self.db_branch).syncBranchAndClose()
+        self.assertTrue(snap.is_stale)
+
+    @run_as_db_user(config.launchpad.dbuser)
+    def test_unrelated_branch(self):
+        # On tip change, unrelated snap packages are left alone.
+        snap = self.factory.makeSnap()
+        removeSecurityProxy(snap).is_stale = False
+        switch_dbuser("branchscanner")
+        self.makeBzrSync(self.db_branch).syncBranchAndClose()
+        self.assertFalse(snap.is_stale)
+
+
 class TestTriggerWebhooks(BzrSyncTestCase):
     """Test triggering of webhooks."""
 

=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py	2016-06-20 20:40:13 +0000
+++ lib/lp/snappy/interfaces/snap.py	2016-06-20 20:40:14 +0000
@@ -527,8 +527,13 @@
     def findByBranch(branch):
         """Return all snap packages for the given Bazaar branch."""
 
-    def findByGitRepository(repository):
-        """Return all snap packages for the given Git repository."""
+    def findByGitRepository(repository, paths=None):
+        """Return all snap packages for the given Git repository.
+
+        :param repository: An `IGitRepository`.
+        :param paths: If not None, only return snap packages for one of
+            these Git reference paths.
+        """
 
     def findByGitRef(ref):
         """Return all snap packages for the given Git reference."""

=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2016-06-20 20:40:13 +0000
+++ lib/lp/snappy/model/snap.py	2016-06-20 20:40:14 +0000
@@ -628,9 +628,12 @@
         """See `ISnapSet`."""
         return IStore(Snap).find(Snap, Snap.branch == branch)
 
-    def findByGitRepository(self, repository):
+    def findByGitRepository(self, repository, paths=None):
         """See `ISnapSet`."""
-        return IStore(Snap).find(Snap, Snap.git_repository == repository)
+        clauses = [Snap.git_repository == repository]
+        if paths is not None:
+            clauses.append(Snap.git_path.is_in(paths))
+        return IStore(Snap).find(Snap, *clauses)
 
     def findByGitRef(self, ref):
         """See `ISnapSet`."""

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2016-06-20 20:40:13 +0000
+++ lib/lp/snappy/tests/test_snap.py	2016-06-20 20:40:14 +0000
@@ -721,6 +721,27 @@
         self.assertContentEqual(
             snaps[2:], snap_set.findByGitRepository(repositories[1]))
 
+    def test_findByGitRepository_paths(self):
+        # ISnapSet.findByGitRepository can restrict by reference paths.
+        repositories = [self.factory.makeGitRepository() for i in range(2)]
+        snaps = []
+        for repository in repositories:
+            for i in range(3):
+                [ref] = self.factory.makeGitRefs(repository=repository)
+                snaps.append(self.factory.makeSnap(git_ref=ref))
+        snap_set = getUtility(ISnapSet)
+        self.assertContentEqual(
+            [], snap_set.findByGitRepository(repositories[0], paths=[]))
+        self.assertContentEqual(
+            [snaps[0]],
+            snap_set.findByGitRepository(
+                repositories[0], paths=[snaps[0].git_ref.path]))
+        self.assertContentEqual(
+            snaps[:2],
+            snap_set.findByGitRepository(
+                repositories[0],
+                paths=[snaps[0].git_ref.path, snaps[1].git_ref.path]))
+
     def test_findByGitRef(self):
         # ISnapSet.findByGitRef returns all Snaps with the given Git
         # reference.


Follow ups