Allow a custom toolchain to be injected into BUILDCONFIG.gn

build/config/BUILDCONFIG.gn

 # Copyright (c) 2013 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 # =============================================================================
 # PLATFORM SELECTION
 # =============================================================================
 #
 # There are two main things to set: "os" and "cpu". The "toolchain" is the name
 # of the GN thing that encodes combinations of these things.
@@ skipping 114 matching lines @@
   is_clang = current_os == "mac" || current_os == "ios" ||
              current_os == "linux" || current_os == "chromeos"
 
   if (current_os == "chromeos") {
     # Allows the target toolchain to be injected as arguments. This is needed
     # to support the CrOS build system which supports per-build-configuration
     # toolchains.
     cros_use_custom_toolchain = false
   }
 
+  # Allows the path to a custom target toolchain to be injected as a single
+  # argument, and set as the default toolchain.
+  # Note: On ChromeOS, do not set this argument. Instead, set
+  # cros_use_custom_toolchain, and set each of the args required by
+  # //build/toolchain/cros:target.
+  custom_toolchain = ""

[brettw, 2015/09/28 19:58:28]

We need to keep this file cleaner than this. Please restructure the cros code so
that everybody uses the same custom_toolchain variable.

[slan, 2015/09/28 20:50:40]

There are two paths around this redundancy:

1) All ChromeOS builds use |custom_toolchain|, and |cros_use_custom_toolchain|
goes away.

The reason that I did not go straight to this solution is that it will break any
ChromeOS build relying on |cros_use_custom_toolchain|. Although this should have
minimal impact in Chrome, it will likely have a huge impact on the CrOS internal
build infrastructure. 

2) Allow chromecast builds to utilize //build/toolchain/cros:target as the
default toolchain. 

This is the path of least resistance, but it a) forces us to introduce
is_chromecast here, and b) is non-intuitive. I would view this as a temporary
solution, but I would prefer it to blocking this patch on  the issues presented
by #1

Which idea did you have in mind?

I will reach out to a couple of CrOS folks to figure out what needs to happen if
we go with #1.

[slan, 2015/09/28 21:21:42]

Removed.

+
   # DON'T ADD MORE FLAGS HERE. Read the comment above.
 }
 
+if (current_os == "chromeos") {
+  assert(
+      custom_toolchain == "",
+      "|custom_toolchain| should not be set in args on ChromeOS. Set cros_use_custom_toolchain instead.")
+
+  if (cros_use_custom_toolchain) {
+    custom_toolchain = "//build/toolchain/cros:target"
+  }
+}
+
 # =============================================================================
 # OS DEFINITIONS
 # =============================================================================
 #
 # We set these various is_FOO booleans for convenience in writing OS-based
 # conditions.
 #
 # - is_android, is_chromeos, is_ios, and is_win should be obvious.
 # - is_mac is set only for desktop Mac. It is not set on iOS.
 # - is_posix is true for mac and any Unix-like system (basically everything
@@ skipping 375 matching lines @@
     set_default_toolchain("//build/toolchain/android:$current_cpu")
   }
 } else if (is_linux) {
   if (is_clang) {
     host_toolchain = "//build/toolchain/linux:clang_$host_cpu"
     set_default_toolchain("//build/toolchain/linux:clang_$current_cpu")
   } else {
     host_toolchain = "//build/toolchain/linux:$host_cpu"
     set_default_toolchain("//build/toolchain/linux:$current_cpu")
   }
-  if (is_chromeos && cros_use_custom_toolchain) {
-    set_default_toolchain("//build/toolchain/cros:target")
-  }
 } else if (is_mac) {
   host_toolchain = "//build/toolchain/mac:clang_x64"
   set_default_toolchain(host_toolchain)
 } else if (is_ios) {
   host_toolchain = "//build/toolchain/mac:clang_x64"
   set_default_toolchain("//build/toolchain/mac:ios_clang_arm")
 } else if (is_nacl) {
   # TODO(GYP): This will need to change when we get NaCl working
   # on multiple platforms, but this whole block of code (how we define
   # host_toolchain) needs to be reworked regardless to key off of host_os
   # and host_cpu rather than the is_* variables.
   host_toolchain = "//build/toolchain/linux:clang_x64"
 }
 
+# Set the custom default toolchain.
+if (custom_toolchain != "") {
+  set_default_toolchain(custom_toolchain)

[brettw, 2015/09/28 19:58:28]

let's restructure the code above to assign to a variable _default_toolchain and
then call set_default_toolchain once at the bottom, either with that value, or
the custom one. It seems like poor form to call a function and then call it
again with a different value.

[slan, 2015/09/28 20:50:40]

Done.

+}
+
 # ==============================================================================
 # COMPONENT SETUP
 # ==============================================================================
 
 if (is_component_build) {
   _component_mode = "shared_library"
 } else {
   _component_mode = "source_set"
 }
 template("component") {
   target(_component_mode, target_name) {
     forward_variables_from(invoker, "*")
 
     # All shared libraries must have the sanitizer deps to properly link in
     # asan mode (this target will be empty in other cases).
     if (!defined(deps)) {
       deps = []
     }
     deps += [ "//build/config/sanitizers:deps" ]
   }
 }