GN header checker enhancements.

GN header checker enhancements.

This makes the check_includes flag work the way one would expect. Previously we
would only skip include checking if all targets that include a file opt out.
This was coming up in the Android Java enum generator where a header file was
being processed by a script (so includes don't matter and there are no deps)
but that header was also listed as a source in a different target with header
checking enabled (which is always the case for this example).

Improves error messaging for cross-compiling. Previously if you were
cross-compiling and forgot a dependency on e.g. base, you'd get a message "you
must depend on "//base:base" or "//base:base" because it found the files in two
different toolchain builds of base. This patch prunes such duplicates when
possible, and turns on toolchain labels if things aren't all in the same
toolchain.

BUG=

Committed: https://crrev.com/f16d8d8d34fb9403852423df5adcf741b9bb8beb
Cr-Commit-Position: refs/heads/master@{#317446}

[brettw, 2015-02-20 22:24:59]

brettw@chromium.org changed reviewers:
+ scottmg@chromium.org

[scottmg, 2015-02-20 22:41:40]

lgtm

https://codereview.chromium.org/946043002/diff/1/tools/gn/header_checker.cc
File tools/gn/header_checker.cc (right):

https://codereview.chromium.org/946043002/diff/1/tools/gn/header_checker.cc#n...
tools/gn/header_checker.cc:121: return a->label().dir() == b->label().dir() &&
maybe GetWithNoToolchain() and == would be clearer

[brettw, 2015-02-20 22:50:03]

https://codereview.chromium.org/946043002/diff/1/tools/gn/header_checker.cc
File tools/gn/header_checker.cc (right):

https://codereview.chromium.org/946043002/diff/1/tools/gn/header_checker.cc#n...
tools/gn/header_checker.cc:121: return a->label().dir() == b->label().dir() &&
On 2015/02/20 22:41:40, scottmg wrote:
> maybe GetWithNoToolchain() and == would be clearer

Slightly but that's constructing extra strings for each invocation which I was
preferring to avoid. If this comes up more often I should make this a function
on the Label class, but this is the first and likely only case we'll need to do
this.

[brettw, 2015-02-20 22:50:07]

The CQ bit was checked by brettw@chromium.org

[commit-bot: I haz the power, 2015-02-20 22:50:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/946043002/1

[tfarina, 2015-02-21 00:25:36]

Looking forward to see this rolling. With this in I will probably be able to
enable 'gn check' for //ui/*.

[brettw, 2015-02-21 00:27:44]

BTW enabling more "gn checks" might be increasingly difficult when
cross-compiling (e.g. Android). If you run into problems that only happen
on android and cross toolchains, you'll have to wait for more magic in the
gn check implementation.

On Fri, Feb 20, 2015 at 4:25 PM, <tfarina@chromium.org> wrote:

> Looking forward to see this rolling. With this in I will probably be able
> to
> enable 'gn check' for //ui/*.
>
> https://codereview.chromium.org/946043002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2015-02-21 00:41:21]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-02-21 00:42:44]

Patchset 1 (id:??) landed as
https://crrev.com/f16d8d8d34fb9403852423df5adcf741b9bb8beb
Cr-Commit-Position: refs/heads/master@{#317446}