[Mac/iOS/GN] Place a version in the command of toolchain tools implemented using scripts.

build/toolchain/mac/BUILD.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.
 
 # TODO(brettw) Use "gcc_toolchain.gni" like the Linux toolchains. This requires
 # some enhancements since the commands on Mac are slightly different than on
 # Linux.
 
 import("../goma.gni")
 import("//build/config/ios/ios_sdk.gni")
@@ skipping 41 matching lines @@
 
     # concurrent_links is picked up from the declare_arg().
 
     # We can't do string interpolation ($ in strings) on things with dots in
     # them. To allow us to use $cc below, for example, we create copies of
     # these values in our scope.
     cc = invoker.cc
     cxx = invoker.cxx
     ld = invoker.ld
 
+    # When implementing tools using Python scripts, a TOOL_VERSION=N env

[brettw, 2016/06/17 17:29:43]

I just filed https://bugs.chromium.org/p/chromium/issues/detail?id=621119 for
this, can you reference that bug in a TODO here?

[Robert Sesek, 2016/06/17 18:07:53]

Done.

+    # variable is placed in front of the command. The N should be incremented
+    # whenever the script is changed, so that the build system rebuilds all
+    # edges that utilize the script. Ideally this should be changed to use
+    # proper input-dirty checking, but that could be expensive. Instead, use a
+    # script to get the tool script's modification time to set as the version.
+    # This won't cause a re-generation of GN files when the tool script changes
+    # but it will cause edges to be marked as dirty if the ninja files are
+    # regenerated. See https://crbug.com/619083 for details.
+    get_tool_mtime = "get_tool_mtime.py"
+
     linker_driver =
         rebase_path("//build/toolchain/mac/linker_driver.py", root_build_dir)
+    linker_driver =
+        "TOOL_VERSION=" +
+        exec_script(get_tool_mtime, [ linker_driver ], "trim string") +
+        " $linker_driver"
 
     # Make these apply to all tools below.
     lib_switch = "-l"
     lib_dir_switch = "-L"
 
     # Object files go in this directory. Use label_name instead of
     # target_output_name since labels will generally have no spaces and will be
     # unique in the directory.
     object_subdir = "{{target_out_dir}}/{{label_name}}"
 
@@ skipping 55 matching lines @@
       depsformat = "gcc"
       description = "OBJCXX {{output}}"
       outputs = [
         "$object_subdir/{{source_name_part}}.o",
       ]
     }
 
     tool("alink") {
       script =
           rebase_path("//build/toolchain/mac/filter_libtool.py", root_build_dir)
-      command = "rm -f {{output}} && python $script libtool -static {{arflags}} -o {{output}} {{inputs}}"
+      tool_version = exec_script(get_tool_mtime, [ script ], "trim string")

[sdefresne, 2016/06/16 18:15:25]

I'm not really sure about using exec_script because it mean that if I change the
script, but not any BUILD.gn file, ninja will not re-run "gn gen" and thus not
regenerate the build files, the command-line will not be updated and thus the
TOOL_VERSION value unchanged ...

So, sadly I need that if we go that route we need to manually update the
TOOL_VERSION (i.e. even more important to update gn to turn those dependencies
in real dependencies).

[Robert Sesek, 2016/06/16 18:26:16]

Right. I wrote that in both the comment above and the CL description ;)
I think this is better than manually modifying the version. I don't trust that I
or others will remember to bump this; the only proper fix is to have tool
scripts properly specified as an input. I think this approach is better than
manual version because this will at least catch issues on the bots without
requiring a manual version bump. And locally there's not much difference between
manually modifying the version number or manually re-running `gn gen`.

+      command = "rm -f {{output}} && TOOL_VERSION=$tool_version python $script libtool -static {{arflags}} -o {{output}} {{inputs}}"
       description = "LIBTOOL-STATIC {{output}}"
       outputs = [
         "{{output_dir}}/{{target_output_name}}{{output_extension}}",
       ]
       default_output_dir = "{{target_out_dir}}"
       default_output_extension = ".a"
       output_prefix = "lib"
     }
 
     tool("solink") {
@@ skipping 121 matching lines @@
       description = stamp_description
     }
     tool("copy") {
       command = copy_command
       description = copy_description
     }
 
     tool("copy_bundle_data") {
       _tool = rebase_path("//build/toolchain/mac/copy_bundle_data.py",
                           root_build_dir)
-      command = "python $_tool {{source}} {{output}}"
+      _tool_version = exec_script(get_tool_mtime, [ _tool ], "trim string")
+      command =
+          "TOOL_VERSION=$_tool_version python $_tool {{source}} {{output}}"
       description = "COPY_BUNDLE_DATA {{source}} {{output}}"
       pool = ":bundle_pool($default_toolchain)"
     }
     tool("compile_xcassets") {
       _tool = rebase_path("//build/toolchain/mac/compile_xcassets.py",
                           root_build_dir)
+      _tool_version = exec_script(get_tool_mtime, [ _tool ], "trim string")

[brettw, 2016/06/17 17:29:43]

It looks like the shortest exec_script time I see on my Mac is 42ms (probably
process launch overhead) . This file is already the long pole for the build and
delays all file writing (all other build files are loaded in less time than this
file takes to execute).

Then these calls are inside the template which is executed 7 times, so I'm
getting this patch will make GN runs 1.1 seconds slower

So you adding 3 calls to exec script will make GN runs >120ms slower every time.

Can we express this instead as one script call at the top of the file that takes
all the scripts as arguments and returns the modification times in one
exec_script call?

[Robert Sesek, 2016/06/17 18:07:53]

Done. Some numbers for reference:

Before this change: 1505ms
Old patchet (exec_script for each tool version): 3431ms
Latest patch set: 1568ms

       if (is_ios) {
         _sdk_name = ios_sdk_name
         _min_deployment_target = ios_deployment_target
       } else {
         _sdk_name = mac_sdk_name
         _min_deployment_target = mac_deployment_target
       }
-      command = "rm -f {{output}} && " +
+      command = "rm -f {{output}} && " + "TOOL_VERSION=$_tool_version " +
                 "python $_tool -p $_sdk_name -t $_min_deployment_target " +
                 "-o {{output}} {{inputs}}"
 
       description = "COMPILE_XCASSETS {{output}}"
       pool = ":bundle_pool($default_toolchain)"
     }
 
     toolchain_args() {
       current_cpu = invoker.toolchain_cpu
       current_os = invoker.toolchain_os
@@ skipping 84 matching lines @@
 }
 
 mac_toolchain("x64") {
   toolchain_cpu = "x64"
   toolchain_os = "mac"
   cc = "${goma_prefix}/gcc"
   cxx = "${goma_prefix}/g++"
   ld = cxx
   is_clang = false
 }