[tools] Build generator script

tools/dev/v8gen.py

+#!/usr/bin/env python
+# Copyright 2016 the V8 project authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Script to generate V8's gn arguments based on common developer defaults
+or builder configurations.
+
+Examples:
+
+# Generate the x64.release config in out.gn/x64.release.
+v8gen.py x64.release
+
+# Generate into out.gn/foo.
+v8gen.py -b x64.release foo
+
+# Pass additional gn arguments after -- (don't use spaces within gn args).
+v8gen.py x64.optdebug -- use_goma=true
+
+# Print wrapped commands.
+v8gen.py x64.optdebug -v
+
+# Print output of wrapped commands.
+v8gen.py x64.optdebug -vv
+
+# Generate gn arguments of builder 'V8 Linux64 - builder' from master
+# 'client.v8' into out.gn/V8_Linux64___builder.
+v8gen.py -m client.v8 -b 'V8 Linux64 - builder'
+"""
+
+import argparse
+import os
+import re
+import subprocess
+import sys
+
+GOMA_DEFAULT = os.path.join(os.path.expanduser("~"), 'goma')
+OUT_DIR = 'out.gn'
+
+
+def _sanitize_nonalpha(text):
+  return ''.join(c if (c.isalnum() or c == '.') else '_' for c in text)
+
+
+class GenerateGnArgs(object):
+  def __init__(self, args):
+    # Split args into this script's arguments and gn args passed to the
+    # wrapped gn.
+    index = args.index('--') if '--' in args else len(args)
+    script_args = args[:index]
+    self.gn_args = args[index + 1:]

[tandrii(chromium), 2016/07/14 12:45:28]

here and also for self.options: since you are marking protected functions with
underscores, why not do the same with attributes as s/self.gn_args/self._gn_args

[Michael Achenbach, 2016/07/15 14:18:09]

Done.

+
+    parser = argparse.ArgumentParser(

[tandrii(chromium), 2016/07/14 12:45:28]

i'd put all code from here to end of __init__ into its own func
parse_arguments(), perhaps in module level, to make it easy to see what local
attrs are in GenerateGnArgs.

[Michael Achenbach, 2016/07/15 14:18:09]

Done.

+      description='Generator for V8\'s gn arguments. '
+                  'Additional gn args can be separated with -- '
+                  'from the arguments to this tool (don\'t use spaces or '
+                  'quote arguments like "use_goma = true").')
+    parser.add_argument(
+        'outdir', nargs='?',
+        help='optional gn output directory')
+    parser.add_argument(
+        '-b', '--builder',
+        help='build configuration or builder name from mb_config.pyl')
+    parser.add_argument(
+        '-m', '--master', default='developer_default',
+        help='config group or master from mb_config.pyl')
+    parser.add_argument(
+        '-p', '--pedantic', action='store_true',
+        help='run gn over command-line gn args to catch errors early')
+    parser.add_argument(
+        '-v', '--verbosity', action='count',
+        help='increase output verbosity (can be specified multiple times)')
+
+    goma = parser.add_mutually_exclusive_group()
+    goma.add_argument(
+        '-g' , '--goma',
+        action='store_true', default=None, dest='goma',
+        help='use goma')
+    goma.add_argument(
+        '--nogoma', '--no-goma',
+        action='store_false', default=None, dest='goma',
+        help='don\'t use goma')
+
+    self.options = parser.parse_args(script_args)
+
+    assert (self.options.outdir or self.options.builder), (

[tandrii(chromium), 2016/07/14 12:45:29]

if ...:
  parser.error(...)  # quits with ret code 2 here.

will result in much better UX

[Michael Achenbach, 2016/07/15 14:18:09]

Done.

+        'please specify either an output directory or a builder/config name, '
+        'e.g. x64.release')
+
+    if not self.options.outdir:
+      # Derive output directory from builder name.
+      self.options.outdir = _sanitize_nonalpha(self.options.builder)
+    else:
+      # Also, if this should work on windows, we might need to use \ where
+      # outdir is used as path, while using / if it's used in a gn context.
+      assert not self.options.outdir.startswith('/'), (
+        'only output directories relative to %s are supported' % OUT_DIR)

[tandrii(chromium), 2016/07/14 12:45:29]

ditto, use parser.error()

[Michael Achenbach, 2016/07/15 14:18:10]

Done.

+
+    if not self.options.builder:
+      # Derive builder from output directory.
+      self.options.builder = self.options.outdir
+
+  def verbose_print_1(self, text):
+    if self.options.verbosity >= 1:
+      print '#################################################################'

[tandrii(chromium), 2016/07/14 12:45:29]

ah, so verbose_print_1 means a section title?
then '#' * 80 is cooler

[Michael Achenbach, 2016/07/15 14:18:09]

Done.

+      print text
+
+  def verbose_print_2(self, text):
+    if self.options.verbosity >= 2:
+      print text

[tandrii(chromium), 2016/07/14 12:45:29]

after reading the whole class, I see that print-1 includes things like "cd xxx".
In which case, maybe don't use '#' * 80 above, but instead indent the text here
with:
  indent = ' ' * 4
  for l in text.splitlines():
    print indent + l

end result would be easier to read just verbose level=1 (-v) and still readable
"-v -v" mode.

[Michael Achenbach, 2016/07/15 14:18:09]

Done. But keeping the ### as it is even more readable.

+
+  def call_cmd(self, args):
+    self.verbose_print_1(' '.join(args))
+    try:
+      output = subprocess.check_output(
+        args=args,
+        stderr=subprocess.STDOUT,
+      )
+      self.verbose_print_2(output)
+    except subprocess.CalledProcessError as e:
+      self.verbose_print_2(e.output)
+      raise
+
+  def _find_work_dir(self, path):
+    """Find the closest v8 root to 'path'."""

[tandrii(chromium), 2016/07/14 12:45:29]

nit: I think `path` is proper way to mark vars, but just path is also OK.

[Michael Achenbach, 2016/07/15 14:18:09]

Done.

+    if os.path.exists(os.path.join(path, 'tools', 'dev', 'v8gen.py')):
+      # Approximate the v8 root dir by a folder where this script exists
+      # in the expected place.
+      return path
+    elif os.path.dirname(path) == path:
+      raise Exception(
+          'This appears to not be called from a recent v8 checkout')
+    else:
+      return self._find_work_dir(os.path.dirname(path))
+
+  @property
+  def _goma_dir(self):
+    return os.path.normpath(os.environ.get('GOMA_DIR') or GOMA_DEFAULT)
+
+  @property
+  def _need_goma_dir(self):
+    return self._goma_dir != GOMA_DEFAULT
+
+  @property
+  def _use_goma(self):
+    if self.options.goma is None:
+      # Auto-detect.
+      return os.path.exists(self._goma_dir) and os.path.isdir(self._goma_dir)
+    else:
+      return self.options.goma
+
+  @property
+  def _goma_args(self):
+    """Gn args for using goma."""
+    # Specify goma args if we want to use goma and if goma isn't specified
+    # via command line already. The command-line always has precedence over
+    # any other specification.
+    if (self._use_goma and
+        not [x for x in self.gn_args if re.match(r'use_goma\s*=.*', x)]):

[tandrii(chromium), 2016/07/14 12:45:29]

maybe

all(not re.match(r'use_goma\s*=.*', x) for x in self.gn_args)

or

not any(re.match(r'use_goma\s*=.*', x) for x in self.gn_args)

[Michael Achenbach, 2016/07/15 14:18:09]

Done.

+      if self._need_goma_dir:
+        return 'use_goma=true\ngoma_dir="%s"' % self._goma_dir
+      else:
+        return 'use_goma=true'
+    else:
+      return ''
+
+  def _append_gn_args(self, type, gn_args_path, more_gn_args):
+    """Append extra gn arguments to the generated args.gn file."""
+    if not more_gn_args:
+      return False
+    self.verbose_print_1("Appending \"\"\"\n%s\n\"\"\" to %s." % (

[tandrii(chromium), 2016/07/14 12:45:28]

use singular quotes and string would be easier to read.

[Michael Achenbach, 2016/07/15 14:18:09]

Weird, I'm using single quotes in the rest of the file, except here were it
would have mattered most :)

[tandrii(chromium), 2016/07/15 14:37:25]

yep, i wondered too :)

+        more_gn_args, os.path.abspath(gn_args_path)))
+    with open(gn_args_path, 'a') as f:
+      f.write('\n# Additional %s args:\n' % type)
+      f.write(more_gn_args)
+      f.write('\n')
+    return True
+
+  def main(self):
+    # Always operate relative to the base directory for better relative-path
+    # handling. This script can be used in any v8 checkout.
+    workdir = self._find_work_dir(os.getcwd())
+    if workdir != os.getcwd():
+      self.verbose_print_1('cd ' + workdir)
+      os.chdir(workdir)
+
+    # The directories are separated with slashes in a gn context (platform
+    # independent).
+    gn_outdir = '/'.join([OUT_DIR, self.options.outdir])
+
+    # Call MB to generate the basic configuration.
+    self.call_cmd([
+      sys.executable,
+      '-u', os.path.join('tools', 'mb', 'mb.py'),
+      'gen',
+      '-f', os.path.join('infra', 'mb', 'mb_config.pyl'),
+      '-m', self.options.master,
+      '-b', self.options.builder,
+      gn_outdir,
+    ])
+
+    # Handle extra gn arguments.
+    gn_args_path = os.path.join(OUT_DIR, self.options.outdir, 'args.gn')
+    more_gn_args = '\n'.join(self.gn_args)

[tandrii(chromium), 2016/07/15 14:37:25]

good, i didn't notice.

+
+    # Append command-line args.
+    modified = self._append_gn_args(
+        'command-line', gn_args_path, '\n'.join(self.gn_args))
+
+    # Append goma args.
+    # TODO(machenbach): We currently can't remove existing goma args from the
+    # original config. E.g. to build like a bot that uses goma, but switch
+    # goma off.
+    modified |= self._append_gn_args(
+        'goma', gn_args_path, self._goma_args)
+
+    # Regenerate ninja files to check for errors in the additional gn args.
+    if modified and self.options.pedantic:
+      self.call_cmd(['gn', 'gen', gn_outdir])
+    return 0
+
+if __name__ == "__main__":
+  gen = GenerateGnArgs(sys.argv[1:])
+  try:
+    sys.exit(gen.main())
+  except Exception:
+    if gen.options.verbosity < 2:
+      print ('\nHint: You can raise verbosity (-vv) to see the output of '
+             'failed commands.\n')
+    raise