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

[Peter Collingbourne via Phabricator via llvm-commits]

pcc marked 3 inline comments as done.
pcc added inline comments.


================
Comment at: llvm/utils/gn/build/toolchain/BUILD.gn:49
+        command =
+            "rm -f {{output}} && $ar rcsDT {{arflags}} {{output}} {{inputs}}"
       }
----------------
thakis wrote:
> (this might be a git merge issue -- be sure to run `git ls-files '*.gn' '*.gni' | xargs -n 1 gn format` though :-)
Yes, I think I might have forgotten to format D56576. This change restores the correct formatting though.


================
Comment at: llvm/utils/gn/build/toolchain/BUILD.gn:169
+
+    target_flags =
+        "--target=aarch64-linux-android21 --sysroot=$android_ndk_path/sysroot"
----------------
phosek wrote:
> 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)
We'll see how that goes. One issue is that we'll need some way of writing the target flags to a file to run the tests:
http://llvm-cs.pcc.me.uk/projects/compiler-rt/test/hwasan/lit.site.cfg.in#5
and I'm not sure how easy that will be with a config.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56577