Use hermetic timestamps in apkbuilder.py and refactor helper function

build/android/gyp/util/build_utils.py

 # Copyright 2013 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.
 
 import ast
 import contextlib
 import fnmatch
 import json
 import os
 import pipes
 import re
 import shlex
 import shutil
 import subprocess
 import sys
 import tempfile
 import zipfile
 
 # Some clients do not add //build/android/gyp to PYTHONPATH.
 import md5_check  # pylint: disable=relative-import
 
 sys.path.append(os.path.join(os.path.dirname(__file__), os.pardir, os.pardir))
 from pylib import constants
 
 COLORAMA_ROOT = os.path.join(constants.DIR_SOURCE_ROOT,
                              'third_party', 'colorama', 'src')
 # aapt should ignore OWNERS files in addition the default ignore pattern.
 AAPT_IGNORE_PATTERN = ('!OWNERS:!.svn:!.git:!.ds_store:!*.scc:.*:<dir>_*:' +
                        '!CVS:!thumbs.db:!picasa.ini:!*~:!*.d.stamp')
-HERMETIC_TIMESTAMP = (2001, 1, 1, 0, 0, 0)
-HERMETIC_FILE_ATTR = (0644 << 16L)
+_HERMETIC_TIMESTAMP = (2001, 1, 1, 0, 0, 0)
+_HERMETIC_FILE_ATTR = (0644 << 16L)
 
 
 @contextlib.contextmanager
 def TempDir():
   dirname = tempfile.mkdtemp()
   try:
     yield dirname
   finally:
     shutil.rmtree(dirname)
 
@@ skipping 175 matching lines @@
       CheckZipPath(name)
       if no_clobber:
         output_path = os.path.join(path, name)
         if os.path.exists(output_path):
           raise Exception(
               'Path already exists from zip: %s %s %s'
               % (zip_path, name, output_path))
       z.extract(name, path)
 
 
-def CreateHermeticZipInfo(zip_path):
-  """Creates a ZipInfo with a zero'ed out timestamp."""
+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.
+
+  Args:
+    zip_file: ZipFile instance to add the file to.
+    zip_path: Destination path within the zip file.
+    src_path: Path of the source file. Mutually exclusive with |data|.
+    data: File data as a string.
+    compress: Whether to enable compression. Default is take from ZipFile
+        constructor.
+  """
+  assert (not src_path) != (not data), (

[jbudorick, 2015/11/17 03:27:17]

I was really confused by the original version of this assert.

Could this be

  assert bool(src_path) != bool(data)

?

[agrieve, 2015/11/17 03:43:01]

embarrassingly, didn't know about bool() :P

+      '|src_path| and |data| are mutually exclusive.')
   CheckZipPath(zip_path)
-  zipinfo = zipfile.ZipInfo(filename=zip_path, date_time=HERMETIC_TIMESTAMP)
-  zipinfo.external_attr = HERMETIC_FILE_ATTR
-  return zipinfo
+  zipinfo = zipfile.ZipInfo(filename=zip_path, date_time=_HERMETIC_TIMESTAMP)
+  zipinfo.external_attr = _HERMETIC_FILE_ATTR
+
+  if src_path:
+    with file(src_path) as f:
+      data = f.read()
+
+  # zipfile will deflate even when it makes the file bigger. To avoid
+  # growing files, disable compression at an arbitrary cut off point.
+  if len(data) < 16:
+    compress = False
+
+  # None converts to ZIP_STORED, when passed explicitly.
+  if compress is None:

[jbudorick, 2015/11/17 03:27:17]

why isn't this just

  zip_file.writestr(zipinfo, data, zipfile.ZIP_DEFLATED if compress else
zipfile.ZIP_STORED)

why is the compress is None case handled differently than the compress is False
case?

[agrieve, 2015/11/17 03:43:01]

Updated this logic to be a bit more clever and elaborated in the comment.

+    zip_file.writestr(zipinfo, data)
+  else:
+    compress_type = zipfile.ZIP_DEFLATED if compress else zipfile.ZIP_STORED
+    zip_file.writestr(zipinfo, data, compress_type)
 
 
 def DoZip(inputs, output, base_dir=None):
   """Creates a zip file from a list of files.
 
   Args:
     inputs: A list of paths to zip, or a list of (zip_path, fs_path) tuples.
     output: Destination .zip file.
     base_dir: Prefix to strip from inputs.
   """
   input_tuples = []
   for tup in inputs:
     if isinstance(tup, basestring):
       tup = (os.path.relpath(tup, base_dir), tup)
     input_tuples.append(tup)
 
   # Sort by zip path to ensure stable zip ordering.
   input_tuples.sort(key=lambda tup: tup[0])
   with zipfile.ZipFile(output, 'w') as outfile:
     for zip_path, fs_path in input_tuples:
-      with file(fs_path) as f:
-        contents = f.read()
-      outfile.writestr(CreateHermeticZipInfo(zip_path), contents)
+      AddToZipHermetic(outfile, zip_path, src_path=fs_path)
 
 
 def ZipDir(output, base_dir):
   """Creates a zip file from a directory."""
   inputs = []
   for root, _, files in os.walk(base_dir):
     for f in files:
       inputs.append(os.path.join(root, f))
   DoZip(inputs, output, base_dir)
 
@@ skipping 10 matching lines @@
   with zipfile.ZipFile(output, 'w') as out_zip:
     for in_file in inputs:
       with zipfile.ZipFile(in_file, 'r') as in_zip:
         for name in in_zip.namelist():
           # Ignore directories.
           if name[-1] == '/':
             continue
           dst_name = path_transform(name, in_file)
           already_added = dst_name in added_names
           if not already_added and not MatchesGlob(dst_name, exclude_patterns):
-            out_zip.writestr(CreateHermeticZipInfo(dst_name), in_zip.read(name))
+            AddToZipHermetic(out_zip, dst_name, data=in_zip.read(name))
             added_names.add(dst_name)
 
 
 def PrintWarning(message):
   print 'WARNING: ' + message
 
 
 def PrintBigWarning(message):
   print '*****     ' * 8
   PrintWarning(message)
@@ skipping 154 matching lines @@
 
   md5_check.CallAndRecordIfStale(
       on_stale_md5,
       record_path=record_path,
       input_paths=input_paths,
       input_strings=input_strings,
       output_paths=output_paths,
       force=force,
       pass_changes=True)