Refactoring: separate build steps in real_main for compile.py

scripts/slave/compile.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.
 
 """A tool to build chrome, executed by buildbot.
 
   When this is run, the current directory (cwd) should be the outer build
   directory (e.g., chrome-release/build/).
 
@@ skipping 54 matching lines @@
       fh = sys.stdout
     fh.write('Environment variables modified in compile.py:\n')
     for k in sorted(list(self.overrides)):
       if k in self:
         fh.write('  %s=%s\n' % (k, self[k]))
       else:
         fh.write('  %s (removed)\n' % k)
     fh.write('\n')
 
 
+# TODO(tikuta): move to goma_utils.py
 def goma_setup(options, env):
   """Sets up goma if necessary.
 
   If using the Goma compiler, first call goma_ctl  to ensure the proxy is
   available, and returns (True, instance of cloudtail subprocess).
   If it failed to start up compiler_proxy, modify options.compiler and
   options.goma_dir and returns (False, None)
 
   """
   if options.compiler not in ('goma', 'goma-clang'):
@@ skipping 131 matching lines @@
   # Drop goma from options.compiler
   options.compiler = options.compiler.replace('goma-', '')
   if options.compiler == 'goma':
     options.compiler = None
   # Reset options.goma_dir.
   options.goma_dir = None
   env['GOMA_DISABLED'] = '1'
   return False, None
 
 
+# TODO(tikuta): move to goma_utils.py
 def goma_teardown(options, env, exit_status, cloudtail_proc):
   """Tears down goma if necessary. """
   if (options.compiler in ('goma', 'goma-clang') and
       options.goma_dir):
     override_gsutil = None
     if options.gsutil_py_path:
       override_gsutil = [sys.executable, options.gsutil_py_path]
 
     # If goma compiler_proxy crashes during the build, there could be crash
     # dump.
@@ skipping 102 matching lines @@
         if not entry:
           continue
         key, value = entry.split('=', 1)
         env_in_file[key] = value
     env_in_file.update(env_to_store)
     with open(path, 'wb') as f:
       f.write(nul.join(['%s=%s' % (k, v) for k, v in env_in_file.iteritems()]))
       f.write(nul * 2)
 
 
-def main_ninja(options, args):
-  """Interprets options, clobbers object files, and calls ninja."""
+def main_ninja(options, args, env):
+  """This function calls ninja.
 
-  # Prepare environment.
-  env = EchoDict(os.environ)
-  goma_ready, goma_cloudtail = goma_setup(options, env)
-  exit_status = -1
+  Args:
+      options (Option): options for ninja command.
+      args (str): extra args for ninja command.
+      env (dict): Used when ninja command executes.
+
+  Returns:
+      (int, str): first element for ninja command exit status.
+                  second element is actual command to run ninja.

[ukai, 2016/07/21 07:29:08]

only returns exit status?

[tikuta, 2016/07/22 08:11:32]

Done.

