Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(7)

Side by Side Diff: tools/mb/docs/design_spec.md

Issue 2585743002: Move tools/mb -> tools-webrtc/mb (Closed)
Patch Set: Fixed presubmits and tests Created 4 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « tools/mb/docs/README.md ('k') | tools/mb/docs/user_guide.md » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
(Empty)
1 # The MB (Meta-Build wrapper) design spec
2
3 [TOC]
4
5 ## Intro
6
7 MB is intended to address two major aspects of the GYP -> GN transition
8 for Chromium:
9
10 1. "bot toggling" - make it so that we can easily flip a given bot
11 back and forth between GN and GYP.
12
13 2. "bot configuration" - provide a single source of truth for all of
14 the different configurations (os/arch/`gyp_define` combinations) of
15 Chromium that are supported.
16
17 MB must handle at least the `gen` and `analyze` steps on the bots, i.e.,
18 we need to wrap both the `gyp_chromium` invocation to generate the
19 Ninja files, and the `analyze` step that takes a list of modified files
20 and a list of targets to build and returns which targets are affected by
21 the files.
22
23 For more information on how to actually use MB, see
24 [the user guide](user_guide.md).
25
26 ## Design
27
28 MB is intended to be as simple as possible, and to defer as much work as
29 possible to GN or GYP. It should live as a very simple Python wrapper
30 that offers little in the way of surprises.
31
32 ### Command line
33
34 It is structured as a single binary that supports a list of subcommands:
35
36 * `mb gen -c linux_rel_bot //out/Release`
37 * `mb analyze -m tryserver.chromium.linux -b linux_rel /tmp/input.json /tmp/outp ut.json`
38
39 ### Configurations
40
41 `mb` will first look for a bot config file in a set of different locations
42 (initially just in //ios/build/bots). Bot config files are JSON files that
43 contain keys for 'GYP_DEFINES' (a list of strings that will be joined together
44 with spaces and passed to GYP, or a dict that will be similarly converted),
45 'gn_args' (a list of strings that will be joined together), and an
46 'mb_type' field that says whether to use GN or GYP. Bot config files
47 require the full list of settings to be given explicitly.
48
49 If no matching bot config file is found, `mb` looks in the
50 `//tools/mb/mb_config.pyl` config file to determine whether to use GYP or GN
51 for a particular build directory, and what set of flags (`GYP_DEFINES` or `gn
52 args`) to use.
53
54 A config can either be specified directly (useful for testing) or by specifying
55 the master name and builder name (useful on the bots so that they do not need
56 to specify a config directly and can be hidden from the details).
57
58 See the [user guide](user_guide.md#mb_config.pyl) for details.
59
60 ### Handling the analyze step
61
62 The interface to `mb analyze` is described in the
63 [user\_guide](user_guide.md#mb_analyze).
64
65 The way analyze works can be subtle and complicated (see below).
66
67 Since the interface basically mirrors the way the "analyze" step on the bots
68 invokes `gyp_chromium` today, when the config is found to be a gyp config,
69 the arguments are passed straight through.
70
71 It implements the equivalent functionality in GN by calling `gn refs
72 [list of files] --type=executable --all --as=output` and filtering the
73 output to match the list of targets.
74
75 ## Analyze
76
77 The goal of the `analyze` step is to speed up the cycle time of the try servers
78 by only building and running the tests affected by the files in a patch, rather
79 than everything that might be out of date. Doing this ends up being tricky.
80
81 We start with the following requirements and observations:
82
83 * In an ideal (un-resource-constrained) world, we would build and test
84 everything that a patch affected on every patch. This does not
85 necessarily mean that we would build 'all' on every patch (see below).
86
87 * In the real world, however, we do not have an infinite number of machines,
88 and try jobs are not infinitely fast, so we need to balance the desire
89 to get maximum test coverage against the desire to have reasonable cycle
90 times, given the number of machines we have.
91
92 * Also, since we run most try jobs against tip-of-tree Chromium, by
93 the time one job completes on the bot, new patches have probably landed,
94 rendering the build out of date.
95
96 * This means that the next try job may have to do a build that is out of
97 date due to a combination of files affected by a given patch, and files
98 affected for unrelated reasons. We want to rebuild and test only the
99 targets affected by the patch, so that we don't blame or punish the
100 patch author for unrelated changes.
101
102 So:
103
104 1. We need a way to indicate which changed files we care about and which
105 we don't (the affected files of a patch).
106
107 2. We need to know which tests we might potentially want to run, and how
108 those are mapped onto build targets. For some kinds of tests (like
109 GTest-based tests), the mapping is 1:1 - if you want to run base_unittests,
110 you need to build base_unittests. For others (like the telemetry and
111 layout tests), you might need to build several executables in order to
112 run the tests, and that mapping might best be captured by a *meta*
113 target (a GN group or a GYP 'none' target like `webkit_tests`) that
114 depends on the right list of files. Because the GN and GYP files know
115 nothing about test steps, we have to have some way of mapping back
116 and forth between test steps and build targets. That mapping
117 is *not* currently available to MB (or GN or GYP), and so we have to
118 enough information to make it possible for the caller to do the mapping.
119
120 3. We might also want to know when test targets are affected by data files
121 that aren't compiled (python scripts, or the layout tests themselves).
122 There's no good way to do this in GYP, but GN supports this.
123
124 4. We also want to ensure that particular targets still compile even if they
125 are not actually tested; consider testing the installers themselves, or
126 targets that don't yet have good test coverage. We might want to use meta
127 targets for this purpose as well.
128
129 5. However, for some meta targets, we don't necessarily want to rebuild the
130 meta target itself, perhaps just the dependencies of the meta target that
131 are affected by the patch. For example, if you have a meta target like
132 `blink_tests` that might depend on ten different test binaries. If a patch
133 only affects one of them (say `wtf_unittests`), you don't want to
134 build `blink_tests`, because that might actually also build the other nine
135 targets. In other words, some meta targets are *prunable*.
136
137 6. As noted above, in the ideal case we actually have enough resources and
138 things are fast enough that we can afford to build everything affected by a
139 patch, but listing every possible target explicitly would be painful. The
140 GYP and GN Ninja generators provide an 'all' target that captures (nearly,
141 see [crbug.com/503241](crbug.com/503241)) everything, but unfortunately
142 neither GN nor GYP actually represents 'all' as a meta target in the build
143 graph, so we will need to write code to handle that specially.
144
145 7. In some cases, we will not be able to correctly analyze the build graph to
146 determine the impact of a patch, and need to bail out (e.g,. if you change a
147 build file itself, it may not be easy to tell how that affects the graph).
148 In that case we should simply build and run everything.
149
150 The interaction between 2) and 5) means that we need to treat meta targets
151 two different ways, and so we need to know which targets should be
152 pruned in the sense of 5) and which targets should be returned unchanged
153 so that we can map them back to the appropriate tests.
154
155 So, we need three things as input:
156
157 * `files`: the list of files in the patch
158 * `test_targets`: the list of ninja targets which, if affected by a patch,
159 should be reported back so that we can map them back to the appropriate
160 tests to run. Any meta targets in this list should *not* be pruned.
161 * `additional_compile_targets`: the list of ninja targets we wish to compile
162 *in addition to* the list in `test_targets`. Any meta targets
163 present in this list should be pruned (we don't need to return the
164 meta targets because they aren't mapped back to tests, and we don't want
165 to build them because we might build too much).
166
167 We can then return two lists as output:
168
169 * `compile_targets`, which is a list of pruned targets to be
170 passed to Ninja to build. It is acceptable to replace a list of
171 pruned targets by a meta target if it turns out that all of the
172 dependendencies of the target are affected by the patch (i.e.,
173 all ten binaries that blink_tests depends on), but doing so is
174 not required.
175 * `test_targets`, which is a list of unpruned targets to be mapped
176 back to determine which tests to run.
177
178 There may be substantial overlap between the two lists, but there is
179 no guarantee that one is a subset of the other and the two cannot be
180 used interchangeably or merged together without losing information and
181 causing the wrong thing to happen.
182
183 The implementation is responsible for recognizing 'all' as a magic string
184 and mapping it onto the list of all root nodes in the build graph.
185
186 There may be files listed in the input that don't actually exist in the build
187 graph: this could be either the result of an error (the file should be in the
188 build graph, but isn't), or perfectly fine (the file doesn't affect the build
189 graph at all). We can't tell these two apart, so we should ignore missing
190 files.
191
192 There may be targets listed in the input that don't exist in the build
193 graph; unlike missing files, this can only indicate a configuration error,
194 and so we should return which targets are missing so the caller can
195 treat this as an error, if so desired.
196
197 Any of the three inputs may be an empty list:
198
199 * It normally doesn't make sense to call analyze at all if no files
200 were modified, but in rare cases we can hit a race where we try to
201 test a patch after it has already been committed, in which case
202 the list of modified files is empty. We should return 'no dependency'
203 in that case.
204
205 * Passing an empty list for one or the other of test_targets and
206 additional_compile_targets is perfectly sensible: in the former case,
207 it can indicate that you don't want to run any tests, and in the latter,
208 it can indicate that you don't want to do build anything else in
209 addition to the test targets.
210
211 * It doesn't make sense to call analyze if you don't want to compile
212 anything at all, so passing [] for both test_targets and
213 additional_compile_targets should probably return an error.
214
215 In the output case, an empty list indicates that there was nothing to
216 build, or that there were no affected test targets as appropriate.
217
218 Note that passing no arguments to Ninja is equivalent to passing
219 `all` to Ninja (at least given how GN and GYP work); however, we
220 don't want to take advantage of this in most cases because we don't
221 actually want to build every out of date target, only the targets
222 potentially affected by the files. One could try to indicate
223 to analyze that we wanted to use no arguments instead of an empty
224 list, but using the existing fields for this seems fragile and/or
225 confusing, and adding a new field for this seems unwarranted at this time.
226
227 There is an "error" field in case something goes wrong (like the
228 empty file list case, above, or an internal error in MB/GYP/GN). The
229 analyze code should also return an error code to the shell if appropriate
230 to indicate that the command failed.
231
232 In the case where build files themselves are modified and analyze may
233 not be able to determine a correct answer (point 7 above, where we return
234 "Found dependency (all)"), we should also return the `test_targets` unmodified
235 and return the union of `test_targets` and `additional_compile_targets` for
236 `compile_targets`, to avoid confusion.
237
238 ### Examples
239
240 Continuing the example given above, suppose we have the following build
241 graph:
242
243 * `blink_tests` is a meta target that depends on `webkit_unit_tests`,
244 `wtf_unittests`, and `webkit_tests` and represents all of the targets
245 needed to fully test Blink. Each of those is a separate test step.
246 * `webkit_tests` is also a meta target; it depends on `content_shell`
247 and `image_diff`.
248 * `base_unittests` is a separate test binary.
249 * `wtf_unittests` depends on `Assertions.cpp` and `AssertionsTest.cpp`.
250 * `webkit_unit_tests` depends on `WebNode.cpp` and `WebNodeTest.cpp`.
251 * `content_shell` depends on `WebNode.cpp` and `Assertions.cpp`.
252 * `base_unittests` depends on `logging.cc` and `logging_unittest.cc`.
253
254 #### Example 1
255
256 We wish to run 'wtf_unittests' and 'webkit_tests' on a bot, but not
257 compile any additional targets.
258
259 If a patch touches WebNode.cpp, then analyze gets as input:
260
261 {
262 "files": ["WebNode.cpp"],
263 "test_targets": ["wtf_unittests", "webkit_tests"],
264 "additional_compile_targets": []
265 }
266
267 and should return as output:
268
269 {
270 "status": "Found dependency",
271 "compile_targets": ["webkit_unit_tests"],
272 "test_targets": ["webkit_tests"]
273 }
274
275 Note how `webkit_tests` was pruned in compile_targets but not in test_targets.
276
277 #### Example 2
278
279 Using the same patch as Example 1, assume we wish to run only `wtf_unittests`,
280 but additionally build everything needed to test Blink (`blink_tests`):
281
282 We pass as input:
283
284 {
285 "files": ["WebNode.cpp"],
286 "test_targets": ["wtf_unittests"],
287 "additional_compile_targets": ["blink_tests"]
288 }
289
290 And should get as output:
291
292 {
293 "status": "Found dependency",
294 "compile_targets": ["webkit_unit_tests"],
295 "test_targets": []
296 }
297
298 Here `blink_tests` was pruned in the output compile_targets, and
299 test_targets was empty, since blink_tests was not listed in the input
300 test_targets.
301
302 #### Example 3
303
304 Build everything, but do not run any tests.
305
306 Input:
307
308 {
309 "files": ["WebNode.cpp"],
310 "test_targets": [],
311 "additional_compile_targets": ["all"]
312 }
313
314 Output:
315
316 {
317 "status": "Found dependency",
318 "compile_targets": ["webkit_unit_tests", "content_shell"],
319 "test_targets": []
320 }
321
322 #### Example 4
323
324 Same as Example 2, but a build file was modified instead of a source file.
325
326 Input:
327
328 {
329 "files": ["BUILD.gn"],
330 "test_targets": ["wtf_unittests"],
331 "additional_compile_targets": ["blink_tests"]
332 }
333
334 Output:
335
336 {
337 "status": "Found dependency (all)",
338 "compile_targets": ["webkit_unit_tests", "wtf_unittests"],
339 "test_targets": ["wtf_unittests"]
340 }
341
342 test_targets was returned unchanged, compile_targets was pruned.
343
344 ## Random Requirements and Rationale
345
346 This section is collection of semi-organized notes on why MB is the way
347 it is ...
348
349 ### in-tree or out-of-tree
350
351 The first issue is whether or not this should exist as a script in
352 Chromium at all; an alternative would be to simply change the bot
353 configurations to know whether to use GYP or GN, and which flags to
354 pass.
355
356 That would certainly work, but experience over the past two years
357 suggests a few things:
358
359 * we should push as much logic as we can into the source repositories
360 so that they can be versioned and changed atomically with changes to
361 the product code; having to coordinate changes between src/ and
362 build/ is at best annoying and can lead to weird errors.
363 * the infra team would really like to move to providing
364 product-independent services (i.e., not have to do one thing for
365 Chromium, another for NaCl, a third for V8, etc.).
366 * we found that during the SVN->GIT migration the ability to flip bot
367 configurations between the two via changes to a file in chromium
368 was very useful.
369
370 All of this suggests that the interface between bots and Chromium should
371 be a simple one, hiding as much of the chromium logic as possible.
372
373 ### Why not have MB be smarter about de-duping flags?
374
375 This just adds complexity to the MB implementation, and duplicates logic
376 that GYP and GN already have to support anyway; in particular, it might
377 require MB to know how to parse GYP and GN values. The belief is that
378 if MB does *not* do this, it will lead to fewer surprises.
379
380 It will not be hard to change this if need be.
381
382 ### Integration w/ gclient runhooks
383
384 On the bots, we will disable `gyp_chromium` as part of runhooks (using
385 `GYP_CHROMIUM_NO_ACTION=1`), so that mb shows up as a separate step.
386
387 At the moment, we expect most developers to either continue to use
388 `gyp_chromium` in runhooks or to disable at as above if they have no
389 use for GYP at all. We may revisit how this works once we encourage more
390 people to use GN full-time (i.e., we might take `gyp_chromium` out of
391 runhooks altogether).
392
393 ### Config per flag set or config per (os/arch/flag set)?
394
395 Currently, mb_config.pyl does not specify the host_os, target_os, host_cpu, or
396 target_cpu values for every config that Chromium runs on, it only specifies
397 them for when the values need to be explicitly set on the command line.
398
399 Instead, we have one config per unique combination of flags only.
400
401 In other words, rather than having `linux_rel_bot`, `win_rel_bot`, and
402 `mac_rel_bot`, we just have `rel_bot`.
403
404 This design allows us to determine easily all of the different sets
405 of flags that we need to support, but *not* which flags are used on which
406 host/target combinations.
407
408 It may be that we should really track the latter. Doing so is just a
409 config file change, however.
410
411 ### Non-goals
412
413 * MB is not intended to replace direct invocation of GN or GYP for
414 complicated build scenarios (aka ChromeOS), where multiple flags need
415 to be set to user-defined paths for specific toolchains (e.g., where
416 ChromeOS needs to specify specific board types and compilers).
417
418 * MB is not intended at this time to be something developers use frequently,
419 or to add a lot of features to. We hope to be able to get rid of it once
420 the GYP->GN migration is done, and so we should not add things for
421 developers that can't easily be added to GN itself.
422
423 * MB is not intended to replace the
424 [CR tool](https://code.google.com/p/chromium/wiki/CRUserManual). Not
425 only is it only intended to replace the gyp\_chromium part of `'gclient
426 runhooks'`, it is not really meant as a developer-facing tool.
OLDNEW
« no previous file with comments | « tools/mb/docs/README.md ('k') | tools/mb/docs/user_guide.md » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698