Compressing .pak resources with new option: "type=GZIPPABLE_BINDATA"

tools/grit/grit/node/include.py

 #!/usr/bin/env python
 # Copyright (c) 2012 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.
 
 """Handling of the <include> element.
 """
 
+import gzip
 import os
+import StringIO
+import subprocess
+import sys
 
+from grit import util
 import grit.format.html_inline
+import grit.format.rc
 import grit.format.rc_header
-import grit.format.rc
-
 from grit.node import base
-from grit import util
 
 class IncludeNode(base.Node):
   """An <include> element."""
   def __init__(self):
     super(IncludeNode, self).__init__()
 
     # Cache flattened data so that we don't flatten the same file
     # multiple times.
     self._flattened_data = None
     # Also keep track of the last filename we flattened to, so we can
@@ skipping 56 matching lines @@
     from grit.format import rc_header
     id_map = rc_header.GetIds(self.GetRoot())
     id = id_map[self.GetTextualIds()[0]]
     if self.attrs['flattenhtml'] == 'true':
       allow_external_script = self.attrs['allowexternalscript'] == 'true'
       data = self._GetFlattenedData(allow_external_script=allow_external_script)
     else:
       filename = self.ToRealPath(self.GetInputPath())
       data = util.ReadFile(filename, util.BINARY)
 
+    if self.attrs['type'] == 'GZIPPABLE_BINDATA':
+      raw_data = data
+      try:

[agrieve, 2016/05/11 18:29:52]

nit: extract this into a private helper function. e.g.:

data = _GzipString(data)

[smaier, 2016/05/13 15:27:25]

Done.

+        # Make call to OS's gzip to get access to --rsyncable option

[agrieve, 2016/05/11 18:29:52]

nit: end with a period.

[smaier, 2016/05/13 15:27:25]

Done.

+        gzip_proc = subprocess.Popen(['gzip', '--stdout', '--rsyncable',
+                                      '--best', '--no-name'],
+                                     stdin=subprocess.PIPE,
+                                     stdout=subprocess.PIPE,
+                                     stderr=subprocess.PIPE)
+        data, stderr = gzip_proc.communicate(raw_data)
+        if gzip_proc.returncode != 0:
+          raise subprocess.CalledProcessError(gzip_proc.returncode, 'gzip',
+                                              stderr)
+      except Exception:
+        if sys.platform == 'linux2':
+          # Only expecting linux gzip calls to work: windows doesn't ship with
+          # gzip, and OSX's gzip does not have an --rsyncable option built in.
+          raise

[agrieve, 2016/05/11 18:29:52]

nit: might be a good idea to store whether this works in a variable to avoid
hitting this failure case over and over.

e.g.:

_has_gzip_rsyncable = None

def _GzipString(data):
  global _has_gzip_rsyncable
  ...
  if _has_gzip_rsyncable is not False:
     ..
     _has_gzip_rsyncable = True
     

[flackr, 2016/05/11 18:54:33]

I'm a little uncomfortable with maybe using the platform gzip program on non
linux platforms (if it exists and succeeds), can we add a configuration
parameter whether we expect to use the platform gzip or not?

I'm assuming we use the platform's gzip for the --rsyncable option, is that
correct? Is there a reason we only need rsyncable on linux? Maybe the
configuration paramater I mentioned should be whether we require rsyncable gzip?

[smaier, 2016/05/13 15:27:25]

I've decided to move away from the fallback entirely, and just apply two
separate gzip attempts for the two classes of platforms. We really only care
about --rsyncable with the release versions, which are built on Linux.

+        else:
+          # On all non-linux platforms, fall back to python's gzip.
+          gzip_output = StringIO.StringIO()
+          gzip.GzipFile('', 'wb', 9, gzip_output, 0).write(raw_data)

[agrieve, 2016/05/11 18:29:52]

nit: I think GzipFile probably wants to be closed. Can do this nicely via:

with gzip_file as GzipFile(...):
  gzip_file.write(raw_dat)

[smaier, 2016/05/13 15:27:25]

Done.

+          data = gzip_output.getvalue()
+          gzip_output.close()
+
     # Include does not care about the encoding, because it only returns binary
     # data.
     return id, data
 
   def Process(self, output_dir):
     """Rewrite file references to be base64 encoded data URLs.  The new file
     will be written to output_dir and the name of the new file is returned."""
     filename = self.ToRealPath(self.GetInputPath())
     flat_filename = os.path.join(output_dir,
         self.attrs['name'] + '_' + os.path.basename(filename))
@@ skipping 40 matching lines @@
     node.StartParsing('include', parent)
     node.HandleAttribute('name', name)
     node.HandleAttribute('type', type)
     node.HandleAttribute('file', file)
     node.HandleAttribute('translateable', translateable)
     node.HandleAttribute('filenameonly', filenameonly)
     node.HandleAttribute('mkoutput', mkoutput)
     node.HandleAttribute('relativepath', relativepath)
     node.EndParsing()
     return node