Use new toolchain_args variable in GN

build/toolchain/gcc_toolchain.gni

Index: build/toolchain/gcc_toolchain.gni
diff --git a/build/toolchain/gcc_toolchain.gni b/build/toolchain/gcc_toolchain.gni
index 0045411afefb1b11b04b3cb7a9e9d5e2322819d8..1cc84b6d88317e41164f46cb6779d39ad0ffcd87 100644
--- a/build/toolchain/gcc_toolchain.gni
+++ b/build/toolchain/gcc_toolchain.gni
@@ -72,61 +72,12 @@ import("//build/toolchain/toolchain.gni")
 #      Location of the strip executable. When specified, strip will be run on
 #      all shared libraries and executables as they are built. The pre-stripped
 #      artifacts will be put in lib.unstripped/ and exe.unstripped/.
-#
-# Optional build argument controls.
-#
-#  - clear_sanitizers
-#      When set to true, is_asan, is_msan, etc.will all be set to false. Often
-#      secondary toolchains do not want to run with sanitizers.
-#  - is_clang
-#      Whether to use clang instead of gcc.
-#  - is_component_build
-#      Whether to forcibly enable or disable component builds for this
-#      toolchain; if not specified, the toolchain will inherit the
-#      default setting.
-#  - is_nacl_glibc
-#      Whether NaCl code is built using Glibc instead of Newlib.
-#  - cc_wrapper
-#      Override the global cc_wrapper setting. e.g. "ccache" or "icecc".
-#      useful to opt-out of cc_wrapper in a particular toolchain by setting
-#      cc_wrapper = "" in it.
-#  - use_debug_fission
-#      Override the global use_debug_fission setting, useful if the particular
-#      toolchain should not be generating split-dwarf code.
-#  - use_goma
-#      Override the global use_goma setting, useful to opt-out of goma in a
-#      particular toolchain by setting use_gome = false in it.
-#  - use_gold
-#      Override the global use_gold setting, useful if the particular
-#      toolchain has a custom link step that is not actually using Gold.
-#  - v8_toolchain_cpu
-#      If defined, set v8_current_cpu to this, else set v8_current_cpu
-#      to current_cpu.
 template("gcc_toolchain") {
   toolchain(target_name) {
     assert(defined(invoker.ar), "gcc_toolchain() must specify a \"ar\" value")
     assert(defined(invoker.cc), "gcc_toolchain() must specify a \"cc\" value")
     assert(defined(invoker.cxx), "gcc_toolchain() must specify a \"cxx\" value")
     assert(defined(invoker.ld), "gcc_toolchain() must specify a \"ld\" value")
-    assert(defined(invoker.toolchain_cpu),
-           "gcc_toolchain() must specify a \"toolchain_cpu\"")
-    assert(defined(invoker.toolchain_os),
-           "gcc_toolchain() must specify a \"toolchain_os\"")
-
-    if (defined(invoker.cc_wrapper)) {
-      cc_wrapper = invoker.cc_wrapper
-    }
-    if (defined(invoker.use_goma)) {
-      use_goma = invoker.use_goma
-    }
-    if (use_goma) {
-      assert(cc_wrapper == "", "Goma and cc_wrapper can't be used together.")
-      compiler_prefix = "$goma_dir/gomacc "
-    } else if (cc_wrapper != "") {
-      compiler_prefix = cc_wrapper + " "
-    } else {
-      compiler_prefix = ""
-    }
 
     # This define changes when the toolchain changes, forcing a rebuild.
     # Nothing should ever use this define.
@@ -136,6 +87,62 @@ template("gcc_toolchain") {
       rebuild_string = ""
     }
 
+    # GN's syntax can't handle more than one scope dereference at once, like
+    # "invoker.toolchain_args.foo", so make a temporary to hold the toolchain
+    # args so we can do "invoker_toolchain_args.foo".
+    if (defined(invoker.toolchain_args)) {
+      invoker_toolchain_args = invoker.toolchain_args
+    } else {
+      invoker_toolchain_args = {
+      }
+    }
+
+    # When invoking this toolchain not as the default one, these args will be
+    # passed to the build. They are ignored when this is the default toolchain.
+    toolchain_args = {
+      # Populate toolchain args from the invoker.
+      forward_variables_from(invoker_toolchain_args, "*")
+
+      # These values need to be passed through unchanged to all secondary
+      # toolchains. BUILDCONFIG.gn sets some defaults based on the values of
+      # the operating system and compiler, and we want to force the values to
+      # be consistent if re-running the computation in another context leads
+      # to different defaults.
+      host_toolchain = host_toolchain
+      target_os = target_os
+      target_cpu = target_cpu

[Dirk Pranke, 2016/08/08 23:28:39]

host_toolchain is derived from host_os, host_cpu, and is_clang. The first two
are constants. target_os and target_cpu are also supposed to be constants.
You're not actually forwarding/resetting is_clang, which makes sense because
that is usually a toolchain arg. So, I'm not sure what you're protecting
against?

[brettw, 2016/08/09 17:26:29]

This logic isn't changing from the old build, this patch is trying not to change
anything.

I agree target_os and target_cpu can probably be removed here. I'll make a note
to try that in a follow up. My perception is that this patch is very risky so
would like to have as small an effect as possible.

host_toolchain is necessary to pass here because is_clang will get modified by
the various toolchains. Those toolchains' values will be propagatedfrom the
invoker's toolchain args (the whole point of this change is that toolchains can
set arguments without having to list them all in this file).

[brettw, 2016/08/09 17:28:31]

If you're curious why this forwarding was added in the first place, you added it
here:
https://codereview.chromium.org/936103003/diff/80001/build/toolchain/gcc_tool...

[brettw, 2016/08/09 18:14:01]

Dependent patch to remove these:
https://codereview.chromium.org/2229063002/

[Dirk Pranke, 2016/08/09 19:18:18]

Right, forwarding host_toolchain still makes sense. Thx.

+
+      if (!defined(invoker_toolchain_args.v8_current_cpu)) {
+        v8_current_cpu = invoker_toolchain_args.current_cpu
+      }
+    }
+
+    # When the invoker has explicitly overridden use_goma or cc_wrapper in the
+    # toolchain args, use those values, otherwise default to the global one.
+    # This works because the only reasonable override that toolchains might
+    # supply for these values are to force-disable them.
+    if (defined(toolchain_args.use_goma)) {
+      toolchain_uses_goma = toolchain_args.use_goma
+    } else {
+      toolchain_uses_goma = use_goma
+    }
+    if (defined(toolchain_args.cc_wrapper)) {
+      toolchain_cc_wrapper = toolchain_args.cc_wrapper
+    } else {
+      toolchain_cc_wrapper = cc_wrapper
+    }
+
+    # Compute the compiler prefix.
+    if (toolchain_uses_goma) {
+      assert(toolchain_cc_wrapper == "",
+             "Goma and cc_wrapper can't be used together.")
+      compiler_prefix = "$goma_dir/gomacc "
+    } else if (toolchain_cc_wrapper != "") {
+      compiler_prefix = toolchain_cc_wrapper + " "
+    } else {
+      compiler_prefix = ""
+    }
+
     cc = compiler_prefix + invoker.cc
     cxx = compiler_prefix + invoker.cxx
     ar = invoker.ar
@@ -434,64 +441,18 @@ template("gcc_toolchain") {
       description = copy_description
     }
 
-    # When invoking this toolchain not as the default one, these args will be
-    # passed to the build. They are ignored when this is the default toolchain.
-    toolchain_args() {
-      current_cpu = invoker.toolchain_cpu
-      current_os = invoker.toolchain_os
-
-      # These values need to be passed through unchanged.
-      host_toolchain = host_toolchain
-      target_os = target_os
-      target_cpu = target_cpu
-
-      if (defined(invoker.is_clang)) {
-        is_clang = invoker.is_clang
-      }
-      if (defined(invoker.is_component_build)) {
-        is_component_build = invoker.is_component_build
-      }
-      if (defined(invoker.is_nacl_glibc)) {
-        is_nacl_glibc = invoker.is_nacl_glibc
-      }
-      if (defined(invoker.symbol_level)) {
-        symbol_level = invoker.symbol_level
-      }
-      if (defined(invoker.use_allocator)) {
-        use_allocator = invoker.use_allocator
-      }
-      if (defined(invoker.use_debug_fission)) {
-        use_debug_fission = invoker.use_debug_fission
-      }
-      if (defined(invoker.use_gold)) {
-        use_gold = invoker.use_gold
-      }
-      if (defined(invoker.use_sysroot)) {
-        use_sysroot = invoker.use_sysroot
-      }
-      if (defined(invoker.v8_toolchain_cpu)) {
-        v8_current_cpu = invoker.v8_toolchain_cpu
-      } else {
-        v8_current_cpu = current_cpu
-      }
-    }
-
     forward_variables_from(invoker, [ "deps" ])
   }
 }
 
