Android: Combine generate_jni() and generate_jar_jni() build templates

This is a reland of a531e3b1, but has changed
enough that the original commit message is not longer accurate. This CL is
now more of a general refactor, and does not fix the issues seen when moving
source files of generate_jni() targets.

Changes from the original CL:
  * Removed Python 3 incompatibility
  * Removed added dependencies on //build/android/gyp/util/build_utils.py
  * Change jni_generator.py to use argparse
  * Do not rename jni output dirs (causes places that use qualified includes
    to break)

Bug: 951701
Change-Id: I94f63eaf55b16f6c18f7f9eb2c37735c31ed2f60
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1617544
Commit-Queue: Eric Stevenson <estevenson@chromium.org>
Reviewed-by: Andrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662618}

base/android/jni_generator/jni_generator.py

@@ -8,16 +8,18 @@ If you change this, please run and update the tests."""
 
 from __future__ import print_function
 
+import argparse
 import base64
 import collections
 import errno
 import hashlib
-import optparse
 import os
 import re
+import shutil
 from string import Template
 import subprocess
 import sys
+import tempfile
 import textwrap
 import zipfile
 
@@ -1515,35 +1517,6 @@ def WrapOutput(output):
   return '\n'.join(ret)
 
 
-def ExtractJarInputFile(jar_file, input_file, out_dir):
-  """Extracts input file from jar and returns the filename.
-
-  The input file is extracted to the same directory that the generated jni
-  headers will be placed in.  This is passed as an argument to script.
-
-  Args:
-    jar_file: the jar file containing the input files to extract.
-    input_files: the list of files to extract from the jar file.
-    out_dir: the name of the directories to extract to.
-
-  Returns:
-    the name of extracted input file.
-  """
-  jar_file = zipfile.ZipFile(jar_file)
-
-  out_dir = os.path.join(out_dir, os.path.dirname(input_file))
-  try:
-    os.makedirs(out_dir)
-  except OSError as e:
-    if e.errno != errno.EEXIST:
-      raise
-  extracted_file_name = os.path.join(out_dir, os.path.basename(input_file))
-  with open(extracted_file_name, 'wb') as outfile:
-    outfile.write(jar_file.read(input_file))
-
-  return extracted_file_name
-
-
 def GenerateJNIHeader(input_file, output_file, options):
   try:
     if os.path.splitext(input_file)[1] == '.class':
@@ -1573,15 +1546,15 @@ def GetScriptName():
   return os.sep.join(script_components[base_index:])
 
 
-def main(argv):
+def main():
   usage = """usage: %prog [OPTIONS]
 This script will parse the given java source code extracting the native
 declarations and print the header file to stdout (or a file).
 See SampleForTests.java for more details.
   """
-  option_parser = optparse.OptionParser(usage=usage)
+  parser = argparse.ArgumentParser(usage=usage)
 
-  option_parser.add_option(
+  parser.add_argument(
       '-j',
       '--jar_file',
       dest='jar_file',
@@ -1590,73 +1563,77 @@ See SampleForTests.java for more details.
       ' Uses javap to extract the methods from a'
       ' pre-compiled class. --input should point'
       ' to pre-compiled Java .class files.')
-  option_parser.add_option(
+  parser.add_argument(
       '-n',
       dest='namespace',
       help='Uses as a namespace in the generated header '
       'instead of the javap class name, or when there is '
       'no JNINamespace annotation in the java source.')
-  option_parser.add_option(
+  parser.add_argument(
       '--input_file',
-      help='Single input file name. The output file name '
-      'will be derived from it. Must be used with '
-      '--output_dir.')
-  option_parser.add_option(
-      '--output_dir', help='The output directory. Must be used with '
-      '--input')
-  option_parser.add_option(
+      action='append',
+      required=True,
+      dest='input_files',
+      help='Input file names, or paths within a .jar if '
+      '--jar-file is used.')
+  parser.add_argument(
+      '--output_file',
+      action='append',
+      dest='output_files',
+      help='Output file names.')
+  parser.add_argument(
       '--script_name',
       default=GetScriptName(),
       help='The name of this script in the generated '
       'header.')
-  option_parser.add_option(
+  parser.add_argument(
       '--includes',
       help='The comma-separated list of header files to '
       'include in the generated header.')
-  option_parser.add_option(
+  parser.add_argument(
       '--ptr_type',
       default='int',
-      type='choice',
       choices=['int', 'long'],
       help='The type used to represent native pointers in '
       'Java code. For 32-bit, use int; '
       'for 64-bit, use long.')
-  option_parser.add_option(
-      '--cpp', default='cpp', help='The path to cpp command.')
-  option_parser.add_option(
+  parser.add_argument('--cpp', default='cpp', help='The path to cpp command.')
+  parser.add_argument(
       '--javap', default='javap', help='The path to javap command.')
-  option_parser.add_option(
+  parser.add_argument(
       '--enable_profiling',
       action='store_true',
       help='Add additional profiling instrumentation.')
-  option_parser.add_option(
+  parser.add_argument(
       '--enable_tracing',
       action='store_true',
       help='Add TRACE_EVENTs to generated functions.')
-  option_parser.add_option(
+  parser.add_argument(
       '--always_mangle', action='store_true', help='Mangle all function names')
-  option_parser.add_option(
+  parser.add_argument(
       '--use_proxy_hash',
       action='store_true',
       help='Hashes the native declaration of methods used '
       'in @JniNatives interface. And uses a shorter name and package'
       ' than GEN_JNI.')
-  options, args = option_parser.parse_args(argv)
-  if options.jar_file:
-    input_file = ExtractJarInputFile(options.jar_file, options.input_file,
-                                     options.output_dir)
-  elif options.input_file:
-    input_file = options.input_file
-  else:
-    option_parser.print_help()
-    print('\nError: Must specify --jar_file or --input_file.')
-    return 1
-  output_file = None
-  if options.output_dir:
-    root_name = os.path.splitext(os.path.basename(input_file))[0]
-    output_file = os.path.join(options.output_dir, root_name) + '_jni.h'
-  GenerateJNIHeader(input_file, output_file, options)
+  args = parser.parse_args()
+  input_files = args.input_files
+  output_files = args.output_files
+  if not output_files:
+    output_files = [None] * len(input_files)
+
+  temp_dir = tempfile.mkdtemp()
+  try:
+    if args.jar_file:
+      with zipfile.ZipFile(args.jar_file) as z:
+        z.extractall(temp_dir, input_files)
+      input_files = [os.path.join(temp_dir, f) for f in input_files]
+
+    for java_path, header_path in zip(input_files, output_files):
+      GenerateJNIHeader(java_path, header_path, args)
+  finally:
+    shutil.rmtree(temp_dir)
 
 
 if __name__ == '__main__':
-  sys.exit(main(sys.argv))
+  sys.exit(main())

build/config/android/rules.gni

@@ -189,26 +189,8 @@ if (enable_java_templates) {
   import("//build/config/sanitizers/sanitizers.gni")
   import("//tools/grit/grit_rule.gni")
 
-  # Declare a jni target
-  #
-  # This target generates the native jni bindings for a set of .java files.
-  #
-  # See base/android/jni_generator/jni_generator.py for more info about the
-  # format of generating JNI bindings.
-  #
-  # Variables
-  #   sources: list of .java files to generate jni for
-  #   jni_package: subdirectory path for generated bindings
-  #
-  # Example
-  #   generate_jni("foo_jni") {
-  #     sources = [
-  #       "android/java/src/org/chromium/foo/Foo.java",
-  #       "android/java/src/org/chromium/foo/FooUtil.java",
-  #     ]
-  #     jni_package = "foo"
-  #   }
-  template("generate_jni") {
+  # JNI target implementation. See generate_jni or generate_jar_jni for usage.
+  template("generate_jni_impl") {
     set_sources_assignment_filter([])
     forward_variables_from(invoker, [ "testonly" ])
 
@@ -232,42 +214,75 @@ if (enable_java_templates) {
       ]
     }
 
-    _foreach_target_name = "${target_name}__jni_gen"
-    action_foreach_with_pydeps(_foreach_target_name) {
+    _jni_target_name = "${target_name}__jni"
+    action_with_pydeps(_jni_target_name) {
+      # The sources aren't compiled so don't check their dependencies.
+      check_includes = false
       script = "//base/android/jni_generator/jni_generator.py"
-      sources = invoker.sources
-      outputs = [
-        "${_jni_output_dir}/{{source_name_part}}_jni.h",
-      ]
-
+      inputs = []
       args = [
-        "--input_file={{source}}",
         "--ptr_type=long",
-        "--output_dir",
-        rebase_path(_jni_output_dir, root_build_dir),
         "--includes",
         rebase_path(_jni_generator_include, _jni_output_dir),
       ]
 
-      if (use_hashed_jni_names) {
-        args += [ "--use_proxy_hash" ]
+      if (defined(invoker.classes)) {
+        if (defined(invoker.jar_file)) {
+          _jar_file = invoker.jar_file
+        } else {
+          _jar_file = android_sdk_jar
+        }
+        inputs += [ _jar_file ]
+        args += [
+          "--jar_file",
+          rebase_path(_jar_file, root_build_dir),
+        ]
+        _input_args = invoker.classes
+        _input_names = invoker.classes
+        if (defined(invoker.always_mangle) && invoker.always_mangle) {
+          args += [ "--always_mangle" ]
+        }
+      } else {
+        assert(defined(invoker.sources))
+        inputs += invoker.sources
+        _input_args = rebase_path(invoker.sources, root_build_dir)
+        _input_names = invoker.sources
+        if (use_hashed_jni_names) {
+          args += [ "--use_proxy_hash" ]
+        }
+        if (defined(invoker.namespace)) {
+          args += [ "-n ${invoker.namespace}" ]
+        }
+      }
+
+      outputs = []
+      foreach(_name, _input_names) {
+        _name_part = get_path_info(_name, "name")
+        outputs += [ "${_jni_output_dir}/${_name_part}_jni.h" ]
+      }
+
+      # Avoid passing GN lists because not all webrtc embedders use //build.
+      foreach(_output, outputs) {
+        args += [
+          "--output_file",
+          rebase_path(_output, root_build_dir),
+        ]
+      }
+      foreach(_input, _input_args) {
+        args += [ "--input_file=$_input" ]
       }
 
       if (enable_profiling) {
         args += [ "--enable_profiling" ]
       }
-      if (defined(invoker.namespace)) {
-        args += [ "-n ${invoker.namespace}" ]
-      }
       if (enable_jni_tracing) {
         args += [ "--enable_tracing" ]
       }
     }
 
     config("jni_includes_${target_name}") {
-      # TODO(cjhopman): #includes should probably all be relative to
-      # _base_output_dir. Remove that from this config once the includes are
-      # updated.
+      # TODO(crbug.com/963471): Change include paths to be relative to
+      # $root_gen_dir to prevent issues when moving JNI targets.
       include_dirs = [
         _base_output_dir,
         _package_output_dir,
@@ -284,12 +299,38 @@ if (enable_java_templates) {
       if (!defined(public_deps)) {
         public_deps = []
       }
-      public_deps += [ ":$_foreach_target_name" ]
+      public_deps += [ ":$_jni_target_name" ]
       public_deps += _jni_generator_include_deps
       public_configs = [ ":jni_includes_${target_name}" ]
     }
   }
 
+  # Declare a jni target
+  #
+  # This target generates the native jni bindings for a set of .java files.
+  #
+  # See base/android/jni_generator/jni_generator.py for more info about the
+  # format of generating JNI bindings.
+  #
+  # Variables
+  #   sources: list of .java files to generate jni for
+  #   jni_package: subdirectory path for generated bindings
+  #   namespace: Specify the namespace for the generated header file.
+  #
+  # Example
+  #   generate_jni("foo_jni") {
+  #     sources = [
+  #       "android/java/src/org/chromium/foo/Foo.java",
+  #       "android/java/src/org/chromium/foo/FooUtil.java",
+  #     ]
+  #     jni_package = "foo"
+  #   }
+  template("generate_jni") {
+    generate_jni_impl(target_name) {
+      forward_variables_from(invoker, "*")
+    }
+  }
+
   # Declare a jni target for a prebuilt jar
   #
   # This target generates the native jni bindings for a set of classes in a .jar.
@@ -316,82 +357,8 @@ if (enable_java_templates) {
   #     jni_package = "foo"
   #   }
   template("generate_jar_jni") {
-    forward_variables_from(invoker, [ "testonly" ])
-
-    if (defined(invoker.jar_file)) {
-      _jar_file = invoker.jar_file
-    } else {
-      _jar_file = android_sdk_jar
-    }
-
-    _always_mangle = defined(invoker.always_mangle) && invoker.always_mangle
-
-    _base_output_dir = "${target_gen_dir}/${target_name}/${invoker.jni_package}"
-    _jni_output_dir = "${_base_output_dir}/jni"
-
-    if (defined(invoker.jni_generator_include)) {
-      _jni_generator_include = invoker.jni_generator_include
-    } else {
-      _jni_generator_include =
-          "//base/android/jni_generator/jni_generator_helper.h"
-    }
-
-    # TODO(cjhopman): make jni_generator.py support generating jni for multiple
-    # .class files from a .jar.
-    _jni_actions = []
-    foreach(_class, invoker.classes) {
-      _classname = get_path_info(_class, "name")
-      _jni_target_name = "${target_name}__jni_${_classname}"
-      _jni_actions += [ ":$_jni_target_name" ]
-      action_with_pydeps(_jni_target_name) {
-        # The sources aren't compiled so don't check their dependencies.
-        check_includes = false
-        script = "//base/android/jni_generator/jni_generator.py"
-        inputs = [
-          _jar_file,
-        ]
-        outputs = [
-          "${_jni_output_dir}/${_classname}_jni.h",
-        ]
-
-        args = [
-          "--jar_file",
-          rebase_path(_jar_file, root_build_dir),
-          "--input_file",
-          _class,
-          "--ptr_type=long",
-          "--output_dir",
-          rebase_path(_jni_output_dir, root_build_dir),
-          "--includes",
-          rebase_path(_jni_generator_include, _jni_output_dir),
-        ]
-
-        if (enable_profiling) {
-          args += [ "--enable_profiling" ]
-        }
-        if (enable_jni_tracing) {
-          args += [ "--enable_tracing" ]
-        }
-        if (_always_mangle) {
-          args += [ "--always_mangle" ]
-        }
-      }
-    }
-
-    config("jni_includes_${target_name}") {
-      include_dirs = [ _base_output_dir ]
-    }
-
-    group(target_name) {
-      public_deps = []
-      forward_variables_from(invoker,
-                             [
-                               "deps",
-                               "public_deps",
-                               "visibility",
-                             ])
-      public_deps += _jni_actions
-      public_configs = [ ":jni_includes_${target_name}" ]
+    generate_jni_impl(target_name) {
+      forward_variables_from(invoker, "*")
     }
   }