Checkpoint work to get GN builds working on Android.

Checkpoint work to get GN builds working on Android.

This fixes enough issues so that we can compile and link some libraries, but
it is not yet a working build.

Work remaining:
- base needs JNI support ('base_jni_headers', etc.)
- we need to figure out what should link and/or be APKs to run the tests
- we need to adjust GN to only build in the host config, not the target config.

R=brettw@chromium.org, cjhopman@chromium.org
TBR=jam
BUG=360936
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=263440

[Dirk Pranke, 2014-04-08 00:39:22]

This patch is a work in progress (as noted in the description), but parts could
probably be carved off and landed.

I have a few style questions noted below ...

https://codereview.chromium.org/227673008/diff/20001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/227673008/diff/20001/BUILD.gn#newcode48
BUILD.gn:48: }
Currently GN fails to build w/ the android config.

It looks like it is trying to build it using the target (android) toolchain,
rather than the host toolchain. Presumably fixing that is easy, but I haven't
looked into it yet.

https://codereview.chromium.org/227673008/diff/20001/build/config/android/BUI...
File build/config/android/BUILD.gn (right):

https://codereview.chromium.org/227673008/diff/20001/build/config/android/BUI...
build/config/android/BUILD.gn:18: ]
Is this the right way to say "exec_script(<path-to-script>)" if script is not
next to the BUILD.gn file?

https://codereview.chromium.org/227673008/diff/20001/ipc/BUILD.gn
File ipc/BUILD.gn (right):

https://codereview.chromium.org/227673008/diff/20001/ipc/BUILD.gn#newcode83
ipc/BUILD.gn:83: if (!is_android) {
What's the best way to indicate "do not build this target on this OS"?

[brettw, 2014-04-09 05:37:07]

https://codereview.chromium.org/227673008/diff/20001/build/config/android/BUI...
File build/config/android/BUILD.gn (right):

https://codereview.chromium.org/227673008/diff/20001/build/config/android/BUI...
build/config/android/BUILD.gn:18: ]
On 2014/04/08 00:39:22, Dirk Pranke wrote:
> Is this the right way to say "exec_script(<path-to-script>)" if script is not
> next to the BUILD.gn file?

Yes, although we shouldn't be running this script in multiple places (the linux
build file also calls this). In this case we should put it in the .gni file so
it only gets forked once.

https://codereview.chromium.org/227673008/diff/20001/ipc/BUILD.gn
File ipc/BUILD.gn (right):

https://codereview.chromium.org/227673008/diff/20001/ipc/BUILD.gn#newcode83
ipc/BUILD.gn:83: if (!is_android) {
On 2014/04/08 00:39:22, Dirk Pranke wrote:
> What's the best way to indicate "do not build this target on this OS"? 

The way you did it

https://codereview.chromium.org/227673008/diff/20001/ipc/BUILD.gn#newcode154
ipc/BUILD.gn:154: "//base:base_i18n",
I renamed base_i18n "i18n" and test_support_base "test_support" in the latest
checkout.

[Dirk Pranke, 2014-04-09 18:49:20]

https://codereview.chromium.org/227673008/diff/20001/build/config/android/BUI...
File build/config/android/BUILD.gn (right):

https://codereview.chromium.org/227673008/diff/20001/build/config/android/BUI...
build/config/android/BUILD.gn:18: ]
On 2014/04/09 05:37:08, brettw wrote:
> On 2014/04/08 00:39:22, Dirk Pranke wrote:
> > Is this the right way to say "exec_script(<path-to-script>)" if script is
not
> > next to the BUILD.gn file?
> 
> Yes, although we shouldn't be running this script in multiple places (the
linux
> build file also calls this). In this case we should put it in the .gni file so
> it only gets forked once.

I don't think I understand this comment, possibly because I don't fully
understand GN's execution model yet to understand what things get executed more
than once vs. just once. 

However, since this file gets conditionally pulled in from BUILDCONFIG.gn, don't
we only get one or the other? Also, since linux and android will have different
sysroots, wouldn't we *have* to call this twice?

https://codereview.chromium.org/227673008/diff/20001/ipc/BUILD.gn
File ipc/BUILD.gn (right):

