Only call CopyStringsFile if convert_to_binary is False.

pylib/gyp/mac_tool.py

 #!/usr/bin/env python
 # Copyright (c) 2012 Google Inc. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Utility functions to perform Xcode-style build steps.
 
 These functions are executed via gyp-mac-tool when using the Makefile generator.
 """
 
@@ skipping 31 matching lines @@
     method = "Exec%s" % self._CommandifyName(args[0])
     return getattr(self, method)(*args[1:])
 
   def _CommandifyName(self, name_string):
     """Transforms a tool name like copy-info-plist to CopyInfoPlist"""
     return name_string.title().replace('-', '')
 
   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.
       # TODO(thakis): This copies file attributes like mtime, while the
       # single-file branch below doesn't. This should probably be changed to
       # be consistent with the single-file branch.
       if os.path.exists(dest):
         shutil.rmtree(dest)
       shutil.copytree(source, dest)
     elif extension == '.xib':
       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):
     """Compiles a XIB file with ibtool into a binary plist in the bundle."""
 
     # ibtool sometimes crashes with relative paths. See crbug.com/314728.
     base = os.path.dirname(os.path.realpath(__file__))
     if os.path.relpath(source):
       source = os.path.join(base, source)
     if os.path.relpath(dest):
@@ skipping 619 matching lines @@
       out.write(struct.pack('<s', '\0'))
       base = os.path.dirname(path) + os.sep
       out.write(struct.pack('<%ds' % len(base), base))
       out.write(struct.pack('<s', '\0'))
       path = os.path.basename(path)
       out.write(struct.pack('<%ds' % len(path), path))
       out.write(struct.pack('<s', '\0'))
 
 if __name__ == '__main__':
   sys.exit(main(sys.argv[1:]))