Port some gtest.gyp settings to the BUILD.gn file

Port some gtest.gyp settings to the BUILD.gn file

BUG=None
TEST=gn, ninja -C out/Default url_unittests links fine.
R=brettw@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=243254

[tfarina, 2014-01-05 23:11:05]

I don't know if I did this right. Please, check.

Thanks!

Btw, do you know how we translate the following from gyp to gn:

'toolsets': ['host', 'target'],

?

[brettw, 2014-01-06 20:41:29]

Thanks! Most of this is unnecessary as I marked below.

Regarding toolset stuff, the exact translation into GN will not be what you
want. What you want will likely depend on the usage.

In GYP, this is a way to differentiate stuff that should be applied to the build
for the current system vs the build for the device (like on ChromeOS or
Android). Some things like V8 need to be compiled for the current system (which
is probably x86 Linux or Mac) to work as build steps, even though most of the
build might be targeting an ARM device.

In GN, the host build will just be is_linux, and the target build will be
is_chromeos (or is_android, or whatever). The fill will be executed multiple
times, once for each context. The same is true for the CPU architecture. So
generally you would check the thing it's actually trying to differentiate (be it
OS type or architecture or whatever) and write a condition using that.

https://codereview.chromium.org/124773002/diff/1/tools/gn/secondary/testing/g...
File tools/gn/secondary/testing/gtest/BUILD.gn (right):

https://codereview.chromium.org/124773002/diff/1/tools/gn/secondary/testing/g...
tools/gn/secondary/testing/gtest/BUILD.gn:12: include_dirs = [ "include" ]  #
Gtest headers need to be able to find themselves.
Can you fix this long line while you're here (probably put it on the previous
line is the most clear).

https://codereview.chromium.org/124773002/diff/1/tools/gn/secondary/testing/g...
tools/gn/secondary/testing/gtest/BUILD.gn:59: "GTEST_HAS_POSIX_RE=0",
You don't need this, same reason as below.

https://codereview.chromium.org/124773002/diff/1/tools/gn/secondary/testing/g...
tools/gn/secondary/testing/gtest/BUILD.gn:71: "GTEST_HAS_RTTI=0",
You don't need this since it's on the config, and direct/all_dependent_configs
also apply to the current target. So setting it in the gtest_config above should
cover this target.

https://codereview.chromium.org/124773002/diff/1/tools/gn/secondary/testing/g...
tools/gn/secondary/testing/gtest/BUILD.gn:73: direct_dependent_configs = [
":gtest_config" ]
You don't need this since you add it below.

https://codereview.chromium.org/124773002/diff/1/tools/gn/secondary/testing/g...
tools/gn/secondary/testing/gtest/BUILD.gn:86: direct_dependent_configs += [
":gtest_config" ]
You don't need this since 'all' handles everything. 'direct' is just one level,
so is a subset of 'all'.

[tfarina, 2014-01-06 22:48:15]

Excellent review Brett!

Please, could you take another look?

Thanks!

https://codereview.chromium.org/124773002/diff/1/tools/gn/secondary/testing/g...
File tools/gn/secondary/testing/gtest/BUILD.gn (right):

https://codereview.chromium.org/124773002/diff/1/tools/gn/secondary/testing/g...
tools/gn/secondary/testing/gtest/BUILD.gn:12: include_dirs = [ "include" ]  #
Gtest headers need to be able to find themselves.
On 2014/01/06 20:41:29, brettw wrote:
> Can you fix this long line while you're here (probably put it on the previous
> line is the most clear).

Done.

https://codereview.chromium.org/124773002/diff/1/tools/gn/secondary/testing/g...
tools/gn/secondary/testing/gtest/BUILD.gn:59: "GTEST_HAS_POSIX_RE=0",
On 2014/01/06 20:41:29, brettw wrote:
> You don't need this, same reason as below.

Done.

https://codereview.chromium.org/124773002/diff/1/tools/gn/secondary/testing/g...
tools/gn/secondary/testing/gtest/BUILD.gn:71: "GTEST_HAS_RTTI=0",
On 2014/01/06 20:41:29, brettw wrote:
> You don't need this since it's on the config, and direct/all_dependent_configs
> also apply to the current target. So setting it in the gtest_config above
should
> cover this target.

Done.

https://codereview.chromium.org/124773002/diff/1/tools/gn/secondary/testing/g...
tools/gn/secondary/testing/gtest/BUILD.gn:73: direct_dependent_configs = [
":gtest_config" ]
On 2014/01/06 20:41:29, brettw wrote:
> You don't need this since you add it below.

Done.

https://codereview.chromium.org/124773002/diff/1/tools/gn/secondary/testing/g...
tools/gn/secondary/testing/gtest/BUILD.gn:86: direct_dependent_configs += [
":gtest_config" ]
On 2014/01/06 20:41:29, brettw wrote:
> You don't need this since 'all' handles everything. 'direct' is just one
level,
> so is a subset of 'all'.

Done.

[brettw, 2014-01-06 23:11:49]

lgtm

[commit-bot: I haz the power, 2014-01-06 23:27:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/124773002/120001

[commit-bot: I haz the power, 2014-01-07 11:08:42]

Change committed as 243254