+
+  """
+
   try:
-    if not goma_ready:
-      assert options.compiler not in ('goma', 'goma-clang')
-      assert options.goma_dir is None
-
-    # ninja is different from all the other build systems in that it requires
-    # most configuration to be done at gyp time. This is why this function does
-    # less than the other comparable functions in this file.
     print 'chdir to %s' % options.src_dir
     os.chdir(options.src_dir)
 
     command = [options.ninja_path, '-w', 'dupbuild=err',
                                    '-C', options.target_output_dir]
 
     # HACK(yyanagisawa): update environment files on |env| update.
     # For compiling on Windows, environment in environment files are used.
     # It means even if enviroment such as GOMA_DISABLED is updated in
     # compile.py, the update will be ignored.
@@ skipping 47 matching lines @@
         if chromium_utils.IsMac() or chromium_utils.IsWindows():
           return min(10 * number_of_processors, 200)
 
         # For Linux, we also would like to use 10 * cpu. However, not sure
         # backend resource is enough, so let me set Linux and Linux x64 builder
         # only for now.
         hostname = goma_utils.GetShortHostname()
         if hostname in (
             ['build14-m1', 'build48-m1'] +
             # Also increasing cpus for v8/blink trybots.
-            ['build%d-m4' % x for x in xrange(45, 48)] +
+            ['build%d-m4' % x for x in range(45, 48)] +

[ukai, 2016/07/21 07:29:08]

why changed?

[tikuta, 2016/07/22 08:11:32]

fixed to use xrange

             # Also increasing cpus for LTO buildbots.
             ['slave%d-c1' % x for x in [20, 33] + range(78, 108)]):
           return min(10 * number_of_processors, 200)
 
         return 50
 
-      goma_jobs = determine_goma_jobs()
+      if not options.goma_jobs:

[ukai, 2016/07/21 07:29:08]

TODO: move determine_goma_jobs out of main_ninja
?

[tikuta, 2016/07/22 08:11:32]

Done.

+        goma_jobs = determine_goma_jobs()
+      else:
+        goma_jobs = options.goma_jobs
       command.append('-j%d' % goma_jobs)
 
     # Run the build.
     env.print_overrides()
     exit_status = chromium_utils.RunCommand(command, env=env)
     if exit_status == 0 and options.ninja_ensure_up_to_date:
       # Run the build again if we want to check that the no-op build is clean.
       filter_obj = EnsureUpToDateFilter()
       # Append `-d explain` to help diagnose in the failure case.
       command += ['-d', 'explain']
       chromium_utils.RunCommand(command, env=env, filter_obj=filter_obj)
       if not filter_obj.was_up_to_date:
         print 'Failing build because ninja reported work to do.'
         print 'This means that after completing a compile, another was run and'
         print 'it resulted in still having work to do (that is, a no-op build'
         print 'wasn\'t a no-op). Consult the first "ninja explain:" line for a'
         print 'likely culprit.'
         return 1
     return exit_status
   finally:
-    goma_teardown(options, env, exit_status, goma_cloudtail)
-
     override_gsutil = None
     if options.gsutil_py_path:
       override_gsutil = [sys.executable, options.gsutil_py_path]
 
     goma_utils.UploadNinjaLog(
         options.target_output_dir, options.compiler, command, exit_status,
         override_gsutil=override_gsutil)
 
 
 def get_target_build_dir(args, options):
   """Keep this function in sync with src/build/landmines.py"""
   if chromium_utils.IsLinux() and options.cros_board:
     # When building ChromeOS's Simple Chrome workflow, the output directory
     # has a CROS board name suffix.
     outdir = 'out_%s' % (options.cros_board,)
   elif options.out_dir:
     outdir = options.out_dir
   else:
     outdir = 'out'
   return os.path.abspath(os.path.join(options.src_dir, outdir, options.target))
 
 
-def real_main():
+def get_parsed_options():
   option_parser = optparse.OptionParser()
   option_parser.add_option('--clobber', action='store_true', default=False,
                            help='delete the output directory before compiling')
   option_parser.add_option('--target', default='Release',
                            help='build target (Debug or Release)')
   option_parser.add_option('--src-dir', default=None,
                            help='path to the root of the source tree')
   option_parser.add_option('--mode', default='dev',
                            help='build mode (dev or official) controlling '
                                 'environment variables set during build')
@@ skipping 28 matching lines @@
                            help='Enable goma CompilerInfo cache')
   option_parser.add_option('--goma-store-local-run-output', default=None,
                            help='Store local run output to goma servers.')
   option_parser.add_option('--goma-fail-fast', action='store_true')
   option_parser.add_option('--goma-disable-local-fallback', action='store_true')
   option_parser.add_option('--goma-jsonstatus',
                            help='Specify a file to dump goma_ctl jsonstatus.')
   option_parser.add_option('--goma-service-account-json-file',
                            help='Specify a file containing goma service account'
                                 ' credentials')
+  option_parser.add_option('--goma-jobs', default=None,
+                           help='The number of jobs for ninja -j.')
   option_parser.add_option('--gsutil-py-path',
                            help='Specify path to gsutil.py script.')
   option_parser.add_option('--ninja-path', default='ninja',
                            help='Specify path to the ninja tool.')
   option_parser.add_option('--ninja-ensure-up-to-date', action='store_true',
                            help='Checks the output of the ninja builder to '
                                 'confirm that a second compile immediately '
                                 'the first is a no-op.')
 
   options, args = option_parser.parse_args()
 
   if not options.src_dir:
     options.src_dir = 'src'
   options.src_dir = os.path.abspath(options.src_dir)
 
   options.target_output_dir = get_target_build_dir(args, options)
 
   assert options.build_tool in (None, 'ninja')
-  return main_ninja(options, args)
+  return options, args
+
+
+def real_main():
+  options, args = get_parsed_options()
+
+  # Prepare environment.
+  env = EchoDict(os.environ)
+
+  # start goma
+  goma_ready, goma_cloudtail = goma_setup(options, env)
+
+  if not goma_ready:
+    assert options.compiler not in ('goma', 'goma-clang')
+    assert options.goma_dir is None

[ukai, 2016/07/21 07:29:08]

better to fix options.goma_jobs based on goma_ready here?

[tikuta, 2016/07/22 08:11:32]

Done.

+
+  # build
+  exit_status = main_ninja(options, args, env)
+
+  # stop goma
+  goma_teardown(options, env, exit_status, goma_cloudtail)
+
+  return exit_status
 
 
 if '__main__' == __name__:
   sys.exit(real_main())