OLD | NEW |
---|---|
1 # The MB (Meta-Build wrapper) design spec | 1 # The MB (Meta-Build wrapper) design spec |
2 | 2 |
3 [TOC] | 3 [TOC] |
4 | 4 |
5 ## Intro | 5 ## Intro |
6 | 6 |
7 MB is intended to address two major aspects of the GYP -> GN transition | 7 MB is intended to address two major aspects of the GYP -> GN transition |
8 for Chromium: | 8 for Chromium: |
9 | 9 |
10 1. "bot toggling" - make it so that we can easily flip a given bot | 10 1. "bot toggling" - make it so that we can easily flip a given bot |
(...skipping 35 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... | |
46 the master name and builder name (useful on the bots so that they do not need | 46 the master name and builder name (useful on the bots so that they do not need |
47 to specify a config directly and can be hidden from the details). | 47 to specify a config directly and can be hidden from the details). |
48 | 48 |
49 See the [user guide](user_guide.md#mb_config.pyl) for details. | 49 See the [user guide](user_guide.md#mb_config.pyl) for details. |
50 | 50 |
51 ### Handling the analyze step | 51 ### Handling the analyze step |
52 | 52 |
53 The interface to `mb analyze` is described in the | 53 The interface to `mb analyze` is described in the |
54 [user\_guide](user_guide.md#mb_analyze). | 54 [user\_guide](user_guide.md#mb_analyze). |
55 | 55 |
56 The way analyze works can be subtle and complicated (see below). | |
57 | |
56 Since the interface basically mirrors the way the "analyze" step on the bots | 58 Since the interface basically mirrors the way the "analyze" step on the bots |
57 invokes gyp\_chromium today, when the config is found to be a gyp config, | 59 invokes gyp\_chromium today, when the config is found to be a gyp config, |
58 the arguments are passed straight through. | 60 the arguments are passed straight through. |
59 | 61 |
60 It implements the equivalent functionality in GN by calling `'gn refs | 62 It implements the equivalent functionality in GN by calling `'gn refs |
61 [list of files] --type=executable --all --as=output` and filtering the | 63 [list of files] --type=executable --all --as=output` and filtering the |
62 output to match the list of targets. | 64 output to match the list of targets. |
63 | 65 |
64 ## Detailed Design Requirements and Rationale | 66 ## Analyze |
67 | |
68 The goal of the `analyze` step is to speed up the cycle time of the try servers | |
69 by only building and runing the tests affected by the files in a patch, rather | |
70 than everything that might be out of date. Doing this ends up being tricky. | |
71 | |
72 We start with the following requirements and observations: | |
73 | |
74 * In an ideal (un-resource-constrained) world, we would build and test | |
75 everything that a patch affected on every patch. | |
76 | |
77 * In the real world, however, we do not have an infinite number of machines, | |
78 and try jobs are not infinitely fast, so we need to balance the desire | |
79 to get maximum test coverage against the desire to have reasonable cycle | |
80 times, given the number of machines we have. In particular, we have a | |
81 goal that most try jobs should complete in a half hour. | |
Paweł Hajdan Jr.
2015/11/06 15:27:53
Consider removing the specific number. I was not a
Dirk Pranke
2015/11/06 17:38:19
See https://groups.google.com/a/chromium.org/d/msg
Paweł Hajdan Jr.
2015/11/09 10:37:52
My understanding is that was about main waterfall,
| |
82 | |
83 * Also, since we run most try jobs against tip-of-tree Chromium, by | |
84 the time one job completes on the bot, new patches have probably landed, | |
85 rendering the build out of date. | |
86 | |
87 * This means that the next try job may have to do a build that is out of | |
88 date due to a combination of files affected by a given patch, and files | |
89 affected for unrelated reasons. We want to rebuild and test only the | |
90 targets affected by the patch, so that we don't blame or punish the | |
smut
2015/11/06 20:56:01
In the first * you said that with no resource cons
Dirk Pranke
2015/11/06 21:14:37
Actually, I meant "everything that a patch affecte
| |
91 patch author for unrelated changes. | |
92 | |
93 So: | |
94 | |
95 1. We need a way to indicate which changed files we care about and which | |
96 we don't (the affected files of a patch). | |
97 | |
98 2. We need to know which tests we might potentially want to run, and how | |
99 those are mapped onto build targets. For some kinds of tests (like | |
100 GTest-based tests), the mapping is 1:1 - if you want to run base_unittests, | |
101 you need to build base_unittests. For others (like the telemetry and | |
102 layout tests), you might need to build several executables in order to | |
103 run the tests, and that mapping might best be captured by a *meta* | |
104 target (a GN group or a GYP 'none' target like 'webkit_tests') that | |
105 depends on the right list of files. Because the GN and GYP files know | |
106 nothing about test steps, we have to have some way of mapping back | |
107 and forth between test steps and build targets. That mapping | |
108 is *not* currently available to MB (or GN or GYP), and so we have to | |
109 enough information to make it possible for the caller to do the mapping. | |
110 | |
111 3. (We might also want to know when test targets are affected by data files | |
112 that aren't compiled (python scripts, or the layout tests themselves). | |
113 There's no good way to do this in GYP, but GN supports this). | |
114 | |
115 4. We also want to ensure that particular targets still compile even if they | |
116 are not actually tested; consider testing the installers themselves, or | |
117 targets that don't yet have good test coverage. We might want to be able to | |
118 specify. We might want to use meta targets for this purpose as well. | |
119 | |
120 5. However, for some meta targets, we don't necessarily want to rebuild the | |
121 meta target itself, perhaps just the dependencies of the meta target that | |
122 are affected by the patch. For example, if you have a meta target like | |
123 'blink_tests' that might depend on ten different binaries. If a patch only | |
Paweł Hajdan Jr.
2015/11/06 15:27:53
Can we use "all" or "chromium_builder_tests" as an
Dirk Pranke
2015/11/06 17:38:19
I don't want to use 'all' because it's special and
| |
124 affects one of them (say webkit_unittests), you don't want to build | |
125 blink_tests, because that might actually also build the other nine targets. | |
126 In other words, some meta targets are *prunable*. | |
127 | |
128 6. In the ideal case we actually have enough resources and things are fast | |
129 enough that we can afford to build everything, so we need a way to indicate | |
130 that rather than actually require us to list every possible target. In other | |
131 words, we want to support 'ninja all' (or, equivalently, just 'ninja'), but | |
132 neither GN nor GYP has a built-in concept of what 'all' means, so we need to | |
133 accomodate that. | |
134 | |
135 7. In some cases, we will not be able to correctly analyze the build graph to | |
136 determine the impact of a patch, and need to bail out (e.g,. if you change a | |
137 build file itself, it may not be easy to tell how that affects the graph). | |
138 In that case we should simply build and run everything. | |
139 | |
140 The interaction between 2) and 5) means that we need to treat meta targets | |
141 two different ways, and so we need to know which targets should be | |
142 pruned in the sense of 5) and which targets should be returned unchanged | |
143 so that we can map them back to the appropriate tests. | |
144 | |
145 So, we need two things as input: | |
Paweł Hajdan Jr.
2015/11/06 15:27:53
Well, if files are counted here, then we'd need th
Dirk Pranke
2015/11/06 17:38:19
Yeah, I understand the confusion. I wasn't complet
| |
146 | |
147 * the list of files in a patch | |
148 * the list of targets which are potentially out of date | |
149 | |
150 And, we need to be able to tell, for each target, if the target is a meta | |
151 target (or 'all'), whether we should treat that target as 'prunable'. | |
152 | |
153 The way we do this is to actually take three lists as input: | |
154 | |
155 * `files`: the list of files in the patch | |
156 * `compile_targets`: the list of ninja targets to compile, determined by | |
157 computing the union of all of the compile targets for each test and | |
158 any additional targets that you might wish to compile. Any meta targets | |
159 present in this list should be pruned. | |
Dirk Pranke
2015/11/06 04:34:19
I need to go back and look at the weird static_lib
Dirk Pranke
2015/11/06 05:08:47
Actually, I also need to update this to note that
Dirk Pranke
2015/11/12 03:49:32
Okay, I've looked at the weird static library logi
| |
160 * `test_targets`: the list of ninja targets which, if out of date, should | |
161 be reported back so that we can map them back to the appropriate tests to | |
162 run. Any meta targets in this list should *not* be pruned. | |
163 | |
164 We can then return two lists as output, a list of pruned targets, and a | |
165 list of unpruned targets. | |
Dirk Pranke
2015/11/06 05:08:47
Need to update this to return a 'status' field as
| |
166 | |
167 Note that `compile_targets` and `test_targets` are actually synthesized | |
168 from the list of tests a bot is configured to run (via the mapping of | |
169 test steps to associated compile targets, and the support for | |
170 `additional_compile_targets` that are desired). | |
171 | |
172 We also have to specify how to deal with 'all': the implementation is | |
173 responsible for recognizing 'all' as a magic string and mapping it onto | |
174 the list of all root nodes in the build graph. | |
175 | |
176 We have to decide how to deal with files that don't actually exist | |
Dirk Pranke
2015/11/06 04:34:19
Note the rules I've given for error handling here
| |
177 in the build graph: this could be either the result of an error | |
178 (the file should be in the build graph, but isn't), or perfectly fine | |
179 (the file doesn't affect the build graph at all). We can't tell these | |
180 two apart, so we should ignore missing files. | |
181 | |
182 We have to decide how to deal with targets that don't exist in the build | |
183 graph: unlike missing files, this can only indicate a configuration error, | |
184 and so it should return an error accordingly. | |
sky
2015/11/09 17:17:49
Is that what happens now with an invalid target? I
Dirk Pranke
2015/11/09 20:15:20
I'm pretty sure it'll fail in a GN build. I don't
| |
185 | |
186 We have to decide how to deal with empty lists for the three | |
187 fields: | |
188 | |
189 * It doesn't make sense to call analyze at all if no files were modified, | |
190 so this should probably return an error. | |
191 | |
192 * It doesn't make sense to call analyze if you don't want to compile | |
193 anything at all, but there is a sense in which compile_targets=[] | |
194 should be treated the same as compile_targets=["all"], since that's | |
Paweł Hajdan Jr.
2015/11/06 15:27:53
I'm worried about this. Can we not do that, at lea
Dirk Pranke
2015/11/06 17:38:19
Nico seems to feel strongly that we should allow t
Paweł Hajdan Jr.
2015/11/09 10:37:52
What is the rationale for his strong opinion here?
Dirk Pranke
2015/11/09 20:15:21
Nico answered that question here:
https://groups
Dirk Pranke
2015/11/12 03:49:32
I've decided that Nico loses out :). Everyone else
| |
195 what Ninja does, so we will preserve the Ninja interpretation here. | |
196 This also means that you can get the ideal behavior of "build everything" | |
197 by not specifying everything, so it's a reasonable default. | |
198 | |
199 * On the other hand, it does make sense for test_targets=[]: it might | |
200 mean that you don't want to run any tests. And, treating [] | |
201 as ["all"] probably doesn't make any sense since no tests actually | |
202 depend on every single target in the build. | |
203 | |
204 We also have an "error" field in case something goes wrong (like the | |
Paweł Hajdan Jr.
2015/11/06 15:27:53
I'd suggest making this change the exit code of th
Dirk Pranke
2015/11/06 17:38:19
It is done already, but I'll add that to the text
| |
205 empty file list case, above, or an internal error in MB/GYP/GN). | |
206 | |
207 In an error case, or in the case where analyze doesn't know what to do, we | |
208 should default to echoing the compile_targets and test_targets back unchanged, | |
209 which will produce a conservative result where we build and run everything, | |
210 just to be safe. In this way even if analyze fails, the failure does not | |
211 have to be fatal. | |
Paweł Hajdan Jr.
2015/11/06 15:27:53
I'd like it to be fatal so that it's noticeable. I
Dirk Pranke
2015/11/06 17:38:19
There's two different cases and I shouldn't have m
Paweł Hajdan Jr.
2015/11/09 10:37:51
sg
| |
212 | |
213 ### Examples | |
214 | |
215 Continuing the example given above, suppose we have the following build | |
216 graph: | |
217 | |
218 * `blink_tests` is a meta target that depends on `webkit_unitests`, | |
219 `wtf_unittests`, and `webkit_tests` and represents all of the targets | |
220 needed to fully test Blink. Each of those is a separate test step. | |
221 * `webkit_tests` is also a meta target; it depends on `content_shell` | |
222 and `image_diff`. | |
223 * `base_unittests` is a separate test binary. | |
224 * `wtf_unittests` depends on `Assertions.cpp` and `AssertionsTest.cpp`. | |
225 * `webkit_unittests` depends on `WebNode.cpp` and `WebNodeTest.cpp`. | |
226 * `content_shell` depends on `WebNode.cpp` and `Assertions.cpp`. | |
227 * `base_unittests` depends on `logging.cc` and `logging_unittest.cc`. | |
228 | |
229 #### Example 1 | |
230 | |
231 We wish to run 'wtf_unittests' and 'webkit_tests' on a bot, but not | |
232 compile any additional targets. | |
233 | |
234 If a patch touches WebNode.cpp, then analyze gets as input: | |
235 | |
236 { | |
237 "files": ["WebNode.cpp"], | |
238 "compile_targets": ["wtf_unittests", "webkit_tests"], | |
sky
2015/11/09 17:17:49
As compile_target is the same as test_targets, doe
Dirk Pranke
2015/11/09 20:15:20
Maybe not, but I think it's easier to just specify
Dirk Pranke
2015/11/12 03:49:31
As we've discussed, I'm changing this to 'addition
| |
239 "test_targets": ["wtf_unittests", "webkit_tests"] | |
240 } | |
241 | |
242 and should return as output: | |
243 | |
244 { | |
245 "compile_targets": ["content_shell"], | |
246 "test_targets": ["webkit_tests"] | |
247 } | |
248 | |
249 Note how `webkit_tests` was pruned in compile_targets but not in test_targets. | |
250 | |
251 #### Example 2 | |
252 | |
253 Using the same patch as Example 1, assume we wish to run only `wtf_unittests`, | |
254 but additionally build everything needed to test Blink (`blink_tests`): | |
255 | |
256 We pass as input: | |
257 | |
258 { | |
259 "files": ["WebNode.cpp"], | |
260 "compile_targets": ["wtf_unittests", "blink_tests"], | |
261 "test_targets": ["wtf_unittests"] | |
262 } | |
263 | |
264 And should get as output: | |
265 | |
266 { | |
267 "compile_targets": ["content_shell"], | |
268 "test_targets": [] | |
269 } | |
270 | |
271 Note that even though `wtf_unittests` is a dependency of `blink_tests`, | |
272 we listed both as input. This is fine and to be expected, since the | |
273 bot scripts don't know that one contains the other. | |
274 | |
275 Also, we pruned `blink_tests` for the output compile_targets, but | |
276 test_targets was empty, since blink_tests was not listed in the input | |
277 test_targets. | |
278 | |
279 #### Example 3 | |
280 | |
281 Suppose we always wish to build everything on the bot, but not run any | |
282 tests. | |
283 | |
284 Input: | |
285 | |
286 { | |
287 "files": ["WebNode.cpp"], | |
288 "compile_targets": [], | |
sky
2015/11/09 17:17:49
My knee jerk reaction is that analyzer should neve
Dirk Pranke
2015/11/09 20:15:20
I understand the reaction, and obviously we can ma
Dirk Pranke
2015/11/12 03:49:32
Updated: Nico loses :).
| |
289 "test_targets": [] | |
290 } | |
291 | |
292 Output: | |
293 | |
294 { | |
295 "compile_targets": ["webkit_unittests", "content_shell"], | |
296 "test_targets": [] | |
297 } | |
298 | |
299 Note here that [] is being treated as ["all"] for compile_targets, but | |
300 not test_targets. | |
Dirk Pranke
2015/11/06 05:08:47
Need to update the examples to contain status fiel
| |
301 | |
302 | |
303 ## Random Requirements and Rationale | |
65 | 304 |
66 This section is collection of semi-organized notes on why MB is the way | 305 This section is collection of semi-organized notes on why MB is the way |
67 it is ... | 306 it is ... |
68 | 307 |
69 ### in-tree or out-of-tree | 308 ### in-tree or out-of-tree |
70 | 309 |
71 The first issue is whether or not this should exist as a script in | 310 The first issue is whether or not this should exist as a script in |
72 Chromium at all; an alternative would be to simply change the bot | 311 Chromium at all; an alternative would be to simply change the bot |
73 configurations to know whether to use GYP or GN, and which flags to | 312 configurations to know whether to use GYP or GN, and which flags to |
74 pass. | 313 pass. |
(...skipping 75 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... | |
150 * Some common flags (goma\_dir being the obvious one) may need to be | 389 * Some common flags (goma\_dir being the obvious one) may need to be |
151 specified via the user, and it's unclear how to integrate this with | 390 specified via the user, and it's unclear how to integrate this with |
152 the concept of build\_configs. | 391 the concept of build\_configs. |
153 | 392 |
154 Right now, MB has hard-coded support for a few flags (i.e., you can | 393 Right now, MB has hard-coded support for a few flags (i.e., you can |
155 pass the --goma-dir flag, and it will know to expand "${goma\_dir}" in | 394 pass the --goma-dir flag, and it will know to expand "${goma\_dir}" in |
156 the string before calling out to the tool. We may want to generalize | 395 the string before calling out to the tool. We may want to generalize |
157 this to a common key/value approach (perhaps then meeting the | 396 this to a common key/value approach (perhaps then meeting the |
158 ChromeOS non-goal, above), or we may want to keep this very strictly | 397 ChromeOS non-goal, above), or we may want to keep this very strictly |
159 limited for simplicity. | 398 limited for simplicity. |
OLD | NEW |