Fix Clank official build failure. Speculatively revert CL.

Clank Official Build Failure:
~~~
  File "../../build/android/gyp/apkbuilder.py", line 383, in <module>
    main(sys.argv[1:])
  File "../../build/android/gyp/apkbuilder.py", line 307, in main
    with zipfile.ZipFile(options.dex_file, 'r') as dex_zip:
  File "/usr/lib/python2.7/zipfile.py", line 770, in __init__
    self._RealGetContents()
  File "/usr/lib/python2.7/zipfile.py", line 857, in _RealGetContents
    x._decodeExtra()
  File "/usr/lib/python2.7/zipfile.py", line 388, in _decodeExtra
    tp, ln = unpack('<HH', extra[:4])
struct.error: unpack requires a string argument of length 4
~~~

This CL speculatively revert the CLs that seems to be linked with build
failure.

The bug happens while trying to zip dex file:
~~~
with zipfile.ZipFile(options.dex_file, 'r') as dex_zip:
~~~
and the following branch of CL deals with zip + dex files:

Revert "dex.py: Remove obsolete flags, code clean-up."
  commit 35fc212e.
  https://chromium-review.googlesource.com/c/chromium/src/+/1694090/4
Revert "Android: Zipalign library dex files for incremental install"
  commit 209eac23.
  https://chromium-review.googlesource.com/c/chromium/src/+/1692492/6

TBR=agrieve@chromium.org

Bug: 983519
Change-Id: Ia90b7f5d16b09ae29cf0617a09a80e5312ed858a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1700064
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Sam Maier <smaier@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Auto-Submit: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676898}

build/android/gyp/dex.py

@@ -4,8 +4,9 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-import argparse
+import json
 import logging
+import optparse
 import os
 import re
 import shutil
@@ -20,55 +21,76 @@ sys.path.insert(1, os.path.join(os.path.dirname(__file__), os.path.pardir))
 import convert_dex_profile
 
 
-_FIXED_ZIP_HEADER_LEN = 30
+def _CheckFilePathEndsWithJar(parser, file_path):
+  if not file_path.endswith(".jar"):
+    parser.error("%s does not end in .jar" % file_path)
+
+
+def _CheckFilePathsEndWithJar(parser, file_paths):
+  for file_path in file_paths:
+    _CheckFilePathEndsWithJar(parser, file_path)
 
 
 def _ParseArgs(args):
   args = build_utils.ExpandFileArgs(args)
-  parser = argparse.ArgumentParser()
 
+  parser = optparse.OptionParser()
   build_utils.AddDepfileOption(parser)
