Make first_ids_file an optional attribute of the .grd file.

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 38 matching lines @@
 
   -o OUTPUTDIR      Specify what directory output paths are relative to.
                     Defaults to the current directory.
 
   -D NAME[=VAL]     Specify a C-preprocessor-like define NAME with optional
                     value VAL (defaults to 1) which will be used to control
                     conditional inclusion of resources.
 
   -E NAME=VALUE     Set environment variable NAME to VALUE (within grit).
 
-  -f FIRSTIDFILE    Path to a python file that specifies the first id of
-                    value to use for resources.  Defaults to the file
-                    resources_ids next to grit.py.  Set to an empty string
-                    if you don't want to use a first id file.
+  -f FIRSTIDSFILE   Path to a python file that specifies the first id of
+                    value to use for resources.  A non-empty value here will
+                    override the value specified in the <grit> node's
+                    first_ids_file.
 
   -w WHITELISTFILE  Path to a file containing the string names of the
                     resources to include.  Anything not listed is dropped.
 
 
 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_id_filename = None
+    first_ids_file = None
     whitelist_filenames = []
     (own_opts, args) = getopt.getopt(args, 'o:D:E:f:w:')
     for (key, val) in own_opts:
       if key == '-o':
         self.output_directory = val
       elif key == '-D':
         name, val = ParseDefine(val)
         self.defines[name] = val
       elif key == '-E':
         (env_name, env_value) = val.split('=')
         os.environ[env_name] = env_value
       elif key == '-f':
-        first_id_filename = val
+        # TODO(joi@chromium.org): Remove this override once change
+        # lands in WebKit.grd to specify the first_ids_file in the
+        # .grd itself.

[tony, 2012/03/28 17:31:56]

I like the command line flag. In Chromium, if someone adds a new grd file, I
want it to warn that they need to pick an ID. If grit_action.gypi uses the -f
flag, new .grd files should get this warning automatically.  If we use
attributes, it's easier for someone to forget.

[Jói, 2012/03/29 11:36:15]

I'd prefer to have just one way to do things, but for now the flag stays and we
can talk before I make further changes.

My plan was to add a presubmit check to Chrome to verify that any .grd file
being modified has the attribute.  What would you say to that approach?  This is
perhaps even more foolproof than the flag, since it would catch somebody adding
a new .grd file that gets built by a customized invocation of grit rather than
using grit_action.gypi.

+        first_ids_file = val
       elif key == '-w':
         whitelist_filenames.append(val)
 
     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_file = open(whitelist_filename)
         self.whitelist_names |= set(whitelist_file.read().strip().split('\n'))
         whitelist_file.close()
 
-    self.res = grd_reader.Parse(opts.input, first_id_filename=first_id_filename,
-                                debug=opts.extra_verbose, defines=self.defines)
+    self.res = grd_reader.Parse(opts.input,
+                                debug=opts.extra_verbose,
+                                first_ids_file=first_ids_file,
+                                defines=self.defines)
     self.res.RunGatherers(recursive = True)
     self.Process()
     return 0
 
   def __init__(self):
     # Default file-creation function is built-in file().  Only done to allow
     # overriding by unit test.
     self.fo_create = file
 
     # key/value pairs of C-preprocessor like defines that are used for
@@ skipping 163 matching lines @@
 
     # Print out any fallback warnings, and missing translation errors, and
     # exit with an error code if there are missing translations in a non-pseudo
     # and non-official build.
     warnings = (self.res.UberClique().MissingTranslationsReport().
         encode('ascii', 'replace'))
     if warnings and self.defines.get('_google_chrome', False):
       print warnings
     if self.res.UberClique().HasMissingTranslations():
       sys.exit(-1)