gn: Allow users to set 'check_includes' in action_foreach().

gn: Allow users to set 'check_includes' in action_foreach().

Otherwise if someone tries to set it now it will get something like:

$ gn gen --args='os="android" cpu_arch="arm"' out_gn_android/Debug
ERROR at //build/config/android/rules.gni:49:22: Assignment had no
effect.
    check_includes = false
                         ^----
You set the variable "check_includes" here and it was unused before it went out of scope.
See //content/test/BUILD.gn:237:5: whence it was called.
generate_jni("jni") {
   ^--------------------

BUG=367595, 376000
TEST=see above
R=brettw@chromium.org

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

[tfarina, 2015-01-30 21:04:52]

Brett, could you review this to me? See CL
https://codereview.chromium.org/871813004/.

Chris, Dirk, if you want to review it also, feel free (I will really appreciate
any comments).

[cjhopman, 2015-01-30 23:04:45]

cjhopman@chromium.org changed reviewers:
+ cjhopman@chromium.org

[cjhopman, 2015-01-30 23:04:46]

https://codereview.chromium.org/885283006/diff/1/tools/gn/action_target_gener...
File tools/gn/action_target_generator.h (right):

https://codereview.chromium.org/885283006/diff/1/tools/gn/action_target_gener...
tools/gn/action_target_generator.h:28: bool FillCheckIncludes();
If we are checking includes for every target type, we should probably just have
FillCheckIncludes in TargetGenerator and call it in TargetGenerator::Run().

Another option is to only check includes for binary targets.

I don't know which of those two approaches would be better.

[brettw, 2015-01-30 23:08:55]

I lean slightly toward doing this for action_foreach. It will only check source
files of types it knows about anyway.

https://codereview.chromium.org/885283006/diff/1/tools/gn/action_target_gener...
File tools/gn/action_target_generator.cc (right):

https://codereview.chromium.org/885283006/diff/1/tools/gn/action_target_gener...
tools/gn/action_target_generator.cc:111: bool
ActionTargetGenerator::FillCheckIncludes() {
To avoid duplication with the binary target generator, can you move this to
TargetGenerator. Then the different specializations can call it when
appropriate.

[tfarina, 2015-01-30 23:18:29]

https://codereview.chromium.org/885283006/diff/1/tools/gn/action_target_gener...
File tools/gn/action_target_generator.cc (right):

https://codereview.chromium.org/885283006/diff/1/tools/gn/action_target_gener...
tools/gn/action_target_generator.cc:111: bool
ActionTargetGenerator::FillCheckIncludes() {
On 2015/01/30 23:08:55, brettw wrote:
> To avoid duplication with the binary target generator, can you move this to
> TargetGenerator. Then the different specializations can call it when
> appropriate.

Done.

[tfarina, 2015-02-02 15:29:17]

Brett, ping!

[brettw, 2015-02-02 23:36:52]

lgtm

[tfarina, 2015-02-05 13:06:12]

The CQ bit was checked by tfarina@chromium.org

[commit-bot: I haz the power, 2015-02-05 13:06:21]

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

[commit-bot: I haz the power, 2015-02-05 13:54:29]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-05 13:56:13]

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