Clean up logging in MB.

tools/mb/mb.py

 #!/usr/bin/env python
 # 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.
 
 """MB - the Meta-Build wrapper around GYP and GN
 
 MB is a wrapper script for GYP and GN that can be used to generate build files
 for sets of canned configurations and analyze them.
 """
@@ skipping 45 matching lines @@
                         help='configuration to analyze')
       subp.add_argument('-f', '--config-file', metavar='PATH',
                         default=self.default_config,
                         help='path to config file '
                             '(default is //tools/mb/mb_config.pyl)')
       subp.add_argument('-g', '--goma-dir', default=self.ExpandUser('~/goma'),
                         help='path to goma directory (default is %(default)s).')
       subp.add_argument('-n', '--dryrun', action='store_true',
                         help='Do a dry run (i.e., do nothing, just print '
                              'the commands that will run)')
-      subp.add_argument('-q', '--quiet', action='store_true',
-                        help='Do not print anything on success, '
-                             'just return an exit code.')
-      subp.add_argument('-v', '--verbose', action='count',
-                        help='verbose logging (may specify multiple times).')
+      subp.add_argument('-v', '--verbose', action='store_true',
+                        help='verbose logging')
 
     parser = argparse.ArgumentParser(prog='mb')
     subps = parser.add_subparsers()
 
     subp = subps.add_parser('analyze',
                             help='analyze whether changes to a set of files '
                                  'will cause a set of binaries to be rebuilt.')
     AddCommonOptions(subp)
     subp.add_argument('--swarming-targets-file',
                       help='save runtime dependencies for targets listed '
@@ skipping 23 matching lines @@
                                  'builder')
     AddCommonOptions(subp)
     subp.set_defaults(func=self.CmdLookup)
 
     subp = subps.add_parser('validate',
                             help='validate the config file')
     subp.add_argument('-f', '--config-file', metavar='PATH',
                       default=self.default_config,
                       help='path to config file '
                           '(default is //tools/mb/mb_config.pyl)')
-    subp.add_argument('-q', '--quiet', action='store_true',
-                      help='Do not print anything on success, '
-                           'just return an exit code.')
     subp.set_defaults(func=self.CmdValidate)
 
     subp = subps.add_parser('help',
                             help='Get help on a subcommand.')
     subp.add_argument(nargs='?', action='store', dest='subcommand',
                       help='The command to get help for.')
     subp.set_defaults(func=self.CmdHelp)
 
     self.args = parser.parse_args(argv)
 
@@ skipping 98 matching lines @@
 
     # Check that every mixin defined is actually referenced somewhere.
     for mixin in self.mixins:
       if not mixin in referenced_mixins:
         errs.append('Unreferenced mixin "%s".' % mixin)
 
     if errs:
       raise MBErr(('mb config file %s has problems:' % self.args.config_file) +
                     '\n  ' + '\n  '.join(errs))
 
-    if not self.args.quiet:
-      self.Print('mb config file %s looks ok.' % self.args.config_file)
+    self.Print('mb config file %s looks ok.' % self.args.config_file)
     return 0
 
   def GetConfig(self):
     self.ReadConfigFile()
     config = self.ConfigFromArgs()
     if not config in self.configs:
       raise MBErr('Config "%s" not found in %s' %
                   (config, self.args.config_file))
 
     return self.FlattenConfig(config)
@@ skipping 105 matching lines @@
 
       gn_runtime_deps_path = self.ToAbsPath(path, 'runtime_deps')
 
       # Since GN hasn't run yet, the build directory may not even exist.
       self.MaybeMakeDirectory(self.ToAbsPath(path))
 
       self.WriteFile(gn_runtime_deps_path, '\n'.join(gn_labels) + '\n')
       cmd.append('--runtime-deps-list-file=%s' % gn_runtime_deps_path)
 
     ret, _, _ = self.Run(cmd)
+    if ret:
+        # If `gn gen` failed, we should exit early rather than trying to
+        # generate isolates. Run() will have already logged any error output.
+        self.Print('GN gen failed: %d' % ret)
+        return ret
 
     for target in swarming_targets:
       if gn_isolate_map[target]['type'] == 'gpu_browser_test':
         runtime_deps_target = 'browser_tests'
       elif gn_isolate_map[target]['type'] == 'script':
         # For script targets, the build target is usually a group,
         # for which gn generates the runtime_deps next to the stamp file
         # for the label, which lives under the obj/ directory.
         label = gn_isolate_map[target]['label']
         runtime_deps_target = 'obj/%s.stamp' % label.replace(':', '/')
