New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 657166 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Last visit 26 days ago
Closed: Oct 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

gn check does not always report missing dependencies

Project Member Reported by jcivelli@chromium.org, Oct 18 2016

Issue description

My patch https://codereview.chromium.org/2431753002/#ps60001 triggers an error when running "gn check":

ERROR at //content/renderer/render_thread_impl.cc:208:11: Include not allowed.
#include "services/ui/public/cpp/gpu_service.h"
          ^-----------------------------------
It is not in any dependency of
  //content/renderer:renderer
The include file is in the target(s):
  //services/ui/public/cpp:cpp
which should somehow be reachable.

when it does not modify that file.
It seems the error was already there but somehow not reported, and an unrelated change in visibility somehow triggered it (actual file change https://codereview.chromium.org/2431753002/diff/60001/services/ui/public/cpp/BUILD.gn)
 

Comment 1 by brettw@chromium.org, Oct 18 2016

I'm guessing //services/ui was never getting compiled on Android before this. GN will skip headers if it doesn't know about them.
//services/ui was indeed not compiled on Android, but it was compiled on other platforms, why wouldn't it throw an error there?

Comment 3 by brettw@chromium.org, Oct 18 2016

Status: WontFix (was: Untriaged)
The dependency is conditionally added in a use_aura condition. Probably all builds either used aura or never compiled this service.
Status: Assigned (was: WontFix)
Shouldn't it be a bug if a file is referring to an in-tree header and GN doesn't know about the header?

render_thread_impl.cc is compiled on the Mac as well but doesn't use Aura; it seems like a problem that it is including gpu_service.h but the dependency isn't specified ...

Comment 5 by brettw@chromium.org, Oct 20 2016

It only checks includes known in the current toolchain. This allows most platform-specific includes to not require nocheck annotations. Otherwise we would need an order of magnitude more of these annotations.
Status: WontFix (was: Assigned)
Ah, yeah, I guess that makes sense. 

Sign in to add a comment