-  parser.add_argument('--output', required=True, help='Dex output path.')
-  parser.add_argument('--input-list', help='GN-list of additional input paths.')
-  parser.add_argument(
-      '--main-dex-list-path',
-      help='File containing a list of the classes to include in the main dex.')
-  parser.add_argument(
-      '--multi-dex',
-      action='store_true',
-      help='Allow multiple dex files within output.')
-  parser.add_argument('--d8-jar-path', required=True, help='Path to D8 jar.')
-  parser.add_argument(
-      '--release',
-      action='store_true',
-      help='Run D8 in release mode. Release mode maximises main dex and '
-      'deletes non-essential line number information (vs debug which minimizes '
-      'main dex and keeps all line number information, and then some.')
-  parser.add_argument(
-      '--min-api', help='Minimum Android API level compatibility.')
-  parser.add_argument('inputs', nargs='*', help='Input .jar files.')
-
-  group = parser.add_argument_group('Dexlayout')
-  group.add_argument(
-      '--dexlayout-profile',
-      help=('Text profile for dexlayout. If present, a dexlayout '
-            'pass will happen'))
-  group.add_argument(
-      '--profman-path',
-      help=('Path to ART profman binary. There should be a lib/ directory at '
-            'the same path with shared libraries (shared with dexlayout).'))
-  group.add_argument(
-      '--dexlayout-path',
-      help=('Path to ART dexlayout binary. There should be a lib/ directory at '
-            'the same path with shared libraries (shared with dexlayout).'))
-  group.add_argument('--dexdump-path', help='Path to dexdump binary.')
-  group.add_argument(
+
+  parser.add_option('--output-directory',
+                    default=os.getcwd(),
+                    help='Path to the output build directory.')
+  parser.add_option('--dex-path', help='Dex output path.')
+  parser.add_option('--configuration-name',
+                    help='The build CONFIGURATION_NAME.')
+  parser.add_option('--proguard-enabled',
+                    help='"true" if proguard is enabled.')
+  parser.add_option('--debug-build-proguard-enabled',
+                    help='"true" if proguard is enabled for debug build.')
+  parser.add_option('--proguard-enabled-input-path',
+                    help=('Path to dex in Release mode when proguard '
+                          'is enabled.'))
+  parser.add_option('--inputs', help='A list of additional input paths.')
+  parser.add_option('--excluded-paths',
+                    help='A list of paths to exclude from the dex file.')
+  parser.add_option('--main-dex-list-path',
+                    help='A file containing a list of the classes to '
+                         'include in the main dex.')
+  parser.add_option('--multidex-configuration-path',
+                    help='A JSON file containing multidex build configuration.')
+  parser.add_option('--multi-dex', default=False, action='store_true',
+                    help='Generate multiple dex files.')
+  parser.add_option('--d8-jar-path', help='Path to D8 jar.')
+  parser.add_option('--release', action='store_true', default=False,
+                    help='Run D8 in release mode. Release mode maximises main '
+                    'dex and deletes non-essential line number information '
+                    '(vs debug which minimizes main dex and keeps all line '
+                    'number information, and then some.')
+  parser.add_option('--min-api',
+                    help='Minimum Android API level compatibility.')
+
+  parser.add_option('--dexlayout-profile',
+                    help=('Text profile for dexlayout. If present, a dexlayout '
+                          'pass will happen'))
+  parser.add_option('--profman-path',
+                    help=('Path to ART profman binary. There should be a '
+                          'lib/ directory at the same path containing shared '
+                          'libraries (shared with dexlayout).'))
+  parser.add_option('--dexlayout-path',
+                    help=('Path to ART dexlayout binary. There should be a '
+                          'lib/ directory at the same path containing shared '
+                          'libraries (shared with dexlayout).'))
+  parser.add_option('--dexdump-path', help='Path to dexdump binary.')
+  parser.add_option(
       '--proguard-mapping-path',
       help=('Path to proguard map from obfuscated symbols in the jar to '
-            'unobfuscated symbols present in the code. If not present, the jar '
-            'is assumed not to be obfuscated.'))
+            'unobfuscated symbols present in the code. If not '
+            'present, the jar is assumed not to be obfuscated.'))
 
-  options = parser.parse_args(args)
+  options, paths = parser.parse_args(args)
+
+  required_options = ('d8_jar_path',)
+  build_utils.CheckOptions(options, parser, required=required_options)
 
   if options.dexlayout_profile:
     build_utils.CheckOptions(
@@ -76,19 +98,67 @@ def _ParseArgs(args):
         parser,
         required=('profman_path', 'dexlayout_path', 'dexdump_path'))
   elif options.proguard_mapping_path is not None:
-    parser.error('Unexpected proguard mapping without dexlayout')
+    raise Exception('Unexpected proguard mapping without dexlayout')
+
+  if options.multidex_configuration_path:
+    with open(options.multidex_configuration_path) as multidex_config_file:
+      multidex_config = json.loads(multidex_config_file.read())
+    options.multi_dex = multidex_config.get('enabled', False)
 
   if options.main_dex_list_path and not options.multi_dex:
-    parser.error('--main-dex-list-path is unused if multidex is not enabled')
+    logging.warning('--main-dex-list-path is unused if multidex is not enabled')
+
+  if options.inputs:
+    options.inputs = build_utils.ParseGnList(options.inputs)
+    _CheckFilePathsEndWithJar(parser, options.inputs)
+  if options.excluded_paths:
+    options.excluded_paths = build_utils.ParseGnList(options.excluded_paths)
+
+  if options.proguard_enabled_input_path:
+    _CheckFilePathEndsWithJar(parser, options.proguard_enabled_input_path)
+  _CheckFilePathsEndWithJar(parser, paths)
+
+  return options, paths
+
 
