Merge lp:~robru/cupstream2distro/detect-new-commits-when-building into lp:cupstream2distro
- detect-new-commits-when-building
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 1160 |
Proposed branch: | lp:~robru/cupstream2distro/detect-new-commits-when-building |
Merge into: | lp:cupstream2distro |
Diff against target: |
330 lines (+85/-106) 6 files modified
citrain/build.py (+12/-32) citrain/recipes/manager.py (+1/-1) citrain/recipes/merge.py (+19/-15) tests/unit/__init__.py (+1/-0) tests/unit/test_recipe_merge.py (+2/-0) tests/unit/test_script_build.py (+50/-58) |
To merge this branch: | bzr merge lp:~robru/cupstream2distro/detect-new-commits-when-building |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot | continuous-integration | Needs Fixing | |
Robert Bruce Park (community) | Approve | ||
Review via email: mp+274960@code.launchpad.net |
Commit message
Detect which packages have new commits to build.
Description of the change
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1168
http://
Executed test runs:
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1170
http://
Executed test runs:
Click here to trigger a rebuild:
http://
Robert Bruce Park (robru) wrote : | # |
Ok I think this is making sense, will just test in staging tomorrow before merging.
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1172
http://
Executed test runs:
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:1173
http://
Executed test runs:
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:1174
http://
Executed test runs:
Click here to trigger a rebuild:
http://
Robert Bruce Park (robru) wrote : | # |
Staging instance has apparently imploded so I'm not able to test this, but I'm confident with the tests and will monitor production closely for regressions.
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
Preview Diff
1 | === modified file 'citrain/build.py' |
2 | --- citrain/build.py 2015-10-17 09:26:56 +0000 |
3 | +++ citrain/build.py 2015-10-20 09:18:51 +0000 |
4 | @@ -47,49 +47,30 @@ |
5 | |
6 | sys.path.append(os.path.dirname(os.path.dirname(__file__))) |
7 | |
8 | -from glob import glob |
9 | -from os.path import basename |
10 | - |
11 | from citrain.recipes.base import missing |
12 | +from citrain.recipes.merge import check_for_unbuilt_revids |
13 | from citrain.recipes.manager import Manager |
14 | -from cupstream2distro.silomanager import SiloState |
15 | +from cupstream2distro.silomanager import SiloState, get_dirty, splitter |
16 | from cupstream2distro.errors import BuildError, CITrainError |
17 | -from cupstream2distro.utils import SILO_DIR, env, run_script |
18 | - |
19 | - |
20 | -def find_all_uploaded(): |
21 | - """Return all source package names and versions uploaded to the ppa. |
22 | - |
23 | - :yields: Source package names that have been uploaded. |
24 | - """ |
25 | - for uploaded in glob(SILO_DIR('*_source.ppa.upload')): |
26 | - yield basename(uploaded).split('_')[0] |
27 | +from cupstream2distro.utils import env, run_script |
28 | |
29 | |
30 | class BuildManager(Manager): |
31 | """Honor PACKAGES_TO_REBUILD during building.""" |
32 | |
33 | - def include_all(self): |
34 | - """Ignore PACKAGES_TO_REBUILD and include all.""" |
35 | - self.choose_packages = super().choose_packages |
36 | - self.types.clear() |
37 | - |
38 | def choose_packages(self): |
39 | """Decide which packages need to be built.""" |
40 | - self.names = set(env.PACKAGES_TO_REBUILD.split() |
41 | - or self.silo_state.all_projects) |
42 | + all_projects = set(self.silo_state.all_projects) |
43 | + self.names = set(splitter(env.PACKAGES_TO_REBUILD) or all_projects) |
44 | self.names.update(missing(self.names)) |
45 | include_all = 'true' in (env.FORCE_REBUILD, env.WATCH_ONLY) |
46 | if not (include_all or env.PACKAGES_TO_REBUILD): |
47 | - # Don't rebuild existing builds without forcing. |
48 | - previous = self.names & set(find_all_uploaded()) |
49 | - if previous: |
50 | - logging.info('{} already built, skipping.'.format( |
51 | - ', '.join(previous))) |
52 | - self.names -= previous |
53 | - if self.names: |
54 | - logging.info('Including {}.'.format(', '.join(self.names))) |
55 | - else: |
56 | + for source, merges in self.silo_state.mps.items(): |
57 | + if check_for_unbuilt_revids(source, merges) == []: |
58 | + logging.info(source + ' has no new commits, skipping.') |
59 | + self.names.discard(source) |
60 | + self.names.update(set(get_dirty(env.SILONAME)) & all_projects) |
61 | + if not self.names: |
62 | raise BuildError( |
63 | 'No packages are being considered. You probably want one of ' |
64 | 'PACKAGES_TO_REBUILD, FORCE_REBUILD, or WATCH_ONLY.') |
65 | @@ -106,8 +87,7 @@ |
66 | manager.do('validate') |
67 | if env.WATCH_ONLY != 'true': |
68 | manager.do('clean', 'collect', 'build', 'upload') |
69 | - manager.include_all() |
70 | - manager.do('watch', 'diff') |
71 | + Manager(silo_state).do('watch', 'diff') |
72 | silo_state.status = 'Packages built.' |
73 | except CITrainError as err: |
74 | logging.error(str(err)) |
75 | |
76 | === modified file 'citrain/recipes/manager.py' |
77 | --- citrain/recipes/manager.py 2015-10-07 19:47:09 +0000 |
78 | +++ citrain/recipes/manager.py 2015-10-20 09:18:51 +0000 |
79 | @@ -54,6 +54,7 @@ |
80 | self.choose_packages() |
81 | self.choose_classes() |
82 | self.instantiate_build_objects() |
83 | + logging.info('Including {}.'.format(', '.join(sorted(self.names)))) |
84 | for phase in phases: |
85 | self.execute_phase(phase) |
86 | |
87 | @@ -64,7 +65,6 @@ |
88 | def choose_packages(self): |
89 | """Act on all silo packages.""" |
90 | self.names = set(self.silo_state.all_projects) |
91 | - logging.info('Including {}.'.format(', '.join(self.names))) |
92 | |
93 | def choose_classes(self): |
94 | """Decide which class to use for each individual package build.""" |
95 | |
96 | === modified file 'citrain/recipes/merge.py' |
97 | --- citrain/recipes/merge.py 2015-10-19 22:13:22 +0000 |
98 | +++ citrain/recipes/merge.py 2015-10-20 09:18:51 +0000 |
99 | @@ -77,6 +77,21 @@ |
100 | raise PrepError('Not all MPs targeting same branch.') |
101 | |
102 | |
103 | +def check_for_unbuilt_revids(source, merges): |
104 | + """Return a list of MPs that have new commits since last build.""" |
105 | + unbuilt = [] |
106 | + logging.info('Checking {} for new commits...'.format(source)) |
107 | + # TODO: check also the merge target revids, which only just started being |
108 | + # recorded and need time to appear in production. |
109 | + for merge in merges: |
110 | + url = merge.web_link |
111 | + revid = Rev(source, url).read() |
112 | + if merge.source_branch.last_scanned_id != revid: |
113 | + unbuilt.append(url) |
114 | + logging.info('New commits: ' + url) |
115 | + return unbuilt |
116 | + |
117 | + |
118 | class Merge(BuildBase): |
119 | """Build a source package from a series of merge proposals. |
120 | |
121 | @@ -86,18 +101,6 @@ |
122 | all_mps = {} |
123 | merges = None |
124 | |
125 | - def check_for_unbuilt_revids(self): |
126 | - """Return a list of MPs that have new commits since last build.""" |
127 | - unbuilt = [] |
128 | - logging.info('Checking {} for new commits...'.format(self.name)) |
129 | - for merge in self.merges: |
130 | - url = merge.web_link |
131 | - revid = Rev(self.name, url).read() |
132 | - if merge.source_branch.last_scanned_id != revid: |
133 | - unbuilt.append(url) |
134 | - logging.error('New commits: ' + url) |
135 | - return unbuilt |
136 | - |
137 | def enforce_merge_states(self, acceptable): |
138 | """Block when merges have bad states.""" |
139 | # (\/) (°,,,°) (\/) |
140 | @@ -128,8 +131,9 @@ |
141 | """ |
142 | logging.info('Preparing merges for {}.'.format(self.name)) |
143 | merges = self.merges = reorder_branches(self.merges) |
144 | - target = merges[0].target_branch.web_link |
145 | - self.get_branch(target) |
146 | + target = merges[0].target_branch |
147 | + Rev(self.name, target.web_link).write(target.last_scanned_id) |
148 | + self.get_branch(target.web_link) |
149 | trunk_version = self.get_package_version() |
150 | DotProject.soft_save(self.name, dest=trunk_version) |
151 | for merge in merges: |
152 | @@ -238,7 +242,7 @@ |
153 | |
154 | def unbuilt_phase(self): |
155 | """Block publication of merges that have new commits.""" |
156 | - if self.check_for_unbuilt_revids(): |
157 | + if check_for_unbuilt_revids(self.name, self.merges): |
158 | self.failures.add(self.name + ' has new, unbuilt commits.') |
159 | |
160 | def merge_phase(self): |
161 | |
162 | === modified file 'tests/unit/__init__.py' |
163 | --- tests/unit/__init__.py 2015-09-22 06:53:49 +0000 |
164 | +++ tests/unit/__init__.py 2015-10-20 09:18:51 +0000 |
165 | @@ -144,6 +144,7 @@ |
166 | self.script.Archive = Mock() |
167 | self.script.Branch = Mock() |
168 | self.script.DotProject = Mock() |
169 | + self.script.Manager = Mock() |
170 | self.script.Package = Mock() |
171 | self.script.Rev = Mock() |
172 | self.script.SiloState = Mock() |
173 | |
174 | === modified file 'tests/unit/test_recipe_merge.py' |
175 | --- tests/unit/test_recipe_merge.py 2015-10-19 22:13:22 +0000 |
176 | +++ tests/unit/test_recipe_merge.py 2015-10-20 09:18:51 +0000 |
177 | @@ -242,6 +242,8 @@ |
178 | ]) |
179 | authors = merge.get_remote_branch_authors() |
180 | self.assertEqual(rev_mock.mock_calls, [ |
181 | + call('a', merge1.target_branch.web_link), |
182 | + call().write(merge1.target_branch.last_scanned_id), |
183 | call('a', merge1.web_link), |
184 | call().write(merge1.source_branch.last_scanned_id), |
185 | call('a', merge2.web_link), |
186 | |
187 | === modified file 'tests/unit/test_script_build.py' |
188 | --- tests/unit/test_script_build.py 2015-10-17 09:26:56 +0000 |
189 | +++ tests/unit/test_script_build.py 2015-10-20 09:18:51 +0000 |
190 | @@ -56,52 +56,40 @@ |
191 | self.series = Mock() |
192 | self.ppa = Mock() |
193 | |
194 | - def test_find_all_uploaded(self): |
195 | - """Find everything that has been uploaded from this silo.""" |
196 | - self.script.glob.return_value = [ |
197 | - '/path/to/foo_0.1_source.ppa.upload', |
198 | - '/path/to/bar_0.2_source.ppa.upload', |
199 | - ] |
200 | - self.assertEqual( |
201 | - sorted(self.script.find_all_uploaded()), ['bar', 'foo']) |
202 | - |
203 | - def test_find_all_uploaded_multiple_series(self): |
204 | - """We can handle uploading multiple series from a single silo.""" |
205 | - self.script.glob.return_value = [ |
206 | - '/path/to/foo_0.1_source.ppa.upload', |
207 | - '/path/to/foo_0.1~utopic_source.ppa.upload', |
208 | - '/path/to/bar_0.2_source.ppa.upload', |
209 | - '/path/to/bar_0.2~utopic_source.ppa.upload', |
210 | - ] |
211 | - self.assertEqual( |
212 | - sorted(self.script.find_all_uploaded()), |
213 | - ['bar', 'bar', 'foo', 'foo']) |
214 | - self.assertEqual( |
215 | - set(self.script.find_all_uploaded()), set(['foo', 'bar'])) |
216 | - |
217 | - def test_buildmanager_include_all(self): |
218 | - """Correctly switch to watching all packages.""" |
219 | - bman = self.script.BuildManager(Mock()) |
220 | - bman.types['foo'] = 'bar' |
221 | - self.assertEqual( |
222 | - bman.choose_packages.__doc__, |
223 | - 'Decide which packages need to be built.') |
224 | - bman.include_all() |
225 | - self.assertEqual( |
226 | - bman.choose_packages.__doc__, |
227 | - 'Act on all silo packages.') |
228 | - self.assertFalse(bman.types) |
229 | - |
230 | def test_buildmanager_choose_packages(self): |
231 | """Determine what packages to act upon.""" |
232 | silo_state = Mock( |
233 | - all_projects=['a', 'b', 'c'], mps=['a'], sources=['b', 'c']) |
234 | - self.script.env.PACKAGES_TO_REBUILD = '' |
235 | - self.script.env.FORCE_REBUILD = 'false' |
236 | - self.script.find_all_uploaded = Mock(return_value=['c']) |
237 | - bman = self.script.BuildManager(silo_state) |
238 | - bman.choose_packages() |
239 | - self.assertEqual(bman.names, set(['a', 'b'])) |
240 | + all_projects=list('abcd'), |
241 | + mps=dict( |
242 | + c=[Mock( |
243 | + source_branch=Mock(last_scanned_id=None), |
244 | + web_link='foo/123')], |
245 | + d=[Mock( |
246 | + source_branch=Mock(last_scanned_id='stale'), |
247 | + web_link='foo/789')]), |
248 | + sources=list('abcd')) |
249 | + self.script.env.PACKAGES_TO_REBUILD = '' |
250 | + self.script.env.FORCE_REBUILD = 'false' |
251 | + bman = self.script.BuildManager(silo_state) |
252 | + bman.choose_packages() |
253 | + self.assertEqual(bman.names, {'a', 'b', 'd'}) |
254 | + |
255 | + def test_buildmanager_choose_packages_dirty(self): |
256 | + """Include dirty packages by default.""" |
257 | + self.script.get_dirty = Mock(return_value=list('abcde')) |
258 | + silo_state = Mock( |
259 | + all_projects=list('abcd'), |
260 | + mps=dict( |
261 | + c=[Mock( |
262 | + source_branch=Mock(last_scanned_id=None), |
263 | + web_link='foo/123')]), |
264 | + sources=list('abcd')) |
265 | + self.script.env.PACKAGES_TO_REBUILD = '' |
266 | + self.script.env.FORCE_REBUILD = 'false' |
267 | + bman = self.script.BuildManager(silo_state) |
268 | + bman.choose_packages() |
269 | + self.script.get_dirty.assert_called_once_with(self.tempdir) |
270 | + self.assertEqual(bman.names, set('abcd')) |
271 | |
272 | def test_buildmanager_choose_packages_twins(self): |
273 | """Always add twins for all builds.""" |
274 | @@ -116,10 +104,14 @@ |
275 | def test_buildmanager_choose_packages_none_found(self): |
276 | """Fail when no packages to act upon.""" |
277 | silo_state = Mock( |
278 | - all_projects=['a', 'b', 'c'], mps=['a'], sources=['b', 'c']) |
279 | + all_projects=['a'], |
280 | + mps=dict(a=[Mock( |
281 | + source_branch=Mock(last_scanned_id=None), |
282 | + web_link='foo/123')]), |
283 | + sources=[]) |
284 | self.script.env.PACKAGES_TO_REBUILD = '' |
285 | self.script.env.FORCE_REBUILD = 'false' |
286 | - self.script.find_all_uploaded = Mock(return_value=list('abc')) |
287 | + self.script.env.WATCH_ONLY = 'false' |
288 | bman = self.script.BuildManager(silo_state) |
289 | with self.assertRaisesRegexp(BuildError, 'No packages are being'): |
290 | bman.choose_packages() |
291 | @@ -132,13 +124,13 @@ |
292 | bman = self.script.BuildManager = Mock() |
293 | self.assertEqual(self.script.main(bman), 0) |
294 | bman.assert_called_once_with(silo_state) |
295 | - self.assertEqual( |
296 | - bman.return_value.mock_calls, [ |
297 | - call.do('validate'), |
298 | - call.do('clean', 'collect', 'build', 'upload'), |
299 | - call.include_all(), |
300 | - call.do('watch', 'diff'), |
301 | - ]) |
302 | + self.assertEqual(bman.return_value.mock_calls, [ |
303 | + call.do('validate'), |
304 | + call.do('clean', 'collect', 'build', 'upload'), |
305 | + ]) |
306 | + self.assertEqual(self.script.Manager.return_value.mock_calls, [ |
307 | + call.do('watch', 'diff'), |
308 | + ]) |
309 | self.assertEqual(silo_state.status, 'Packages built.') |
310 | silo_state.mark_dirty.assert_called_once_with() |
311 | silo_state.save_config.assert_called_once_with() |
312 | @@ -150,12 +142,12 @@ |
313 | bman = self.script.BuildManager = Mock() |
314 | self.assertEqual(self.script.main(bman), 0) |
315 | bman.assert_called_once_with(silo_state) |
316 | - self.assertEqual( |
317 | - bman.return_value.mock_calls, [ |
318 | - call.do('validate'), |
319 | - call.include_all(), |
320 | - call.do('watch', 'diff'), |
321 | - ]) |
322 | + self.assertEqual(bman.return_value.mock_calls, [ |
323 | + call.do('validate'), |
324 | + ]) |
325 | + self.assertEqual(self.script.Manager.return_value.mock_calls, [ |
326 | + call.do('watch', 'diff'), |
327 | + ]) |
328 | silo_state.save_config.assert_called_once_with() |
329 | |
330 | def test_main_error(self): |
TODO:
* write tests
* test in staging
* should probably also scan trunk revids
* should also include packages marked dirty.