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

tools/grit/grit/node/include.py

Index: tools/grit/grit/node/include.py
diff --git a/tools/grit/grit/node/include.py b/tools/grit/grit/node/include.py
index ebcd53108f503005dadae9ae67b192245fb0a66d..caa7782cbba38ae8988e0122af829d4905a95261 100755
--- a/tools/grit/grit/node/include.py
+++ b/tools/grit/grit/node/include.py
@@ -6,14 +6,17 @@
 """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_header
 import grit.format.rc
-
+import grit.format.rc_header
 from grit.node import base
-from grit import util
 
 class IncludeNode(base.Node):
   """An <include> element."""
@@ -90,6 +93,31 @@ class IncludeNode(base.Node):
       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