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

build/toolchain/mac/BUILD.gn

Index: build/toolchain/mac/BUILD.gn
diff --git a/build/toolchain/mac/BUILD.gn b/build/toolchain/mac/BUILD.gn
index 146744030c2e838f51fe3b71a9adc55c1e20f721..3ea247265aa12a25fbde0be8e3227653a02ad3da 100644
--- a/build/toolchain/mac/BUILD.gn
+++ b/build/toolchain/mac/BUILD.gn
@@ -59,8 +59,23 @@ template("mac_toolchain") {
     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"
@@ -136,7 +151,8 @@ template("mac_toolchain") {
     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}}",
@@ -278,13 +294,16 @@ template("mac_toolchain") {
     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
@@ -292,7 +311,7 @@ template("mac_toolchain") {
         _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}}"