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

Proposed by Tim Penhey
Status: Rejected
Rejected by: Tim Penhey
Proposed branch: lp:~thumper/launchpad/bugjam-684515-recipe-builds
Merge into: lp:launchpad
Diff against target: 71 lines (+27/-1)
4 files modified
lib/lp/code/model/sourcepackagerecipe.py (+5/-0)
lib/lp/code/model/sourcepackagerecipebuild.py (+0/-1)
lib/lp/code/model/tests/test_sourcepackagerecipe.py (+19/-0)
lib/lp/code/model/tests/test_sourcepackagerecipebuild.py (+3/-0)
To merge this branch: bzr merge lp:~thumper/launchpad/bugjam-684515-recipe-builds
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Disapprove
Review via email: mp+43730@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

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
Revision history for this message
Tim Penhey (thumper) wrote :

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.

Revision history for this message
Aaron Bentley (abentley) wrote :

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

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/model/sourcepackagerecipe.py'
2--- lib/lp/code/model/sourcepackagerecipe.py 2010-12-07 05:18:25 +0000
3+++ lib/lp/code/model/sourcepackagerecipe.py 2010-12-15 02:58:55 +0000
4@@ -246,6 +246,11 @@
5 queue_record = build.buildqueue_record
6 if manual:
7 queue_record.manualScore(queue_record.lastscore + 100)
8+ # If this build is going into the daily build archive, then we don't
9+ # want to attempt to build it during the next automatic daily build
10+ # run.
11+ if archive == self.daily_build_archive:
12+ self.is_stale = False
13 return build
14
15 def getBuilds(self, pending=False):
16
17=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
18--- lib/lp/code/model/sourcepackagerecipebuild.py 2010-11-30 12:23:01 +0000
19+++ lib/lp/code/model/sourcepackagerecipebuild.py 2010-12-15 02:58:55 +0000
20@@ -201,7 +201,6 @@
21 errorlog.globalErrorUtility.raising(info)
22 else:
23 builds.append(build)
24- recipe.is_stale = False
25 return builds
26
27 def _unqueueBuild(self):
28
29=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipe.py'
30--- lib/lp/code/model/tests/test_sourcepackagerecipe.py 2010-12-01 11:26:57 +0000
31+++ lib/lp/code/model/tests/test_sourcepackagerecipe.py 2010-12-15 02:58:55 +0000
32@@ -366,6 +366,25 @@
33 queue_record.score()
34 self.assertEqual(2605, queue_record.lastscore)
35
36+ def test_requestBuild_into_daily_ppa_resets_stale(self):
37+ # A stale recipe that is requested to build into the daily ppa is no
38+ # longer stale.
39+ recipe = self.factory.makeSourcePackageRecipe()
40+ recipe.requestBuild(recipe.daily_build_archive,
41+ recipe.owner, list(recipe.distroseries)[0],
42+ PackagePublishingPocket.RELEASE)
43+ self.assertFalse(recipe.is_stale)
44+
45+ def test_requestBuild_into_other_ppa_is_still_stale(self):
46+ # A stale recipe that is requested to build into a PPA other than the
47+ # daily PPA is still considered stale.
48+ recipe = self.factory.makeSourcePackageRecipe()
49+ ppa = self.factory.makeArchive()
50+ recipe.requestBuild(
51+ ppa, ppa.owner, list(recipe.distroseries)[0],
52+ PackagePublishingPocket.RELEASE)
53+ self.assertTrue(recipe.is_stale)
54+
55 def test_requestBuild_relative_build_score(self):
56 """Offsets for archives are respected."""
57 recipe = self.factory.makeSourcePackageRecipe()
58
59=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
60--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2010-11-19 12:22:15 +0000
61+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2010-12-15 02:58:55 +0000
62@@ -252,6 +252,9 @@
63 second_distroseries = \
64 self.factory.makeSourcePackageRecipeDistroseries("hoary")
65 recipe.distroseries.add(second_distroseries)
66+ # The scanner normally marks recipes as stale when new revisions are
67+ # pushed. For this test fake it.
68+ removeSecurityProxy(recipe).is_stale = True
69 builds = SourcePackageRecipeBuild.makeDailyBuilds()
70 self.assertEqual(
71 [second_distroseries], [build.distroseries for build in builds])