New build flag system, convert Google Now flag

build/write_build_header.py

+# Copyright 2015 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.
+
+# This writes headers for build flags. See build_header.gni for usage of
+# this system as a whole.
+#
+# The parameters are passed in a response file so we don't have to worry
+# about command line lengths. The name of the response file is passed on the
+# command line.
+#
+# The format of the response file is:
+#    [--flags <list of one or more flag values>]
+#    [--variables <list of one or more variable values]
+
+import optparse
+import shlex
+import sys
+
+
+class Options:
+  def __init__(self, output, rulename, header_guard, flags, variables):
+    self.output = output
+    self.rulename = rulename
+    self.header_guard = header_guard
+    self.flags = flags
+    self.variables = variables
+
+
+def GetOptions():
+  parser = optparse.OptionParser()
+  parser.add_option('--output', help="Output header name")
+  parser.add_option('--rulename',
+                    help="Helpful name of build rule for including in the " +
+                         "comment at the top of the file.")
+  parser.add_option('--path-for-guard',

[Mark Mentovai, 2015/11/19 21:52:55]

Might be nice if this accepted the base directory and the path within the base
directory as separate arguments, basing the guard on the latter, and doing a
path join to form the output path. It’s less redundancy in the arguments to the
script, which makes it harder to screw up usage.

[brettw, 2015/11/20 18:47:23]

Yes, this is nicer. Changed.

+                    help="Path to header for basing the include guard on.")
+  parser.add_option('--definitions',
+                    help="Name of the response file containing the flags and " +
+                         "variables.")
+  cmdline_options, cmdline_flags = parser.parse_args()
+
+  # Compute header guard by replacing some chars with _ and upper-casing.
+  header_guard = cmdline_options.path_for_guard.upper()
+  header_guard = \
+      header_guard.replace('/', '_').replace('\\', '_').replace('.', '_')

[Mark Mentovai, 2015/11/19 21:52:56]

Three replaces in a row, maybe better as an re.sub.

[brettw, 2015/11/20 18:47:23]

My brain doesn't like regular expressions and I find this a lot easier to read.
It should also be faster (not that speed matters here).

+  header_guard += '_'
+
+  # Definition file has two sections, --flags and --variables. In GYP these
+  # are newline separated. Otherwise these are shell formatted.
+  with open(cmdline_options.definitions, 'r') as def_file:
+    defs = shlex.split(def_file.read())
+  flags_index = defs.index('--flags')
+  vars_index = defs.index('--variables')
+
+  # Everything between flags and variables are flags. Values are 0/1/true/false.
+  flags = []
+  for flag in defs[flags_index + 1 : vars_index]:
+    equals_index = flag.index('=')
+    key = flag[:equals_index]
+    value = flag[equals_index + 1:]
+
+    # Canonicalize and validate the value.
+    if value == 'true':
+      value = 1

[Mark Mentovai, 2015/11/19 21:52:55]

Stick with '1' or use str(value) when you flags.append() to make sure
everything’s always a str, and not rely on every user of flags to have to treat
it as a str on the way out.

+    elif value == 'false':
+      value = 0
+    elif value != '0' and value != '1':
+      print "The flag %s should have a value or 0, 1, true, or false." % flag
+      print "Instead I see %s" % value
+      sys.exit(1)
+    flags.append((key, value))
+
+  # Everything after variables are variables.
+  variables = []
+  for var in defs[vars_index + 1:]:
+    equals_index = var.index('=')
+    variables.append((var[:equals_index], var[equals_index + 1:]))
+
+  return Options(output=cmdline_options.output,
+                 rulename=cmdline_options.rulename,
+                 header_guard=header_guard,
+                 flags=flags,
+                 variables=variables)
+
+
+def WriteHeader(options):
+  with open(options.output, 'w') as output_file:
+    output_file.write("/* Generated by build/write_build_header.py */\n")

[Mark Mentovai, 2015/11/19 21:52:56]

// comments like we usually use?

[brettw, 2015/11/20 00:02:37]

We do have C files floating around for various reasons I wanted to be able to
include these from those files, and since this was generated I didn't think it
would be weird since people won't normally be looking at them. However, the
build_header.h doesn't use C comments. I'll fix that to use C comments if it's
OK with you. If you feel strongly this should all be C++ only, I can change this
file instead.

[brettw, 2015/11/20 18:47:23]

I changed to //, we can switch to /* */ if necessary later.

+    if options.rulename:
+      output_file.write("/* From " + options.rulename + " */\n")

[Mark Mentovai, 2015/11/19 21:52:56]

I’m inclined to deal with jokers by scrubbing newlines and */s and other things
that might cause unpredictable results.

[brettw, 2015/11/20 18:47:23]

I think this is unnecessary. Invocations to this script are all done by my
wrappers, and it's quite hard to get things like newlines in here (it's actually
impossible to get newlines from GN in the way this is invoked). If you put a
weird thing in, you'll get the exact same thing out, which should be not
surprising and then you can fix your bug. Trying to clean up people's messes
here just encourages the mess in the first place.

+
+    output_file.write('\n#ifndef %s\n' % options.header_guard)
+    output_file.write('#define %s\n\n' % options.header_guard)
+    output_file.write('#include "build/build_header.h"\n\n')
+
+    for pair in options.flags:
+      output_file.write('#define BUILDFLAG_FLAG_VALUE_%s %s\n' % pair)
+    for pair in options.variables:
+      output_file.write('#define BUILDFLAG_VAR_VALUE_%s %s\n' % pair)

[Mark Mentovai, 2015/11/19 21:52:56]

Similarly, this should make sure there are no newlines, comment characters,
trailing backslashes…

+
+    output_file.write('\n#endif  /* %s */\n' % options.header_guard)
+
+
+options = GetOptions()
+WriteHeader(options)