Add a python2_action() template and use it in :histograms_xml.

As one of the first steps in making the build work with Python 3,
we will need a way to indicate which actions still require Python 2
to run (most work fine under Python 3 already, but some do not).

This CL introduces a python2_action() template, which works just
like a regular action() rule, but ensures that the script runs
under Python 2 instead of whichever Python version you happen to be
running.

This CL also switches //tools/metrics:histograms_xml to use it
(because it seems to need it) as a proof that it works correctly
and does not regress in a normal (python 2) build.

Bug: 941669, 1118214
Change-Id: I98fbc073cf9443acece3caaae2ed35f9d60c38e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2363655
Commit-Queue: Dirk Pranke <dpranke@google.com>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799909}

build/config/python.gni

@@ -63,6 +63,86 @@ template("python_library") {
   }
 }
 
+# This is a wrapper around action() that ensures that the script is
+# run under a Python2 executable, even if the main script_executable is
+# Python3.
+#
+# It supports all of action()'s arguments.
+#
+# TODO(crbug.com/1112471): Remove this once everything runs cleanly under
+# Python3.
+template("python2_action") {
+  action(target_name) {
+    # Forward all variables. Ensure that testonly and visibility are forwarded
+    # explicitly, since this performs recursive scope lookups, which is
+    # required to ensure their definition from scopes above the caller are
+    # properly handled. All other variables are forwarded with "*", which
+    # doesn't perform recursive lookups at all. See https://crbug.com/862232
+    forward_variables_from(invoker,
+                           [
+                             "testonly",
+                             "visibility",
+                           ])
+    forward_variables_from(invoker,
+                           "*",
+                           [
+                             "script",
+                             "inputs",
+                             "args",
+                             "testonly",
+                             "visibility",
+                           ])
+
+    script = "//build/util/python2_action.py"
+    if (!defined(invoker.inputs)) {
+      inputs = [ invoker.script ]
+    } else {
+      inputs = [ invoker.script ] + invoker.inputs
+    }
+    args = [ rebase_path(invoker.script, root_build_dir) ] + invoker.args
+  }
+}
+
+# This is a wrapper around action() that ensures that the script is
+# run under a Python2 executable, even if the main script_executable is
+# Python3.
+#
+# It supports all of action()'s arguments.
+#
+# TODO(crbug.com/1112471): Remove this once everything runs cleanly under
+# Python3.
+template("python2_action_foreach") {
+  action_foreach(target_name) {
+    # Forward all variables. Ensure that testonly and visibility are forwarded
+    # explicitly, since this performs recursive scope lookups, which is
+    # required to ensure their definition from scopes above the caller are
+    # properly handled. All other variables are forwarded with "*", which
+    # doesn't perform recursive lookups at all. See https://crbug.com/862232
+    forward_variables_from(invoker,
+                           [
+                             "testonly",
+                             "visibility",
+                           ])
+    forward_variables_from(invoker,
+                           "*",
+                           [
+                             "script",
+                             "inputs",
+                             "args",
+                             "testonly",
+                             "visibility",
+                           ])
+
+    script = "//build/run_under_python2.py"
+    if (!defined(invoker.inputs)) {
+      inputs = [ invoker.script ]
+    } else {
+      inputs = [ invoker.script ] + invoker.inputs
+    }
+    args = [ rebase_path(invoker.script, root_build_dir) ] + invoker.args
+  }
+}
+
 # A template used for actions that execute a Python script, which has an
 # associated .pydeps file. In other words:
 #
@@ -103,6 +183,7 @@ template("action_with_pydeps") {
     # Happens every time the template is instantiated, but benchmarking shows no
     # perceivable impact on overall 'gn gen' speed.
     _pydeps_file = invoker.script + "deps"
+
     _pydeps_lines =
         read_file(_pydeps_file, "list lines")  # https://crbug.com/1102058
     _pydeps_entries = filter_exclude(_pydeps_lines, [ "#*" ])
@@ -115,6 +196,14 @@ template("action_with_pydeps") {
     # expects paths that are relative to the current BUILD.gn
     _script_dir = get_path_info(_pydeps_file, "dir")
     inputs += rebase_path(_pydeps_entries, ".", _script_dir)
+
+    if (defined(run_under_python2) && run_under_python2) {
+      inputs += [ invoker.script ]
+      _args = args
+      args = []
+      args = [ rebase_path(invoker.script, root_build_dir) ] + _args
+      script = "//build/run_under_python2.py"
+    }
   }
 }
 
@@ -140,7 +229,11 @@ template("action_foreach_with_pydeps") {
     # Read and filter out comments.
     # Happens every time the template is instantiated, but benchmarking shows no
     # perceivable impact on overall 'gn gen' speed.
-    _pydeps_file = invoker.script + "deps"
+    if (defined(invoker.deps_file)) {
+      _pydeps_file = invoker.deps_file
+    } else {
+      _pydeps_file = invoker.script + "deps"
+    }
     _pydeps_lines = read_file(_pydeps_file, "list lines")
     _pydeps_entries = filter_exclude(_pydeps_lines, [ "#*" ])
 
@@ -152,5 +245,13 @@ template("action_foreach_with_pydeps") {
     # expects paths that are relative to the current BUILD.gn
     _script_dir = get_path_info(script, "dir")
     inputs += rebase_path(_pydeps_entries, ".", _script_dir)
+
+    if (defined(run_under_python2) && run_under_python2) {
+      inputs += [ invoker.script ]
+      _args = args
+      args = []
+      args = [ rebase_path(invoker.script, root_build_dir) ] + _args
+      script = "//build/run_under_python2.py"
+    }
   }
 }

build/util/python2_action.py

+# Copyright 2020 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.
+"""Script for ensuring that a python action runs under Python2, not Python3."""
+
+import subprocess
+import sys
+
+if sys.version_info.major == 2:
+  # If we get here, we're already Python2, so just re-execute the
+  # command without the wrapper.
+  exe = sys.executable
+elif sys.executable.endswith('.exe'):
+  # If we get here, we're a Python3 executable likely running on
+  # Windows, so look for the Python2 wrapper in depot_tools.
+  exe = 'python.bat'
+else:
+  # If we get here, we are a Python3 executable. Hope that we can find
+  # a `python2.7` in path somewhere.
+  exe = 'python2.7'
+
+sys.exit(subprocess.call([exe] + sys.argv[1:]))

tools/metrics/BUILD.gn

@@ -2,11 +2,17 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-action("histograms_xml") {
+import("//build/config/python.gni")
+
+# TODO(crbug.com/1118214) - Make work under Python 3.
+python2_action("histograms_xml") {
   script = "histograms/merge_xml.py"
   output = "$root_out_dir/histograms.xml"
   outputs = [ output ]
-  args = ["--output", rebase_path(output, root_build_dir)]
+  args = [
+    "--output",
+    rebase_path(output, root_build_dir),
+  ]
 }
 
 copy("actions_xml") {