@@ skipping 29 matching lines @@
             self.ToSrcRelPath('%s%s%s.isolated' % (path, os.sep, target)),
             '--isolate',
             self.ToSrcRelPath('%s%s%s.isolate' % (path, os.sep, target)),
           ],
           'dir': self.chromium_src_dir,
           'version': 1,
         },
         isolate_path + 'd.gen.json',
       )
 
-
     return ret
 
   def GNCmd(self, subcommand, path, gn_args=''):
     if self.platform == 'linux2':
       gn_path = os.path.join(self.chromium_src_dir, 'buildtools', 'linux64',
                              'gn')
     elif self.platform == 'darwin':
       gn_path = os.path.join(self.chromium_src_dir, 'buildtools', 'mac',
                              'gn')
     else:
@@ skipping 44 matching lines @@
     ret, _, _ = self.Run(cmd)
     if not ret and self.args.verbose:
       outp = json.loads(self.ReadFile(self.args.output_path[0]))
       self.Print()
       self.Print('analyze output:')
       self.PrintJSON(outp)
       self.Print()
 
     return ret
 
-  def RunGNIsolate(self, vals):
-    build_path = self.args.path[0]
-    inp = self.ReadInputJSON(['targets'])
-    if self.args.verbose:
-      self.Print()
-      self.Print('isolate input:')
-      self.PrintJSON(inp)
-      self.Print()
-    output_path = self.args.output_path[0]
-
-    for target in inp['targets']:
-      runtime_deps_path = self.ToAbsPath(build_path, target + '.runtime_deps')
-
-      if not self.Exists(runtime_deps_path):
-        self.WriteFailureAndRaise('"%s" does not exist' % runtime_deps_path,
-                                  output_path)
-
-      command, extra_files = self.GetIsolateCommand(target, vals, None)
-
-      runtime_deps = self.ReadFile(runtime_deps_path).splitlines()
-
-
-      isolate_path = self.ToAbsPath(build_path, target + '.isolate')
-      self.WriteFile(isolate_path,
-        pprint.pformat({
-          'variables': {
-            'command': command,
-            'files': sorted(runtime_deps + extra_files),
-          }
-        }) + '\n')
-
-      self.WriteJSON(
-        {
-          'args': [
-            '--isolated',
-            self.ToSrcRelPath('%s/%s.isolated' % (build_path, target)),
-            '--isolate',
-            self.ToSrcRelPath('%s/%s.isolate' % (build_path, target)),
-          ],
-          'dir': self.chromium_src_dir,
-          'version': 1,
-        },
-        isolate_path + 'd.gen.json',
-      )
-
-    return 0
-
   def GetIsolateCommand(self, target, vals, gn_isolate_map):
     # This needs to mirror the settings in //build/config/ui.gni:
     # use_x11 = is_linux && !use_ozone.
     # TODO(dpranke): Figure out how to keep this in sync better.
     use_x11 = (sys.platform == 'linux2' and
                not 'target_os="android"' in vals['gn_args'] and
                not 'use_ozone=true' in vals['gn_args'])
 
     asan = 'is_asan=true' in vals['gn_args']
     msan = 'is_msan=true' in vals['gn_args']
@@ skipping 143 matching lines @@
 
     ret = 0
     response_file = self.TempFile()
     response_file.write('\n'.join(inp['files']) + '\n')
     response_file.close()
 
     matching_targets = []
     try:
       cmd = self.GNCmd('refs', self.args.path[0]) + [
           '@%s' % response_file.name, '--all', '--as=output']
-      ret, out, _ = self.Run(cmd)
+      ret, out, _ = self.Run(cmd, force_verbose=False)
       if ret and not 'The input matches no targets' in out:
         self.WriteFailureAndRaise('gn refs returned %d: %s' % (ret, out),
                                   output_path)
       build_dir = self.ToSrcRelPath(self.args.path[0]) + os.sep
       for output in out.splitlines():
         build_output = output.replace(build_dir, '')
         if build_output in inp['targets']:
           matching_targets.append(build_output)
 
       cmd = self.GNCmd('refs', self.args.path[0]) + [
           '@%s' % response_file.name, '--all']
-      ret, out, _ = self.Run(cmd)
+      ret, out, _ = self.Run(cmd, force_verbose=False)
       if ret and not 'The input matches no targets' in out:
         self.WriteFailureAndRaise('gn refs returned %d: %s' % (ret, out),
                                   output_path)
       for label in out.splitlines():
         build_target = label[2:]
         # We want to accept 'chrome/android:chrome_public_apk' and
         # just 'chrome_public_apk'. This may result in too many targets
         # getting built, but we can adjust that later if need be.
         for input_target in inp['targets']:
           if (input_target == build_target or
               build_target.endswith(':' + input_target)):
             matching_targets.append(input_target)
     finally:
       self.RemoveFile(response_file.name)
 
     if matching_targets:
       # TODO: it could be that a target X might depend on a target Y
       # and both would be listed in the input, but we would only need
       # to specify target X as a build_target (whereas both X and Y are
       # targets). I'm not sure if that optimization is generally worth it.
