Only call CopyStringsFile if convert_to_binary is False.

pylib/gyp/mac_tool.py

Index: pylib/gyp/mac_tool.py
diff --git a/pylib/gyp/mac_tool.py b/pylib/gyp/mac_tool.py
index e33c1d1273de072485ed36bece4ebc68d9025cf1..ea701192c837d6674e89efbac276deffd3378b92 100755
--- a/pylib/gyp/mac_tool.py
+++ b/pylib/gyp/mac_tool.py
@@ -49,6 +49,7 @@ class MacTool(object):
   def ExecCopyBundleResource(self, source, dest, convert_to_binary):
     """Copies a resource file to the bundle/Resources directory, performing any
     necessary compilation on each resource."""
+    convert_to_binary = convert_to_binary == 'True'
     extension = os.path.splitext(source)[1].lower()
     if os.path.isdir(source):
       # Copy tree.
@@ -62,14 +63,14 @@ class MacTool(object):
       return self._CopyXIBFile(source, dest)
     elif extension == '.storyboard':
       return self._CopyXIBFile(source, dest)
-    elif extension == '.strings':
+    elif extension == '.strings' and not convert_to_binary:
       self._CopyStringsFile(source, dest)
     else:
       if os.path.exists(dest):
         os.unlink(dest)
       shutil.copy(source, dest)

[sdefresne, 2016/07/05 16:52:14]

The file is copied here (i.e. in the else clause).

[justincohen, 2016/07/06 09:18:05]

Will this create a lot of churn? I wonder why we delete before copy here and not
on line 67?

On 2016/07/05 16:52:14, sdefresne wrote:

[sdefresne, 2016/07/06 09:28:06]

I don't know why we do not delete first for the other target. I do not think it
would create too much additional work as the majority of the files copied in
application bundle are not special and already fall in that last "else".

For ".strings" file, it will remove work, as the current code does read the file
as a plist, re-read the file as a plain data, convert from UTF-8 to UTF-8 and
save as dest, then invoke "plutil" to convert it to binary1 property list file.

After my change, it will remove the file if it exists, copy it using shutil.copy
without doing any modification and then use "plutil" to convert it to binary1
property list. This is mostly used for the InfoPlist.strings and
Localizable.strings file generated by generate_localizable_strings step in
Chromium build.

I can probably remove the copy and directly use "plutil" to do the copy (since
it supports an explicit output parameter), but this will be a bigger change to
this code.

The goal of this CL is to allow supporting .strings file already in binary1
format in which case the plutil call will be a noop (as plutil is smart enough
to avoid doing anything then). It is also required for compatibility with gn (as
we want to generate .strings in binary1 format directly and just not convert
them when copying to the application bundle).

 
-    if extension in ('.plist', '.strings') and convert_to_binary == 'True':
+    if convert_to_binary and extension in ('.plist', '.strings'):
       self._ConvertToBinary(dest)
 
   def _CopyXIBFile(self, source, dest):