GN(android): Use list of libraries rather than native_lib_dir in all places

Addresses a TODO and makes rules more well suited for adding in
loadable_modules

BUG=559289

Review URL: https://codereview.chromium.org/1483683002

Cr-Original-Commit-Position: refs/heads/master@{#362463}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 7867a73b9d808755d40b8278b312122269720462

android/gyp/apkbuilder.py

@@ -45,9 +45,10 @@ def _ParseArgs(args):
                       required=True)
   parser.add_argument('--dex-file',
                       help='Path to the classes.dex to use')
-  # TODO(agrieve): Switch this to be a list of files rather than a directory.
-  parser.add_argument('--native-libs-dir',
-                      help='Directory containing native libraries to include',
+  parser.add_argument('--native-libs',
+                      action='append',
+                      help='GYP-list of native libraries to include. '
+                           'Can be specified multiple times.',
                       default=[])
   parser.add_argument('--android-abi',
                       help='Android architecture to use for native libraries')
@@ -62,18 +63,17 @@ def _ParseArgs(args):
       options.uncompressed_assets)
   options.native_lib_placeholders = build_utils.ParseGypList(
       options.native_lib_placeholders)
+  all_libs = []
+  for gyp_list in options.native_libs:
+    all_libs.extend(build_utils.ParseGypList(gyp_list))
+  options.native_libs = all_libs
 
