https://chromium.googlesource.com/chromium/src/+/95ba4446355ffd95eca68d3c3ba7fc3c2e364b6d%5E%21/#F1 might cause the posix.stamp warning, but undoing that doesn't help with most other warnings.

With clang=true, out/gn/build.ninja has:

subninja toolchain.ninja
subninja clang_x64/toolchain.ninja
subninja clang_x86/toolchain.ninja
subninja clang_x86/toolchain.ninja

With gcc it instead has:

subninja toolchain.ninja
subninja clang_x64/toolchain.ninja
subninja x86/toolchain.ninja
subninja clang_x86/toolchain.ninja

Ideally I suppose it should just have a single x64 dep? It's not like we need to generate 32-bit v8 snapshots on x64, right?

+dpranke in case this looks familiar to him (looks possibly related to the v8 snapshot toolchain magic, but the output in comment 2 looks like a borderline gn bug even)

Looking more at this, it looks like the the clang_x86 toolchains really are separate toolchains (one targeting linux, one android), they just happen to share a name.  The following diff makes things go, but I'm not sure it's the right fix:


$ git diff
diff --git a/build/config/BUILDCONFIG.gn b/build/config/BUILDCONFIG.gn
index 6685405..942605e 100644
--- a/build/config/BUILDCONFIG.gn
+++ b/build/config/BUILDCONFIG.gn
@@ -216,9 +216,9 @@ if (target_os == "android") {
   assert(host_os == "linux" || host_os == "mac",
          "Android builds are only supported on Linux and Mac hosts.")
   if (is_clang) {
-    _default_toolchain = "//build/toolchain/android:clang_$target_cpu"
+    _default_toolchain = "//build/toolchain/android:android_clang_$target_cpu"
   } else {
-    _default_toolchain = "//build/toolchain/android:$target_cpu"
+    _default_toolchain = "//build/toolchain/android:android_$target_cpu"
   }
 } else if (target_os == "chromeos" || target_os == "linux") {
   # See comments in build/toolchain/cros/BUILD.gn about board compiles.
diff --git a/build/config/android/config.gni b/build/config/android/config.gni
index 5dc4ff3..df64d40 100644
--- a/build/config/android/config.gni
+++ b/build/config/android/config.gni
@@ -334,10 +334,10 @@ if (is_android) {
   if (defined(android_secondary_abi_cpu)) {
     if (is_clang) {
       android_secondary_abi_toolchain =
-          "//build/toolchain/android:clang_${android_secondary_abi_cpu}"
+          "//build/toolchain/android:android_clang_${android_secondary_abi_cpu}"
     } else {
       android_secondary_abi_toolchain =
-          "//build/toolchain/android:${android_secondary_abi_cpu}"
+          "//build/toolchain/android:android_${android_secondary_abi_cpu}"
     }
   }
 }
diff --git a/build/toolchain/android/BUILD.gn b/build/toolchain/android/BUILD.gn
index 917c469..ad7b762 100644
--- a/build/toolchain/android/BUILD.gn
+++ b/build/toolchain/android/BUILD.gn
@@ -70,12 +70,12 @@ template("android_gcc_toolchain") {
 }
 
 template("android_gcc_toolchains_helper") {
-  android_gcc_toolchain(target_name) {
+  android_gcc_toolchain("android_$target_name") {
     forward_variables_from(invoker, "*")
     toolchain_args.is_clang = false
   }
 
-  android_gcc_toolchain("clang_$target_name") {
+  android_gcc_toolchain("android_clang_$target_name") {
     forward_variables_from(invoker, "*")
     toolchain_args.is_clang = true
   }



It seems like a good change for the immediate problem, but I'm not sure if the x64 android build should depend on 32-bit x86 toolchains at all.

Aha, we're explicitly building 32-bit binaries on android in 64-bit builds for  bug 605315 . To test that theory, I locally disabled this and that makes the warning go away (just to prove my theory, I'm not proposing to land that of course). It does.

$ git diff
diff --git a/android_webview/BUILD.gn b/android_webview/BUILD.gn
index 20c2a1c..81a90c5 100644
--- a/android_webview/BUILD.gn
+++ b/android_webview/BUILD.gn
@@ -129,7 +129,7 @@ android_assets("monochrome_webview_assets") {
     "//third_party/icu:icu_assets",
     "//v8:v8_external_startup_data_assets",
   ]
-  if (android_64bit_target_cpu && build_apk_secondary_abi) {
+  if (false && android_64bit_target_cpu && build_apk_secondary_abi) {
     deps += [ ":v8_snapshot_secondary_abi_assets" ]
   }
 }
@@ -271,7 +271,7 @@ shared_library("libwebviewchromium") {
   configs -= [ "//build/config/android:hide_native_jni_exports" ]
 }
 
-if (android_64bit_target_cpu) {
+if (false && android_64bit_target_cpu) {
   android_assets("v8_snapshot_secondary_abi_assets") {
     _secondary_abi_out_dir =
         get_label_info("//v8($android_secondary_abi_toolchain)", "root_out_dir")
diff --git a/android_webview/system_webview_apk_tmpl.gni b/android_webview/system_webview_apk_tmpl.gni
index 8b37716..f7f4890 100644
--- a/android_webview/system_webview_apk_tmpl.gni
+++ b/android_webview/system_webview_apk_tmpl.gni
@@ -24,7 +24,7 @@ template("system_webview_apk_tmpl") {
     _native_lib_file =
         rebase_path("$root_gen_dir/CHROME_VERSION.json", root_out_dir)
     native_lib_version_arg = "@FileArg($_native_lib_file:full-quoted)"
-    if (build_apk_secondary_abi && android_64bit_target_cpu) {
+    if (false && build_apk_secondary_abi && android_64bit_target_cpu) {
       secondary_abi_shared_libraries = [ "//android_webview:libwebviewchromium($android_secondary_abi_toolchain)" ]
     }
 
diff --git a/build/config/android/config.gni b/build/config/android/config.gni
index 5dc4ff3..aa96470 100644
--- a/build/config/android/config.gni
+++ b/build/config/android/config.gni
@@ -331,15 +331,15 @@ if (is_android) {
     android_app_secondary_abi = "mips"
   }
 
-  if (defined(android_secondary_abi_cpu)) {
-    if (is_clang) {
-      android_secondary_abi_toolchain =
-          "//build/toolchain/android:clang_${android_secondary_abi_cpu}"
-    } else {
-      android_secondary_abi_toolchain =
-          "//build/toolchain/android:${android_secondary_abi_cpu}"
-    }
-  }
+  #if (defined(android_secondary_abi_cpu)) {
+  #  if (is_clang) {
+  #    android_secondary_abi_toolchain =
+  #        "//build/toolchain/android:android_clang_${android_secondary_abi_cpu}"
+  #  } else {
+  #    android_secondary_abi_toolchain =
+  #        "//build/toolchain/android:android_${android_secondary_abi_cpu}"
+  #  }
+  #}
 }
 
 declare_args() {
diff --git a/chrome/android/BUILD.gn b/chrome/android/BUILD.gn
index 2e5868c..c7e2ed1a 100644
--- a/chrome/android/BUILD.gn
+++ b/chrome/android/BUILD.gn
@@ -544,7 +544,7 @@ if (current_toolchain == default_toolchain) {
     generate_resource_whitelist("monochrome_resource_whitelist") {
       # Always use the 32-bit library's whitelist since the 64-bit one is
       # webview-only.
-      if (!android_64bit_target_cpu) {
+      if (true || !android_64bit_target_cpu) {
         _fat_lib_toolchain = current_toolchain
       } else {
         _fat_lib_toolchain = android_secondary_abi_toolchain
@@ -598,7 +598,7 @@ if (current_toolchain == default_toolchain) {
 # with secondary 32-bit toolchain in 64-bit platform because we
 # need 64-bit //android_webview/monochrome and 32-bit this target
 # for 64-bit APK.
-if (!android_64bit_target_cpu ||
+if (true || !android_64bit_target_cpu ||
     current_toolchain == android_secondary_abi_toolchain) {
   shared_library("monochrome") {
     sources = [

(i.e. this is a regression from the work in  bug 605315 )

Yes, I think your fix is (at least mostly) correct. In this case, we need different toolchains for each combination of <os, compiler, cpu_arch>, and two of them have the same name, because the os isn't embedded in the toolchain name, so sadness ensues.

The general problem is actually a bit worse: we potentially need different toolchains for each combination of toolchain_args values, and I doubt that we're really doing that. Someone (possibly me, but I'd be happy for someone else to step in) to audit all of the toolchains to see if there might be other collisions.

Cool, thanks. Sent out as https://codereview.chromium.org/2463143002/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/042f129b96ea6e989df1efe0e4a30dcac09f05a7

commit 042f129b96ea6e989df1efe0e4a30dcac09f05a7
Author: thakis <thakis@chromium.org>
Date: Mon Oct 31 20:22:40 2016

Android: Prefix target toolchain names with "android_".

In x64 builds with clang, we also build a 32-bit binary to ship webview
in both 32-bit and 64-bit.  The 32-bit part is built twice, once for
the linux host to be able to run v8's mksnapshot, and once for the
android target for the actual binary.

Before this change, both the host toolchain and the target toolchain
were called "clang_x86", and they clobbered each other.  (In gcc builds,
the target toolchain was called just "x86" while the host still used
clang, so it happened to work there, mostly by accident.)

BUG= 660857 , 605315 

Review-Url: https://codereview.chromium.org/2463143002
Cr-Commit-Position: refs/heads/master@{#428783}

[modify] https://crrev.com/042f129b96ea6e989df1efe0e4a30dcac09f05a7/build/config/BUILDCONFIG.gn
[modify] https://crrev.com/042f129b96ea6e989df1efe0e4a30dcac09f05a7/build/config/android/config.gni
[modify] https://crrev.com/042f129b96ea6e989df1efe0e4a30dcac09f05a7/build/toolchain/android/BUILD.gn

Filed  bug 661054  to make gn diag on this.