https://codereview.chromium.org/227673008/diff/20001/ipc/BUILD.gn#newcode83
ipc/BUILD.gn:83: if (!is_android) {
On 2014/04/09 05:37:08, brettw wrote:
> On 2014/04/08 00:39:22, Dirk Pranke wrote:
> > What's the best way to indicate "do not build this target on this OS"? 
> 
> The way you did it

It seems like the if (!is_android) approach handles the case where the build
instructions flat out won't work (which is the case now), but doesn't handle the
case where you simply don't want the target.

Would the latter be addressed by modifying the "root" group in src/BUILD.gn to
conditionally include or exclude ipc_tests from deps?

https://codereview.chromium.org/227673008/diff/20001/ipc/BUILD.gn#newcode154
ipc/BUILD.gn:154: "//base:base_i18n",
On 2014/04/09 05:37:08, brettw wrote:
> I renamed base_i18n "i18n" and test_support_base "test_support" in the latest
> checkout.

Ok, will fix.

[brettw, 2014-04-09 20:33:19]

https://codereview.chromium.org/227673008/diff/20001/build/config/android/BUI...
File build/config/android/BUILD.gn (right):

https://codereview.chromium.org/227673008/diff/20001/build/config/android/BUI...
build/config/android/BUILD.gn:18: ]
I guess you're right we'll only get one or the other. So this is fine.

https://codereview.chromium.org/227673008/diff/20001/ipc/BUILD.gn
File ipc/BUILD.gn (right):

https://codereview.chromium.org/227673008/diff/20001/ipc/BUILD.gn#newcode83
ipc/BUILD.gn:83: if (!is_android) {
GN always generates build rules for all targets in the default toolchain that it
encounters. We considered making it only generate those with a dependency chain
from the root, but people kind of expect the GYP behavior (write a new unit test
target and it's compilable immediately).

This is not the case for non-default toolchains or else you would get a nacl
build of chrome_browser or something. If this comes up a lot, we can also switch
the default toolchain to use non-greedy mode (only generate the required
targets).

[Dirk Pranke, 2014-04-10 01:49:52]

Updated; please take another look?

It's probably easier to land this as-is rather than carve it up into separate
CLs. It looks like we're starting to add more and more targets to the build, and
rebasing to keep track of things that don't yet work on Android will get more
and more annoying :).

[brettw, 2014-04-10 19:54:49]

lgtm

https://codereview.chromium.org/227673008/diff/40001/url/BUILD.gn
File url/BUILD.gn (right):

https://codereview.chromium.org/227673008/diff/40001/url/BUILD.gn#newcode82
url/BUILD.gn:82: "//base:base_i18n",
Revert this change. I assume this is a bad merge. The old value should be
correct unless I'm missing something.

[Dirk Pranke, 2014-04-10 20:09:22]

https://codereview.chromium.org/227673008/diff/40001/url/BUILD.gn
File url/BUILD.gn (right):

https://codereview.chromium.org/227673008/diff/40001/url/BUILD.gn#newcode82
url/BUILD.gn:82: "//base:base_i18n",
On 2014/04/10 19:54:49, brettw wrote:
> Revert this change. I assume this is a bad merge. The old value should be
> correct unless I'm missing something.

Hm, yeah, must be a bad merge, thanks. Same error shows up in ipc as well.

[Dirk Pranke, 2014-04-10 20:22:42]

TBR'ing jam@ to make the OWNERS presubmit happy ...

[Dirk Pranke, 2014-04-10 20:22:46]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2014-04-10 20:23:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dpranke@chromium.org/227673008/60001

[Dirk Pranke, 2014-04-10 20:25:14]

The CQ bit was unchecked by dpranke@chromium.org

[Dirk Pranke, 2014-04-11 22:03:31]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2014-04-11 22:04:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dpranke@chromium.org/227673008/60001

[commit-bot: I haz the power, 2014-04-11 22:05:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-11 22:05:01]

Failed to apply patch for BUILD.gn:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file BUILD.gn
  Hunk #1 FAILED at 48.
  1 out of 1 hunk FAILED -- saving rejects to file BUILD.gn.rej

Patch:       BUILD.gn
Index: BUILD.gn
diff --git a/BUILD.gn b/BUILD.gn
index
00197e679b3a4d7fa3098b65fc31746a56508697..f126cc959e017466d8f0240ac0e15a5b9c1c32d2
100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -48,4 +48,15 @@ group("root") {
     "//ui/gfx/geometry",
     "//url",
   ]
+
+  if (is_android) {
+    deps -= [
+      "//skia",
+      "//third_party/freetype2",
+      "//third_party/libusb",
+      "//tools/gn",
+      "//ui/events",
+      "//ui/gfx/geometry",
+    ]
+  }
 }

[Dirk Pranke, 2014-04-11 22:21:23]

The CQ bit was checked by dpranke@chromium.org

[commit-bot: I haz the power, 2014-04-11 22:21:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dpranke@chromium.org/227673008/80001

[commit-bot: I haz the power, 2014-04-12 01:19:17]

Change committed as 263440