-  if options.input_list:
-    options.inputs += build_utils.ParseGnList(options.input_list)
+def _MoveTempDexFile(tmp_dex_dir, dex_path):
+  """Move the temp dex file out of |tmp_dex_dir|.
+
+  Args:
+    tmp_dex_dir: Path to temporary directory created with tempfile.mkdtemp().
+      The directory should have just a single file.
+    dex_path: Target path to move dex file to.
+
+  Raises:
+    Exception if there are multiple files in |tmp_dex_dir|.
+  """
+  tempfiles = os.listdir(tmp_dex_dir)
+  if len(tempfiles) > 1:
+    raise Exception('%d files created, expected 1' % len(tempfiles))
+
+  tmp_dex_path = os.path.join(tmp_dex_dir, tempfiles[0])
+  shutil.move(tmp_dex_path, dex_path)
+
+
+def _NoClassFiles(jar_paths):
+  """Returns True if there are no .class files in the given JARs.
+
+  Args:
+    jar_paths: list of strings representing JAR file paths.
 
-  return options
+  Returns:
+    (bool) True if no .class files are found.
+  """
+  for jar_path in jar_paths:
+    with zipfile.ZipFile(jar_path) as jar:
+      if any(name.endswith('.class') for name in jar.namelist()):
+        return False
+  return True
 
 
 def _RunD8(dex_cmd, input_paths, output_path):
-  dex_cmd = dex_cmd + ['--output', output_path] + input_paths
+  dex_cmd += ['--output', output_path]
+  dex_cmd += input_paths
   build_utils.CheckOutput(dex_cmd, print_stderr=False)
 
 
@@ -172,7 +242,7 @@ def _LayoutDex(binary_profile, input_dex, dexlayout_path, temp_dir):
   output_files = os.listdir(dexlayout_output_dir)
   if not output_files:
     raise Exception('dexlayout unexpectedly produced no output')
-  return sorted([os.path.join(dexlayout_output_dir, f) for f in output_files])
+  return [os.path.join(dexlayout_output_dir, f) for f in output_files]
 
 
 def _ZipMultidex(file_dir, dex_files):
@@ -214,53 +284,37 @@ def _ZipMultidex(file_dir, dex_files):
   return zip_name
 
 
-def _ZipAligned(dex_files, output_path):
-  """Creates a .dex.jar with 4-byte aligned files.
+def _ZipSingleDex(dex_file, zip_name):
+  """Zip up a single dex file.
 
   Args:
