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