Merge lp:~thumper/launchpad/bugjam-684515-recipe-builds into lp:launchpad

[Tim Penhey]

If a user requests a daily build for a recipe, don't try to automatically build it.

The caveat to that is that the recipe is built into the daily build PPA.

This change does mean that if a manual build is requested for a subset of the distro series into the daily build PPA, that an automatic build of the remaining distroseries will not be attempted. However, we may want to simplify the request build process and the distroseries to not have differences - this isn't however part of the change in this branch.

[Aaron Bentley (abentley)]

This will prevent all daily builds, regardless of distroseries. In fact, the manual build could have been done deliberately for a particular distroseries, to see whether the recipe works for that distroseries.

I don't think this bug is suitable for the bug jam. It's not trivial. It probably means doing full manifest support so that we can abort early.

review: Disapprove

[Tim Penhey (thumper)]

Yes, I did mention that in the description. I think that we will be solving more problems than we create.

We are fixing the problem where users who are keen to get their new change built ASAP end up getting failed to upload problems. Given that there is a simple work-around for the situation where extra distroseries are needed to be built, I still think that this fix is worth landing.

Also, since the implementation was simple, I did think it was fine for the bug jam.

[Aaron Bentley (abentley)]

There hasn't been any apparent urgency until now. AFAIK, Robert and Jelmer are the only people talking about this.

I'd rather take a day or two and fix it properly than introduce technical debt with a quick-and-dirty fix. I recognise that a proper fix might not be appropriate during the bug jam.

Unmerged revisions

12071. By Tim Penhey on 2010-12-15

Manual builds into the daily PPA mean that we don't try the automatic daily build (unless new revisions turn up).

=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
--- lib/lp/code/model/sourcepackagerecipe.py	2010-12-07 05:18:25 +0000
+++ lib/lp/code/model/sourcepackagerecipe.py	2010-12-15 02:58:55 +0000
@@ -246,6 +246,11 @@
         queue_record = build.buildqueue_record
         if manual:
             queue_record.manualScore(queue_record.lastscore + 100)
+        # If this build is going into the daily build archive, then we don't
+        # want to attempt to build it during the next automatic daily build
+        # run.
+        if archive == self.daily_build_archive:
+            self.is_stale = False
         return build
 
     def getBuilds(self, pending=False):
 
=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py	2010-11-30 12:23:01 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py	2010-12-15 02:58:55 +0000
@@ -201,7 +201,6 @@
                     errorlog.globalErrorUtility.raising(info)
                 else:
                     builds.append(build)
-            recipe.is_stale = False
         return builds
 
     def _unqueueBuild(self):
 
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipe.py	2010-12-01 11:26:57 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py	2010-12-15 02:58:55 +0000
@@ -366,6 +366,25 @@
         queue_record.score()
         self.assertEqual(2605, queue_record.lastscore)
 
+    def test_requestBuild_into_daily_ppa_resets_stale(self):
+        # A stale recipe that is requested to build into the daily ppa is no
+        # longer stale.
+        recipe = self.factory.makeSourcePackageRecipe()
+        recipe.requestBuild(recipe.daily_build_archive,
+            recipe.owner, list(recipe.distroseries)[0],
+            PackagePublishingPocket.RELEASE)
+        self.assertFalse(recipe.is_stale)
+
+    def test_requestBuild_into_other_ppa_is_still_stale(self):
+        # A stale recipe that is requested to build into a PPA other than the
+        # daily PPA is still considered stale.
+        recipe = self.factory.makeSourcePackageRecipe()
+        ppa = self.factory.makeArchive()
+        recipe.requestBuild(
+            ppa, ppa.owner, list(recipe.distroseries)[0],
+            PackagePublishingPocket.RELEASE)
+        self.assertTrue(recipe.is_stale)
+
     def test_requestBuild_relative_build_score(self):
         """Offsets for archives are respected."""
         recipe = self.factory.makeSourcePackageRecipe()
 
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2010-11-19 12:22:15 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py	2010-12-15 02:58:55 +0000
@@ -252,6 +252,9 @@
         second_distroseries = \
             self.factory.makeSourcePackageRecipeDistroseries("hoary")
         recipe.distroseries.add(second_distroseries)
+        # The scanner normally marks recipes as stale when new revisions are
+        # pushed.  For this test fake it.
+        removeSecurityProxy(recipe).is_stale = True
         builds = SourcePackageRecipeBuild.makeDailyBuilds()
         self.assertEqual(
             [second_distroseries], [build.distroseries for build in builds])