-      self.WriteJSON({'targets': sorted(matching_targets),
-                      'build_targets': sorted(matching_targets),
+      self.WriteJSON({'targets': sorted(set(matching_targets)),
+                      'build_targets': sorted(set(matching_targets)),
                       'status': 'Found dependency'}, output_path)
     else:
       self.WriteJSON({'targets': [],
                       'build_targets': [],
                       'status': 'No dependency'}, output_path)
 
-    if not ret and self.args.verbose:
+    if self.args.verbose:
       outp = json.loads(self.ReadFile(output_path))
       self.Print()
       self.Print('analyze output:')
       self.PrintJSON(outp)
       self.Print()
 
     return 0
 
   def ReadInputJSON(self, required_keys):
     path = self.args.input_path[0]
     output_path = self.args.output_path[0]
     if not self.Exists(path):
       self.WriteFailureAndRaise('"%s" does not exist' % path, output_path)
 
     try:
       inp = json.loads(self.ReadFile(path))
     except Exception as e:
       self.WriteFailureAndRaise('Failed to read JSON input from "%s": %s' %
                                 (path, e), output_path)
 
     for k in required_keys:
       if not k in inp:
         self.WriteFailureAndRaise('input file is missing a "%s" key' % k,
                                   output_path)
 
     return inp
 
   def WriteFailureAndRaise(self, msg, output_path):
     if output_path:
-      self.WriteJSON({'error': msg}, output_path)
+      self.WriteJSON({'error': msg}, output_path, force_verbose=True)
     raise MBErr(msg)
 
-  def WriteJSON(self, obj, path):
+  def WriteJSON(self, obj, path, force_verbose=False):
     try:
-      self.WriteFile(path, json.dumps(obj, indent=2, sort_keys=True) + '\n')
+      self.WriteFile(path, json.dumps(obj, indent=2, sort_keys=True) + '\n',
+                     force_verbose=force_verbose)
     except Exception as e:
       raise MBErr('Error %s writing to the output path "%s"' %
                  (e, path))
 
   def PrintCmd(self, cmd):
     if cmd[0] == sys.executable:
       cmd = ['python'] + cmd[1:]
     self.Print(*[pipes.quote(c) for c in cmd])
 
   def PrintJSON(self, obj):
     self.Print(json.dumps(obj, indent=2, sort_keys=True))
 
   def Print(self, *args, **kwargs):
     # This function largely exists so it can be overridden for testing.
     print(*args, **kwargs)
 
-  def Run(self, cmd, env=None):
+  def Run(self, cmd, env=None, force_verbose=True):
     # This function largely exists so it can be overridden for testing.
-    if self.args.dryrun or self.args.verbose:
+    if self.args.dryrun or self.args.verbose or force_verbose:
       self.PrintCmd(cmd)
     if self.args.dryrun:
       return 0, '', ''
+
     ret, out, err = self.Call(cmd, env=env)
-    if self.args.verbose:
+    if self.args.verbose or force_verbose:
       if out:
         self.Print(out, end='')
       if err:
         self.Print(err, end='', file=sys.stderr)
     return ret, out, err
 
   def Call(self, cmd, env=None):
     p = subprocess.Popen(cmd, shell=False, cwd=self.chromium_src_dir,
                          stdout=subprocess.PIPE, stderr=subprocess.PIPE,
                          env=env)
@@ skipping 21 matching lines @@
       return fp.read()
 
   def RemoveFile(self, path):
     # This function largely exists so it can be overriden for testing.
     os.remove(path)
 
   def TempFile(self, mode='w'):
     # This function largely exists so it can be overriden for testing.
     return tempfile.NamedTemporaryFile(mode=mode, delete=False)
 
-  def WriteFile(self, path, contents):
+  def WriteFile(self, path, contents, force_verbose=False):
     # This function largely exists so it can be overriden for testing.
-    if self.args.dryrun or self.args.verbose:
+    if self.args.dryrun or self.args.verbose or force_verbose:
       self.Print('\nWriting """\\\n%s""" to %s.\n' % (contents, path))
     with open(path, 'w') as fp:
       return fp.write(contents)
 
 
 class MBErr(Exception):
   pass
 
 
 if __name__ == '__main__':
   try:
     sys.exit(main(sys.argv[1:]))
   except MBErr as e:
     print(e)
     sys.exit(1)
   except KeyboardInterrupt:
     print("interrupted, exiting", stream=sys.stderr)
     sys.exit(130)