[PATCH] D56577: gn build: Add a stage2 toolchain for Android.

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: llvm/utils/gn/build/toolchain/BUILD.gn:158
+      "//:llvm-ar($host_toolchain)",
+    ]
+
----------------
thakis wrote:
> Whoa, do deps on toolchains work now? Is that new?
It's been around for a while, but it's discouraged. `gn help toolchain` says for toolchain deps:

    This concept is somewhat inefficient to express in Ninja (it requires a lot
    of duplicate of rules) so should only be used when absolutely necessary.

I don't think there's a cleaner way for multi-stage builds though since you need to somehow express that dependency.


================
Comment at: llvm/utils/gn/build/toolchain/BUILD.gn:169
+
+    target_flags =
+        "--target=aarch64-linux-android21 --sysroot=$android_ndk_path/sysroot"
----------------
thakis wrote:
> That's alright for now, but I think eventually we want to remove the target_foo stuff from the toolchain definition again and instead use configs for this (like https://cs.chromium.org/chromium/src/build/config/compiler/BUILD.gn?type=cs&q=build/config/compiler&sq=package:chromium&g=0&l=217)
That would be my preference. Rather than combining different ways of specifying flags, we should just introduce default configs for each platform and then add those to defaults for each target type (e.g. https://fuchsia.googlesource.com/build/+/master/config/BUILDCONFIG.gn#146)


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56577/new/

https://reviews.llvm.org/D56577