re-enable FindBadConstructs clang plugin for standalone v8 build |
||
Issue descriptionIn https://chromium-review.googlesource.com/c/chromium/src/+/970424 we turned on the FindBadConstruct clang plugin's no-realpath flag, which worked in chromium builds, but broke standalone V8 builds: > Ahh, I see the problem my change caused: the "/v8/" pattern in > banned_directories_ works when building the chromium tree because > paths to v8 sources relative to the out dir look like "../../v8/foo", > whereas in a standalone v8 build the equivalent path would be > "../../foo". > Things just happen to work in the standalone v8 build because the > checkout location is a directory named "v8" on the bots, so the > absolute path contains this and then all sources are considered > "banned". I don't think the style checker plugin should make > assumptions about the location of the root checkout dir, but I > need to think some more about how this should be implemented. So prevent this from blocking V8 rolls, the plugin was disabled in standalone v8 builds: https://chromium-review.googlesource.com/998153 To avoid more trouble, let's re-enable the plugin in standalone v8 builds in such a way that v8 code is not misclassified as "chromium" code, so we still run the tests that chromium builds run on "third party" code.
,
Apr 6 2018
The plugin looks like it might runs some checks on "third party" code, which is how the chromium build treats v8. If this is the case (I'm not 100% sure), then I think the standalone v8 build should attempt to run the same checks, to avoid delaying v8 rolls. Otherwise I agree- no need to enable the plugin in v8 builds if it doesn't actually check anything.
,
Apr 12 2018
The only third_party directory that's checked is blink (and that's because it's not really third party anymore--and even then, it's on by default there yet). The plugin itself is really only intended to be run on code that lives in the chromium repo.
,
Apr 12 2018
Should this be then closed as WontFix?
,
Apr 26 2018
I think so- wontfix'ing. |
||
►
Sign in to add a comment |
||
Comment 1 by thakis@chromium.org
, Apr 6 2018