gn check suddenly complaining about aura includes on mac |
||||
Issue description
In some existing files in the codebase, it seems as though gn check should be complaining for MacOS, for example:
//chrome/browser/extensions/api/automation_internal/automation_internal_api.cc
has
#if defined(USE_AURA)
#include "chrome/browser/ui/aura/accessibility/automation_manager_aura.h"
#include "ui/aura/env.h"
#endif
The env include ought to need a nogncheck, because the //chrome/browser/extensions:extensions target should not depend on the //ui/aura:aura target. However, gn check is silent, which means there's either a bug in the dependency graph or a bug in gn check. When I make a seemingly unrelated change[1], this error and many others are triggered[2]:
ERROR at //chrome/browser/extensions/api/automation_internal/automation_internal_api.cc:47:11: Include not allowed.
#include "ui/aura/env.h"
^------------
It is not in any dependency of
//chrome/browser/extensions:extensions
The include file is in the target(s):
//ui/aura:aura
which should somehow be reachable.
[1] https://chromium-review.googlesource.com/c/chromium/src/+/1359592/18
[2] https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8927467851073862336/+/steps/generate_build_files__with_patch_/0/stdout
,
Dec 11
IIRC, GN ignores files that are missing because, in general, generated files won't have been generated yet when you run `gn gen --check`. I can't remember if there are other reasons. brettw@, am I remembering correctly, and are there other reasons we might not complain about a missing header? I still don't understand what's going on with the check failure, though, so I'm looking into it more.
,
Dec 11
,
Dec 11
There are also platform files. Lots of code does this: #if ANDROID #include "foo_android.h" #endif When 'foo_android.h" isn't in the build, we can ignore these. Otherwise each of these with need "nogncheck" annotations since it doesn't understand ifdefs.
,
Dec 11
@brettw - so, it's not just that the file doesn't exist on disk, it also applies to files that aren't pulled into a component in the build?
,
Dec 11
Aha, I see what's going on. //ui/views:views_tests_catalog references //services/ws/test_ws:manifest. //services/ws/test_ws/BUILD.gn contains other targets, including //services/ws/test_ws:lib, which has a dependency on //ui/aura:aura. So, by GN transitively reading stuff in, it then becomes aware of //ui/aura/env.h, at which point all of these "illegal" includes crop up. So, this is working as intended, given the limitations of gn check. The workaround works by not defining :views_tests_catalog, so none of that other stuff gets pulled in. |
||||
►
Sign in to add a comment |
||||
Comment 1 by est...@chromium.org
, Dec 11Seems the problem is the catalog template that was moved into the //ui/views/BUILD.gn file. Not entirely sure why, but I did noticed that gn check will ignore header files it doesn't know about, e.g. #include "source/that/doesnt/exist.h" won't raise an alarm. So I think that somehow this catalog caused gn to know about ui/aura/env.h for some definition of "know about". I can solve the gn issues by inserting a check for enable_mus. catalog("views_tests_catalog") { testonly = true if (enable_mus) { embedded_services = [ ":unittests_manifest", ":interactive_ui_tests_manifest", ] standalone_services = [ "//services/ws/test_ws:manifest", "//services/ws/ime/test_ime_driver:manifest", ] } }