-    dex_files: List of dex files.
-    output_path: The output file in which to write the zip.
+    dex_file: A dexfile whose name is ignored.
+    zip_name: The output file in which to write the zip.
   """
-  with zipfile.ZipFile(output_path, 'w') as z:
-    for i, dex_file in enumerate(dex_files):
-      name = 'classes{}.dex'.format(i + 1 if i > 0 else '')
-      zip_info = build_utils.HermeticZipInfo(filename=name)
-      cur_offset = z.fp.tell()
-      header_size = _FIXED_ZIP_HEADER_LEN + len(name)
-      alignment_needed = (4 - cur_offset + header_size) % 4
-
-      # Extra field used to 4-byte align classes.dex. Alignment speeds up
-      # execution when dex files are used via incremental install.
-      zip_info.extra = b'\0' * alignment_needed
-      with open(dex_file) as f:
-        z.writestr(zip_info, f.read())
-
-
-def _PerformDexlayout(tmp_dir, tmp_dex_output, options):
-  if options.proguard_mapping_path is not None:
-    matching_profile = os.path.join(tmp_dir, 'obfuscated_profile')
-    convert_dex_profile.ObfuscateProfile(
-        options.dexlayout_profile, tmp_dex_output,
-        options.proguard_mapping_path, options.dexdump_path, matching_profile)
-  else:
-    logging.warning('No obfuscation for %s', options.dexlayout_profile)
-    matching_profile = options.dexlayout_profile
-  binary_profile = _CreateBinaryProfile(matching_profile, tmp_dex_output,
-                                        options.profman_path, tmp_dir)
-  output_files = _LayoutDex(binary_profile, tmp_dex_output,
-                            options.dexlayout_path, tmp_dir)
-  if len(output_files) > 1:
-    return _ZipMultidex(tmp_dir, output_files)
-
-  if zipfile.is_zipfile(output_files[0]):
-    return output_files[0]
-
-  final_output = os.path.join(tmp_dir, 'dex_classes.zip')
-  _ZipAligned(output_files, final_output)
-  return final_output
-
-
-def _PerformDexing(options):
+  build_utils.DoZip([('classes.dex', dex_file)], zip_name)
+
+
+def main(args):
+  options, paths = _ParseArgs(args)
+  if ((options.proguard_enabled == 'true'
+          and options.configuration_name == 'Release')
+      or (options.debug_build_proguard_enabled == 'true'
+          and options.configuration_name == 'Debug')):
+    paths = [options.proguard_enabled_input_path]
+
+  if options.inputs:
+    paths += options.inputs
+
+  if options.excluded_paths:
+    # Excluded paths are relative to the output directory.
+    exclude_paths = options.excluded_paths
+    paths = [p for p in paths if not
+             os.path.relpath(p, options.output_directory) in exclude_paths]
+
+  input_paths = list(paths)
+  if options.multi_dex and options.main_dex_list_path:
+    input_paths.append(options.main_dex_list_path)
+
   dex_cmd = ['java', '-jar', options.d8_jar_path, '--no-desugaring']
   if options.multi_dex and options.main_dex_list_path:
     dex_cmd += ['--main-dex-list', options.main_dex_list_path]
@@ -269,39 +323,62 @@ def _PerformDexing(options):
   if options.min_api:
     dex_cmd += ['--min-api', options.min_api]
 
+  is_dex = options.dex_path.endswith('.dex')
+  is_jar = options.dex_path.endswith('.jar')
+
   with build_utils.TempDir() as tmp_dir:
     tmp_dex_dir = os.path.join(tmp_dir, 'tmp_dex_dir')
     os.mkdir(tmp_dex_dir)
-    _RunD8(dex_cmd, options.inputs, tmp_dex_dir)
-    dex_files = [os.path.join(tmp_dex_dir, f) for f in os.listdir(tmp_dex_dir)]
-
-    if not options.output.endswith('.dex'):
-      tmp_dex_output = os.path.join(tmp_dir, 'tmp_dex_output.zip')
-      _ZipAligned(sorted(dex_files), tmp_dex_output)
+    if is_jar and _NoClassFiles(paths):
+      # Handle case where no classfiles are specified in inputs
+      # by creating an empty JAR
+      with zipfile.ZipFile(options.dex_path, 'w') as outfile:
+        outfile.comment = 'empty'
+    else:
+      # .dex files can't specify a name for D8. Instead, we output them to a
+      # temp directory then move them after the command has finished running
+      # (see _MoveTempDexFile). For other files, tmp_dex_dir is None.
+      _RunD8(dex_cmd, paths, tmp_dex_dir)
+
+    tmp_dex_output = os.path.join(tmp_dir, 'tmp_dex_output')
+    if is_dex:
+      _MoveTempDexFile(tmp_dex_dir, tmp_dex_output)
     else:
-      # Output to a .dex file.
-      if len(dex_files) > 1:
-        raise Exception('%d files created, expected 1' % len(dex_files))
-      tmp_dex_output = dex_files[0]
+      # d8 supports outputting to a .zip, but does not have deterministic file
+      # ordering: https://issuetracker.google.com/issues/119945929
+      build_utils.ZipDir(tmp_dex_output, tmp_dex_dir)
 
     if options.dexlayout_profile:
-      tmp_dex_output = _PerformDexlayout(tmp_dir, tmp_dex_output, options)
+      if options.proguard_mapping_path is not None:
+        matching_profile = os.path.join(tmp_dir, 'obfuscated_profile')
+        convert_dex_profile.ObfuscateProfile(
+            options.dexlayout_profile, tmp_dex_output,
+            options.proguard_mapping_path, options.dexdump_path,
+            matching_profile)
+      else:
+        logging.warning('No obfuscation for %s', options.dexlayout_profile)
+        matching_profile = options.dexlayout_profile
+      binary_profile = _CreateBinaryProfile(matching_profile, tmp_dex_output,
+                                            options.profman_path, tmp_dir)
+      output_files = _LayoutDex(binary_profile, tmp_dex_output,
+                                options.dexlayout_path, tmp_dir)
+      target = None
+      if len(output_files) > 1:
+        target = _ZipMultidex(tmp_dir, output_files)
+      else:
+        output = output_files[0]
+        if not zipfile.is_zipfile(output):
+          target = os.path.join(tmp_dir, 'dex_classes.zip')
+          _ZipSingleDex(output, target)
+        else:
+          target = output
+      shutil.move(os.path.join(tmp_dir, target), tmp_dex_output)
 
     # The dex file is complete and can be moved out of tmp_dir.
-    shutil.move(tmp_dex_output, options.output)
-
-
-def main(args):
-  options = _ParseArgs(args)
-
-  input_paths = list(options.inputs)
-  if options.multi_dex and options.main_dex_list_path:
-    input_paths.append(options.main_dex_list_path)
-
-  _PerformDexing(options)
+    shutil.move(tmp_dex_output, options.dex_path)
 
   build_utils.WriteDepfile(
-      options.depfile, options.output, input_paths, add_pydeps=False)
+      options.depfile, options.dex_path, input_paths, add_pydeps=False)
 
 
 if __name__ == '__main__':

build/android/gyp/util/build_utils.py

@@ -34,6 +34,10 @@ DIR_SOURCE_ROOT = os.environ.get('CHECKOUT_SOURCE_ROOT',
     os.path.abspath(os.path.join(os.path.dirname(__file__),
                                  os.pardir, os.pardir, os.pardir, os.pardir)))
 
+HERMETIC_TIMESTAMP = (2001, 1, 1, 0, 0, 0)
+_HERMETIC_FILE_ATTR = (0o644 << 16)
+
+
 try:
   string_types = basestring
 except NameError:
@@ -310,14 +314,6 @@ def ExtractAll(zip_path, path=None, no_clobber=True, pattern=None,
   return extracted
 
 
-def HermeticZipInfo(*args, **kwargs):
-  """Creates a ZipInfo with a constant timestamp and external_attr."""
-  ret = zipfile.ZipInfo(*args, **kwargs)
-  ret.date_time = (2001, 1, 1, 0, 0, 0)
-  ret.external_attr = (0o644 << 16)
-  return ret
-
-
 def AddToZipHermetic(zip_file, zip_path, src_path=None, data=None,
                      compress=None):
   """Adds a file to the given ZipFile with a hard-coded modified time.
