Changes to generator scripts in BUILD.gn only breaks in a clean build. |
||
Issue descriptionIn this patch: https://codereview.chromium.org/2794853002, I split a Python code generator (make_computed_style_base) into two separate generators (make_computed_style_base and make_css_value_id_mappings). I added make_css_value_id_mappings to BUILD.gn as a target, but forgot to add the target to the 'targets_generating_sources' list. This meant that the output file of make_css_value_id_mappings would not be generated. However, the patch passed all the trybots, and failed to compile on the buildbots due to the missing output of make_css_value_id_mappings, causing a tree closure ( crbug.com/710778 ). What I think happened on the trybots was: 1. The output of make_css_value_id_mappings was originally generated from make_computed_style_base, so the output file was already in the build directory when the trybots were building the patch. 3. Only make_computed_style_base was run. 4. The code compiled fine because the output file was there, even though it was never generated. But obviously on a clean build, this would cause a compile error. Is there anything we can do to prevent such problems in the future? Having the same code pass trybots but fail buildbots is really annoying.
,
Apr 14 2017
This is exactly the sort of thing that you'd expect to work in an incremental build and break in a clobber build. We used to have a clobber build run as part of the CQ but I took it out a few months ago when we were having capacity issues. We could add the builder back in at this point, but I'm not sure if it's worth it; I don't yet have a good sense of how often something needs to catch failures to be worth it. This is also the sort of thing that `gn check` could catch, except that I think we intentionally don't check generated files because it'd be too expensive to do so. You can certainly write an action() that takes as arguments the list of outputs that are expected, but I don't think we'd do that automatically (I'm not even sure how we'd do that unless we set an env var with the list or something). I also don't know that that would've caught the problem?
,
Nov 10 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by alancutter@chromium.org
, Apr 13 2017