Give build a --write-only-new option.

grit/tool/build.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.
 
 '''The 'grit build' tool along with integration for this tool with the
 SCons build system.
 '''
 
 import filecmp
@@ skipping 91 matching lines @@
   -h HEADERFORMAT   Custom format string to use for generating rc header files.
                     The string should have two placeholders: {textual_id}
                     and {numeric_id}. E.g. "#define {textual_id} {numeric_id}"
                     Otherwise it will use the default "#define SYMBOL 1234"
 
   --output-all-resource-defines
   --no-output-all-resource-defines  If specified, overrides the value of the
                     output_all_resource_defines attribute of the root <grit>
                     element of the input .grd file.
 
+  --write-only-new  Write output files to a temporary file first, and copy

[newt (away), 2014/12/04 22:09:42]

Does this also need to be added to grit_info.py as an unused argument so that
calls to grit_info.py will succeed? E.g. I believe chrome/policy_templates.gypi
calls grit_info.py with the same arguments that it calls grit.py.

[Nico, 2014/12/04 22:33:07]

I think policy_templates only passes the same defines, not the same flags.
buildinfo.py doesn't look at any of the other flags here either.

+                    it to the real output only if the new file is different
+                    from the old file.  This allows some build systems to
+                    realize that dependent build steps might be unnecessary,
+                    at the cost of comparing the output data at grit time.
+
 Conditional inclusion of resources only affects the output of files which
 control which resources get linked into a binary, e.g. it affects .rc files
 meant for compilation but it does not affect resource header files (that define
 IDs).  This helps ensure that values of IDs stay the same, that all messages
 are exported to translation interchange files (e.g. XMB files), etc.
 '''
 
   def ShortDescription(self):
     return 'A tool that builds RC files for compilation.'
 
   def Run(self, opts, args):
     self.output_directory = '.'
     first_ids_file = None
     whitelist_filenames = []
     assert_output_files = []
     target_platform = None
     depfile = None
     depdir = None
     rc_header_format = None
     output_all_resource_defines = None
+    write_only_new = False
     (own_opts, args) = getopt.getopt(args, 'a:o:D:E:f:w:t:h:',
         ('depdir=','depfile=','assert-file-list=',
          'output-all-resource-defines',
-         'no-output-all-resource-defines',))
+         'no-output-all-resource-defines',
+         'write-only-new'))
     for (key, val) in own_opts:
       if key == '-a':
         assert_output_files.append(val)
       elif key == '--assert-file-list':
         with open(val) as f:
           assert_output_files += f.read().splitlines()
       elif key == '-o':
         self.output_directory = val
       elif key == '-D':
         name, val = util.ParseDefine(val)
@@ skipping 13 matching lines @@
       elif key == '--no-output-all-resource-defines':
         output_all_resource_defines = False
       elif key == '-t':
         target_platform = val
       elif key == '-h':
         rc_header_format = val
       elif key == '--depdir':
         depdir = val
       elif key == '--depfile':
         depfile = val
+      elif key == '--write-only-new':
+        write_only_new = True
 
     if len(args):
       print 'This tool takes no tool-specific arguments.'
       return 2
     self.SetOptions(opts)
     if self.scons_targets:
       self.VerboseOut('Using SCons targets to identify files to output.\n')
     else:
       self.VerboseOut('Output directory: %s (absolute path: %s)\n' %
                       (self.output_directory,
                        os.path.abspath(self.output_directory)))
 
     if whitelist_filenames:
       self.whitelist_names = set()
       for whitelist_filename in whitelist_filenames:
         self.VerboseOut('Using whitelist: %s\n' % whitelist_filename);
         whitelist_contents = util.ReadFile(whitelist_filename, util.RAW_TEXT)
         self.whitelist_names.update(whitelist_contents.strip().split('\n'))
 
+    self.write_only_new = write_only_new
+
     self.res = grd_reader.Parse(opts.input,
                                 debug=opts.extra_verbose,
                                 first_ids_file=first_ids_file,
                                 defines=self.defines,
                                 target_platform=target_platform)
 
     # If the output_all_resource_defines option is specified, override the value
     # found in the grd file.
     if output_all_resource_defines is not None:
       self.res.SetShouldOutputAllResourceDefines(output_all_resource_defines)
@@ skipping 31 matching lines @@
 
     # Set to a list of filenames for the output nodes that are relative
     # to the current working directory.  They are in the same order as the
     # output nodes in the file.
     self.scons_targets = None
 
     # The set of names that are whitelisted to actually be included in the
     # output.
     self.whitelist_names = None
 
+    # Whether to compare outputs to their old contents before writing.
+    self.write_only_new = False
+
   @staticmethod
   def AddWhitelistTags(start_node, whitelist_names):
     # Walk the tree of nodes added attributes for the nodes that shouldn't
     # be written into the target files (skip markers).
     from grit.node import include
     from grit.node import message
     from grit.node import structure
     for node in start_node:
       # Same trick data_pack.py uses to see what nodes actually result in
       # real items.
@@ skipping 84 matching lines @@
 
       # Now copy from the temp file back to the real output, but on Windows,
       # only if the real output doesn't exist or the contents of the file
       # changed.  This prevents identical headers from being written and .cc
       # files from recompiling (which is painful on Windows).
       if not os.path.exists(output.GetOutputFilename()):
         os.rename(output.GetOutputFilename() + '.tmp',
                   output.GetOutputFilename())
       else:
         # CHROMIUM SPECIFIC CHANGE.
         # This clashes with gyp + vstudio, which expect the output timestamp

[newt (away), 2014/12/04 22:09:42]

If this is still an issue when using visual studio, are we going to pass
--write-only-new by default, or on non-windows platforms, or when using GN?
What's the plan? :)

[Nico, 2014/12/04 22:33:07]

We don't support visual studio anymore, but maybe it's still an issue for ios.

https://codereview.chromium.org/740463005 is the plan, it checks GENERATOR and
only enables it for ninja builds.

-        # to change on a rebuild, even if nothing has changed.
-        #files_match = filecmp.cmp(output.GetOutputFilename(),
-        #    output.GetOutputFilename() + '.tmp')
-        #if (output.GetType() != 'rc_header' or not files_match
-        #    or sys.platform != 'win32'):
-        shutil.copy2(output.GetOutputFilename() + '.tmp',
-                     output.GetOutputFilename())
+        # to change on a rebuild, even if nothing has changed, so only do
+        # it when opted in.
+        if not self.write_only_new:
+          write_file = True
+        else:
+          files_match = filecmp.cmp(output.GetOutputFilename(),
+              output.GetOutputFilename() + '.tmp')
+          write_file = not files_match
+        if write_file:
+          shutil.copy2(output.GetOutputFilename() + '.tmp',

[newt (away), 2014/12/04 22:09:42]

Seems like it would be quicker to delete output.h, then move the output.h.tmp to
output.h, rather than copying file contents around. Any idea why this uses a
copy and delete instead?

[Nico, 2014/12/04 22:33:07]

That's the usual way of doing things so that if you interrupt the command you
don't end up with a partially-written file. (The usual usual way is to do rm &&
mv instead of cp, that way it's not even slower.)

+                       output.GetOutputFilename())
         os.remove(output.GetOutputFilename() + '.tmp')
 
       self.VerboseOut(' done.\n')
 
     # Print warnings if there are any duplicate shortcuts.
     warnings = shortcuts.GenerateDuplicateShortcutsWarnings(
         self.res.UberClique(), self.res.GetTcProject())
     if warnings:
       print '\n'.join(warnings)
 
@@ skipping 84 matching lines @@
     self.MakeDirectoriesTo(depfile)
     outfile = self.fo_create(depfile, 'wb')
     outfile.writelines(depfile_contents)
 
   @staticmethod
   def MakeDirectoriesTo(file):
     '''Creates directories necessary to contain |file|.'''
     dir = os.path.split(file)[0]
     if not os.path.exists(dir):
       os.makedirs(dir)