-  if not options.android_abi and (options.native_libs_dir or
+  if not options.android_abi and (options.native_libs or
                                   options.native_lib_placeholders):
-    raise Exception('Must specify --android-abi with --native-libs-dir')
+    raise Exception('Must specify --android-abi with --native-libs')
   return options
 
 
-def _ListSubPaths(path):
-  """Returns a list of full paths to all files in the given path."""
-  return [os.path.join(path, name) for name in os.listdir(path)]
-
-
 def _SplitAssetPath(path):
   """Returns (src, dest) given an asset path in the form src[:dest]."""
   path_parts = path.split(':')
@@ -122,10 +122,7 @@ def main(args):
   args = build_utils.ExpandFileArgs(args)
   options = _ParseArgs(args)
 
-  native_libs = []
-  if options.native_libs_dir:
-    native_libs = _ListSubPaths(options.native_libs_dir)
-    native_libs.sort()
+  native_libs = sorted(options.native_libs)
 
   input_paths = [options.resource_apk, __file__] + native_libs
   if options.dex_file:

android/gyp/pack_relocations.py

@@ -64,6 +64,8 @@ def main(args):
   parser.add_option('--libraries', action='append',
       help='List of libraries')
   parser.add_option('--stamp', help='Path to touch on success')
+  parser.add_option('--filelistjson',
+                    help='Output path of filelist.json to write')
 
   options, _ = parser.parse_args(args)
   enable_packing = (options.enable_packing == '1' and
@@ -80,10 +82,12 @@ def main(args):
 
   build_utils.MakeDirectory(options.packed_libraries_dir)
 
+  output_paths = []
   for library in libraries:
     library_path = os.path.join(options.stripped_libraries_dir, library)
     output_path = os.path.join(
         options.packed_libraries_dir, os.path.basename(library))
+    output_paths.append(output_path)
 
     if enable_packing and library not in exclude_packing_set:
       PackLibraryRelocations(options.android_pack_relocations,
@@ -92,6 +96,9 @@ def main(args):
     else:
       CopyLibraryUnchanged(library_path, output_path)
 
+  if options.filelistjson:
+    build_utils.WriteJson({ 'files': output_paths }, options.filelistjson)
+
   if options.depfile:
     build_utils.WriteDepfile(
         options.depfile,

android/incremental_install/create_install_script.py

@@ -37,14 +37,14 @@ def _ResolvePath(path):
 # Exported to allow test runner to be able to install incremental apks.
 def GetInstallParameters():
   apk_path = {apk_path}
-  lib_dir = {lib_dir}
+  native_libs = {native_libs}
   dex_files = {dex_files}
   splits = {splits}
   show_proguard_warning = {show_proguard_warning}
 
   return dict(apk_path=_ResolvePath(apk_path),
               dex_files=[_ResolvePath(p) for p in dex_files],
-              lib_dir=_ResolvePath(lib_dir),
+              native_libs=[_ResolvePath(p) for p in native_libs],
               show_proguard_warning=show_proguard_warning,
               splits=[_ResolvePath(p) for p in splits])
 
@@ -57,8 +57,8 @@ def main():
       _ResolvePath(cmd_path),
       '--output-directory', _ResolvePath(output_directory),
   ]
-  if params['lib_dir']:
-    cmd_args.extend(('--lib-dir', params['lib_dir']))
+  for native_lib in params['native_libs']:
+    cmd_args.extend(('--native_lib', native_lib))
   for dex_path in params['dex_files']:
     cmd_args.extend(('--dex-file', dex_path))
   for split in params['splits']:
@@ -92,8 +92,11 @@ def _ParseArgs(args):
                       default=[],
                       help='A glob matching the apk splits. '
                            'Can be specified multiple times.')
-  parser.add_argument('--lib-dir',
-                      help='Path to native libraries directory.')
+  parser.add_argument('--native-libs',
+                      action='append',
+                      default=[],
+                      help='GYP-list of paths to native libraries. Can be '
+                      'repeated.')
   parser.add_argument('--dex-file',
                       action='append',
                       default=[],
@@ -108,6 +111,10 @@ def _ParseArgs(args):
 
   options = parser.parse_args(args)
   options.dex_files += build_utils.ParseGypList(options.dex_file_list)
+  all_libs = []
+  for gyp_list in options.native_libs:
+    all_libs.extend(build_utils.ParseGypList(gyp_list))
+  options.native_libs = all_libs
   return options
 
 
@@ -125,7 +132,7 @@ def main(args):
       'cmd_path': pformat(relativize(installer_path)),
       'apk_path': pformat(relativize(options.apk_path)),
       'output_directory': pformat(relativize(options.output_directory)),
-      'lib_dir': pformat(relativize(options.lib_dir)),
+      'native_libs': pformat([relativize(p) for p in options.native_libs]),
       'dex_files': pformat([relativize(p) for p in options.dex_files]),
       'show_proguard_warning': pformat(options.show_proguard_warning),
       'splits': pformat([relativize(p) for p in options.splits]),

android/incremental_install/installer.py

@@ -11,7 +11,6 @@ import glob
 import logging
 import os
 import posixpath
-import shutil
 import sys
 
 sys.path.append(os.path.join(os.path.dirname(__file__), os.pardir))
@@ -65,7 +64,7 @@ def Uninstall(device, package):
   logging.info('Uninstall took %s seconds.', main_timer.GetDelta())
 
 
-def Install(device, apk, split_globs=None, lib_dir=None, dex_files=None,
+def Install(device, apk, split_globs=None, native_libs=None, dex_files=None,
             enable_device_cache=True, use_concurrency=True,
             show_proguard_warning=False):
   """Installs the given incremental apk and all required supporting files.
@@ -74,7 +73,7 @@ def Install(device, apk, split_globs=None, lib_dir=None, dex_files=None,
     device: A DeviceUtils instance.
     apk: The path to the apk, or an ApkHelper instance.
     split_globs: Glob patterns for any required apk splits (optional).
-    lib_dir: Directory containing the app's native libraries (optional).
+    native_libs: List of app's native libraries (optional).
     dex_files: List of .dex.jar files that comprise the app's Dalvik code.
     enable_device_cache: Whether to enable on-device caching of checksums.
     use_concurrency: Whether to speed things up using multiple threads.
@@ -105,11 +104,14 @@ def Install(device, apk, split_globs=None, lib_dir=None, dex_files=None,
 
   # Push .so and .dex files to the device (if they have changed).
   def do_push_files():
-    if lib_dir:
+    if native_libs:
       push_native_timer.Start()
-      device_lib_dir = posixpath.join(device_incremental_dir, 'lib')
-      device.PushChangedFiles([(lib_dir, device_lib_dir)],
-                              delete_device_stale=True)
+      with build_utils.TempDir() as temp_dir:
+        device_lib_dir = posixpath.join(device_incremental_dir, 'lib')
+        for path in native_libs:
+          os.symlink(path, os.path.join(temp_dir, os.path.basename(path)))
+        device.PushChangedFiles([(temp_dir, device_lib_dir)],
+                                delete_device_stale=True)
       push_native_timer.Stop(log=False)
 
     if dex_files:
@@ -121,7 +123,7 @@ def Install(device, apk, split_globs=None, lib_dir=None, dex_files=None,
         # Ensure no two files have the same name.
         transformed_names = _TransformDexPaths(dex_files)
         for src_path, dest_name in zip(dex_files, transformed_names):
-          shutil.copyfile(src_path, os.path.join(temp_dir, dest_name))
+          os.symlink(src_path, os.path.join(temp_dir, dest_name))
         device.PushChangedFiles([(temp_dir, device_dex_dir)],
                                 delete_device_stale=True)
       push_dex_timer.Stop(log=False)
@@ -201,10 +203,14 @@ def main():
                       dest='splits',
                       help='A glob matching the apk splits. '
                            'Can be specified multiple times.')
-  parser.add_argument('--lib-dir',
-                      help='Path to native libraries directory.')
-  parser.add_argument('--dex-files',
-                      help='List of dex files to push.',
+  parser.add_argument('--native_lib',
+                      dest='native_libs',
+                      help='Path to native library (repeatable)',
+                      action='append',
+                      default=[])
+  parser.add_argument('--dex-file',
+                      dest='dex_files',
+                      help='Path to dex files (repeatable)',
                       action='append',
                       default=[])
   parser.add_argument('-d', '--device', dest='device',
@@ -270,7 +276,7 @@ def main():
   if args.uninstall:
     Uninstall(device, apk.GetPackageName())
   else:
-    Install(device, apk, split_globs=args.splits, lib_dir=args.lib_dir,
+    Install(device, apk, split_globs=args.splits, native_libs=args.native_libs,
             dex_files=args.dex_files, enable_device_cache=args.cache,
             use_concurrency=args.threading,
             show_proguard_warning=args.show_proguard_warning)

config/android/internal_rules.gni

@@ -667,7 +667,8 @@ template("emma_instr") {
 #   output_apk_path: Output path for the generated .apk.
 #   native_lib_placeholders: List of placeholder filenames to add to the apk
 #     (optional).
-#   native_libs_dir: Directory containing native libraries.
+#   native_libs: List of native libraries.
+#   native_libs_filearg: @FileArg() of additionaly native libraries.
 #   write_asset_list: Adds an extra file to the assets, which contains a list of
 #     all other asset files.
 template("package_apk") {
@@ -689,9 +690,7 @@ template("package_apk") {
       "//tools/android/md5sum",
     ]  # Used when deploying APKs
 
-    inputs = [
-      invoker.resource_packaged_apk_path,
-    ]
+    inputs = invoker.native_libs + [ invoker.resource_packaged_apk_path ]
     if (defined(invoker.dex_path)) {
       inputs += [ invoker.dex_path ]
     }
@@ -727,13 +726,16 @@ template("package_apk") {
       _rebased_dex_path = rebase_path(invoker.dex_path, root_build_dir)
       args += [ "--dex-file=$_rebased_dex_path" ]
     }
-    if (defined(invoker.native_libs_dir) || _native_lib_placeholders != []) {
+    if (invoker.native_libs != [] || defined(invoker.native_libs_filearg) ||
+        _native_lib_placeholders != []) {
       args += [ "--android-abi=$android_app_abi" ]
     }
-    if (defined(invoker.native_libs_dir)) {
-      _rebased_native_libs_dir =
-          rebase_path(invoker.native_libs_dir, root_build_dir)
-      args += [ "--native-libs-dir=$_rebased_native_libs_dir/$android_app_abi" ]
+    if (invoker.native_libs != []) {
+      _rebased_native_libs = rebase_path(invoker.native_libs, root_build_dir)
+      args += [ "--native-libs=$_rebased_native_libs" ]
+    }
+    if (defined(invoker.native_libs_filearg)) {
+      args += [ "--native-libs=${invoker.native_libs_filearg}" ]
     }
     if (_native_lib_placeholders != []) {
       args += [ "--native-lib-placeholders=$_native_lib_placeholders" ]
@@ -840,6 +842,10 @@ template("create_apk") {
   if (defined(invoker.incremental_deps)) {
     _incremental_deps = invoker.incremental_deps
   }
+  _native_libs = []
+  if (defined(invoker.native_libs)) {
+    _native_libs = invoker.native_libs
+  }
 
   # TODO(agrieve): Remove support for asset_location in favor of using
   # android_assets() everywhere (http://crbug.com/547162).
@@ -1014,10 +1020,11 @@ template("create_apk") {
                              "assets_build_config",
                              "emma_instrument",
                              "native_lib_placeholders",
-                             "native_libs_dir",
+                             "native_libs_filearg",
                              "write_asset_list",
                            ])
     deps = _deps + [ ":${_package_resources_target_name}" ]
+    native_libs = _native_libs
 
     if (defined(_dex_path)) {
       dex_path = _dex_path
@@ -1045,8 +1052,12 @@ template("create_apk") {
           get_label_info(_dex_target, "target_gen_dir") + "/bootstrap.dex"
     }
 
+    native_libs = []
+
     # http://crbug.com/384638
-    if (defined(invoker.native_libs_dir)) {
+    _has_native_libs =
+        defined(invoker.native_libs_filearg) || _native_libs != []
+    if (_has_native_libs) {
       native_lib_placeholders = [ "libfix.crbug.384638.so" ]
     }
 

config/android/rules.gni

@@ -1293,30 +1293,29 @@ template("android_apk") {
     _srcjar_deps += invoker.srcjar_deps
   }
 
-  _load_library_from_apk = false
+  _use_chromium_linker =
+      defined(invoker.use_chromium_linker) && invoker.use_chromium_linker &&
+      chromium_linker_supported
+  _enable_relocation_packing =
+      defined(invoker.enable_relocation_packing) &&
+      invoker.enable_relocation_packing && _use_chromium_linker
+  _load_library_from_apk =
+      defined(invoker.load_library_from_apk) && invoker.load_library_from_apk
+
+  assert(_use_chromium_linker || true)  # Mark as used.
+  assert(_enable_relocation_packing || true)  # Mark as used.
+
+  assert(
+      !_load_library_from_apk || _use_chromium_linker,
+      "Loading library from the apk requires use" + " of the Chromium linker.")
 
   # The dependency that makes the chromium linker, if any is needed.
-  _chromium_linker_dep = []
+  _native_libs_deps = []
 
   if (defined(invoker.native_libs) && invoker.native_libs != []) {
-    _use_chromium_linker = false
-    if (defined(invoker.use_chromium_linker)) {
-      _use_chromium_linker =
-          invoker.use_chromium_linker && chromium_linker_supported
-      _chromium_linker_dep = [ "//base/android/linker:chromium_android_linker" ]
-    }
-
-    if (defined(invoker.load_library_from_apk) &&
-        invoker.load_library_from_apk) {
-      _load_library_from_apk = true
-      assert(_use_chromium_linker,
-             "Loading library from the apk requires use" +
-                 " of the Chromium linker.")
-    }
-
     if (is_component_build) {
       _native_libs += [ "$root_shlib_dir/libc++_shared.so" ]
-      _chromium_linker_dep += [ "//build/android:cpplib_stripped" ]
+      _native_libs_deps += [ "//build/android:cpplib_stripped" ]
     }
 
     # Allow native_libs to be in the form "foo.so" or "foo.cr.so"
@@ -1326,21 +1325,6 @@ template("android_apk") {
             _first_ext_removed,
             "$root_shlib_dir/{{source_name_part}}$shlib_extension")
 
-    # Add in target_cpu so that other architectures are not accidentally
-    # included when switching target_cpu without doing a clean build.
-    _native_libs_dir = gen_dir + "/lib-$target_cpu"
-
-    if (_use_chromium_linker) {
-      _native_libs +=
-          [ "$root_shlib_dir/libchromium_android_linker$shlib_extension" ]
-    }
-
-    _enable_relocation_packing = false
-    if (_use_chromium_linker && defined(invoker.enable_relocation_packing) &&
-        invoker.enable_relocation_packing) {
-      _enable_relocation_packing = true
-    }
-
     _native_lib_version_rule = ""
     if (defined(invoker.native_lib_version_rule)) {
       _native_lib_version_rule = invoker.native_lib_version_rule
@@ -1387,7 +1371,7 @@ template("android_apk") {
     build_config = _build_config
     android_manifest = _android_manifest
 
-    deps = _chromium_linker_dep + _android_manifest_deps
+    deps = _native_libs_deps + _android_manifest_deps
     if (defined(invoker.deps)) {
       deps += invoker.deps
     }
@@ -1592,62 +1576,67 @@ template("android_apk") {
     }
   }
 
-  if (_native_libs != []) {
+  _native_libs_file_arg_dep = ":$build_config_target"
+  _native_libs_file_arg = "@FileArg($_rebased_build_config:native:libraries)"
+
+  if (_native_libs != [] && _enable_relocation_packing) {
     _prepare_native_target_name = "${_template_name}__prepare_native"
+    _native_libs_dir = "$gen_dir/packed-libs"
+    _native_libs_json = "$gen_dir/packed-libs/filelist.json"
+    _rebased_native_libs_json = rebase_path(_native_libs_json, root_build_dir)
+
+    _native_libs_file_arg_dep = ":$_prepare_native_target_name"
+    _native_libs_file_arg = "@FileArg($_rebased_native_libs_json:files)"
+
     action(_prepare_native_target_name) {
       forward_variables_from(invoker,
                              [
-                               "data_deps",
+                               "deps",
                                "public_deps",
                              ])
       script = "//build/android/gyp/pack_relocations.py"
-      packed_libraries_dir = "$_native_libs_dir/$android_app_abi"
       depfile = "$target_gen_dir/$target_name.d"
       outputs = [
         depfile,
+        _native_libs_json,
       ]
 
-      inputs = _native_libs
-      deps = _chromium_linker_dep
+      inputs = _native_libs + [ _build_config ]
 
-      inputs += [ _build_config ]
-      deps += [ ":$build_config_target" ]
-
-      rebased_gdbserver = rebase_path(android_gdbserver, root_build_dir)
-      skip_packing_list = [
-        rebased_gdbserver,
-        "libchromium_android_linker.so",
+      deps += _native_libs_deps
+      deps += [
+        ":$build_config_target",
+        relocation_packer_target,
       ]
 
-      enable_packing_arg = 0
-      if (_enable_relocation_packing) {
-        enable_packing_arg = 1
-        deps += [ relocation_packer_target ]
-      }
-
       args = [
         "--depfile",
         rebase_path(depfile, root_build_dir),
-        "--enable-packing=$enable_packing_arg",
-        "--exclude-packing-list=$skip_packing_list",
+        "--enable-packing=1",
         "--android-pack-relocations",
         rebase_path(relocation_packer_exe, root_build_dir),
         "--stripped-libraries-dir",
         rebase_path(root_build_dir, root_build_dir),
         "--packed-libraries-dir",
-        rebase_path(packed_libraries_dir, root_build_dir),
+        rebase_path(_native_libs_dir, root_build_dir),
         "--libraries=@FileArg(${_rebased_build_config}:native:libraries)",
-        "--clear-dir",
+        "--filelistjson=$_rebased_native_libs_json",
       ]
+    }
+  }
 
-      if (defined(invoker.deps)) {
-        deps += invoker.deps
-      }
+  _extra_native_libs = []
+  _extra_native_libs_deps = []
+  if (_native_libs != []) {
+    if (is_debug) {
+      _extra_native_libs = [ android_gdbserver ]
+    }
 
-      if (is_debug) {
-        inputs += [ android_gdbserver ]
-        args += [ "--libraries=$rebased_gdbserver" ]
-      }
+    if (_use_chromium_linker) {
+      _extra_native_libs =
+          [ "$root_shlib_dir/libchromium_android_linker$shlib_extension" ]
+      _extra_native_libs_deps +=
+          [ "//base/android/linker:chromium_android_linker" ]
     }
   }
 
@@ -1660,6 +1649,7 @@ template("android_apk") {
                              "deps",
                              "extensions_to_not_compress",
                              "language_splits",
+                             "public_deps",
                              "shared_resources",
                              "write_asset_list",
                            ])
@@ -1700,13 +1690,14 @@ template("android_apk") {
             ]
 
     if (_native_libs != [] && !_create_abi_split) {
-      native_libs_dir = _native_libs_dir
+      deps += _native_libs_deps + _extra_native_libs_deps +
+              [ _native_libs_file_arg_dep ]
+      native_libs_filearg = _native_libs_file_arg
+      native_libs = _extra_native_libs
 
       # Placeholders necessary for some older devices.
       # http://crbug.com/395038
       forward_variables_from(invoker, [ "native_lib_placeholders" ])
-
-      deps += [ ":$_prepare_native_target_name" ]
     }
   }
 
@@ -1738,17 +1729,22 @@ template("android_apk") {
       keystore_path = _keystore_path
       keystore_password = _keystore_password
 
-      native_libs_dir = _native_libs_dir
-
       # Placeholders necessary for some older devices.
       # http://crbug.com/395038
-      forward_variables_from(invoker, [ "native_lib_placeholders" ])
+      deps = []
+      forward_variables_from(invoker,
+                             [
+                               "deps",
+                               "native_lib_placeholders",
+                               "public_deps",
+                             ])
 
-      deps = [
-        ":${_manifest_rule}",
-        ":${_prepare_native_target_name}",
-      ]
-      incremental_deps = deps
+      incremental_deps = deps + [ ":$_manifest_rule" ]
+      deps = []
+      deps = incremental_deps + _native_libs_deps + _extra_native_libs_deps +
+             [ _native_libs_file_arg_dep ]
+      native_libs_filearg = _native_libs_file_arg
+      native_libs = _extra_native_libs
     }
   }
 
@@ -1757,7 +1753,7 @@ template("android_apk") {
     script = "//build/android/incremental_install/create_install_script.py"
     depfile = "$target_gen_dir/$target_name.d"
     deps = [
-      ":$build_config_target",
+      _native_libs_file_arg_dep,
     ]
 
     _generated_script_path =
@@ -1783,9 +1779,13 @@ template("android_apk") {
     if (_proguard_enabled) {
       args += [ "--show-proguard-warning" ]
     }
-    if (defined(_native_libs_dir)) {
-      _rebased_native_libs_dir = rebase_path(_native_libs_dir, root_build_dir)
-      args += [ "--lib-dir=$_rebased_native_libs_dir/$android_app_abi" ]
+    if (defined(_native_libs_file_arg)) {
+      args += [ "--native-libs=$_native_libs_file_arg" ]
+    }
+    if (_extra_native_libs != []) {
+      _rebased_extra_native_libs =
+          rebase_path(_extra_native_libs, root_build_dir)
+      args += [ "--native-libs=$_rebased_extra_native_libs" ]
     }
     if (_create_density_splits) {
       args += [ "--split=${_rebased_apk_path_no_ext}-density-*.apk" ]
@@ -1804,7 +1804,8 @@ template("android_apk") {
     forward_variables_from(invoker, [ "data_deps" ])
 
     # device/commands is used by the installer script to push files via .zip.
-    data_deps += [ "//build/android/pylib/device/commands" ]
+    data_deps += [ "//build/android/pylib/device/commands" ] +
+                 _native_libs_deps + _extra_native_libs_deps
 
     # Since the _incremental.apk does not include use .so nor .dex from the
     # actual target, but instead loads them at runtime, we need to explicitly
@@ -1814,9 +1815,6 @@ template("android_apk") {
       ":${_template_name}__create_incremental",
       ":${java_target}",
     ]
-    if (_native_libs != []) {
-      public_deps += [ ":$_prepare_native_target_name" ]
-    }
   }
 }