Sanity check that expected .cc/.h files are generated by the bindings compiler |
||||||
Issue descriptionIn https://chromium-review.googlesource.com/c/chromium/src/+/760542 I removed the last uses of '(sequence<Dictionary> or Dictionary)' in an IDL file. I did not initially remove dictionary_sequence_or_dictionary.{cc,h} from bindings/core/v8/BUILD.gn, because I wasn't aware of its existence. This lead to some unexpected behavior where a bindings test file (bindings/core/v8/IDLTypesTest.cpp) was attempting to use dictionary_sequence_or_dictionary.h, but either: i. Got an old version of the file that had been generated a long time ago, OR ii. Couldn't find the file to include (after I did a gn clean and rebuild) because the compiler wasn't generating it since there weren't any uses. Of the two outcomes, (i) worries me the most because it is feasible that someone wouldn't realize the file wasn't being generated anymore locally, and I'm not sure the bots would either (do they do a gn clean between builds?). It would be nice if the build could do a 'sanity check' to make sure that any file listed in bindings_core_generated_union_type_files in bindings/core/v8/BUILD.gn are actually used by some IDL file somewhere, and fail the build with a clear message if not.
,
Jan 3 2018
That's indeed an issue, but in this case IDLTypesTest.cpp is just a unit test file without a corresponding IDL.
,
Jan 3
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 7
+cc: peria@, do you have any thoughts?
,
Jan 11
,
Jan 17
(6 days ago)
We can't make (i) error, because a developer can create or update files under gen/ to test files independently from generated code. Build system cannot distinguish an unlisted file is updated by hand or generated in the past. And for (ii), compile bots do clean builds and they detect such unexpected behaviors. I think this "Sanity check" is to make sure that all listed files are existing or generated by action(), and if any listed file does not exist nor be generated, we can drop the filename from the GN file. It should be a feature request for GN.
,
Jan 17
(6 days ago)
Correction: GN doesn't work for build-time behavior. This may need support of build systems like ninja. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by smcgruer@chromium.org
, Jan 3 2018