-# This is a shorthand for gcc_toolchain instances based on the
-# Chromium-built version of Clang.  Only the toolchain_cpu and
-# toolchain_os variables need to be specified by the invoker, and
-# optionally toolprefix if it's a cross-compile case.  Note that for
-# a cross-compile case this toolchain requires a config to pass the
-# appropriate -target option, or else it will actually just be doing
-# a native compile.  The invoker can optionally override use_gold too.
+# This is a shorthand for gcc_toolchain instances based on the Chromium-built
+# version of Clang. Only the toolchain_cpu and toolchain_os variables need to
+# be specified by the invoker, and optionally toolprefix if it's a
+# cross-compile case. Note that for a cross-compile case this toolchain
+# requires a config to pass the appropriate -target option, or else it will
+# actually just be doing a native compile. The invoker can optionally override
+# use_gold too.
 template("clang_toolchain") {
-  assert(defined(invoker.toolchain_cpu),
-         "clang_toolchain() must specify a \"toolchain_cpu\"")
-  assert(defined(invoker.toolchain_os),
-         "clang_toolchain() must specify a \"toolchain_os\"")
   if (defined(invoker.toolprefix)) {
     toolprefix = invoker.toolprefix
   } else {
@@ -503,23 +464,41 @@ template("clang_toolchain") {
     cc = "$prefix/clang"
     cxx = "$prefix/clang++"
     ld = cxx
-    is_clang = true
 
     readelf = "${toolprefix}readelf"
     ar = "${toolprefix}ar"
     nm = "${toolprefix}nm"
 
-    forward_variables_from(invoker,
-                           [
-                             "strip",
-                             "toolchain_cpu",
-                             "toolchain_os",
-                             "use_gold",
-                             "v8_toolchain_cpu",
-                           ])
+    forward_variables_from(invoker, [ "strip" ])
+
+    toolchain_args = {
+      if (defined(invoker.toolchain_args)) {
+        forward_variables_from(invoker.toolchain_args, "*")
+      }
+      is_clang = true
+    }
 
+    # Backwards-compatible handling for toolchain definitions in the Native
+    # Client repo.
+    #
+    # TODO(brettw) bug 634446 remove this when
+    # //native_client/src/trusted/service_runtime/linux/BUILD.gn is updated to
+    # use the new-style toolchain_args.
+    if (defined(invoker.toolchain_cpu)) {
+      assert(!defined(toolchain_args.current_cpu))
+      toolchain_args.current_cpu = invoker.toolchain_cpu
+    }
+    if (defined(invoker.toolchain_os)) {
+      assert(!defined(toolchain_args.current_os))
+      toolchain_args.current_os = invoker.toolchain_os
+    }
     if (defined(invoker.use_debug_fission)) {
-      use_debug_fission = invoker.use_debug_fission
+      assert(!defined(toolchain_args.use_debug_fission))
+      toolchain_args.use_debug_fission = invoker.use_debug_fission
+    }
+    if (defined(invoker.use_gold)) {
+      assert(!defined(toolchain_args.use_gold))
+      toolchain_args.use_gold = invoker.use_gold
     }
   }
 }