@@ -333,7 +329,8 @@ def AddToZipHermetic(zip_file, zip_path, src_path=None, data=None,
   assert (src_path is None) != (data is None), (
       '|src_path| and |data| are mutually exclusive.')
   _CheckZipPath(zip_path)
-  zipinfo = HermeticZipInfo(filename=zip_path)
+  zipinfo = zipfile.ZipInfo(filename=zip_path, date_time=HERMETIC_TIMESTAMP)
+  zipinfo.external_attr = _HERMETIC_FILE_ATTR
 
   if src_path and os.path.islink(src_path):
     zipinfo.filename = zip_path

build/config/android/internal_rules.gni

@@ -1295,11 +1295,13 @@ if (enable_java_templates) {
           invoker.output,
         ]
 
+        _rebased_output = rebase_path(invoker.output, root_build_dir)
+
         args = [
           "--depfile",
           rebase_path(depfile, root_build_dir),
-          "--output",
-          rebase_path(outputs[0], root_build_dir),
+          "--dex-path",
+          _rebased_output,
         ]
 
         if (_proguard_enabled) {
@@ -1320,7 +1322,7 @@ if (enable_java_templates) {
 
         if (defined(invoker.input_dex_classpath)) {
           inputs += [ invoker.build_config ]
-          args += [ "--input-list=@FileArg(${invoker.input_dex_classpath})" ]
+          args += [ "--inputs=@FileArg(${invoker.input_dex_classpath})" ]
         }
 
         inputs += _dexing_jars