Add option to not use compile.py in chromium recipe_module

scripts/slave/recipe_modules/chromium/api.py

 # Copyright 2013 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.
 
 import re
 
 from recipe_engine import recipe_api
 from recipe_engine import util as recipe_util
 
 class TestLauncherFilterFileInputPlaceholder(recipe_util.InputPlaceholder):
@@ skipping 138 matching lines @@
     # calling chromium.set_config(), above, because otherwise the chromium
     # call would reset the gclient config to its defaults.
     self.m.gclient.set_config(
         'chromium',
         PATCH_PROJECT=self.m.properties.get('patch_project'))
     for c in bot_config.get('gclient_apply_config', []):
       self.m.gclient.apply_config(c)
 
     return (buildername, bot_config)
 
+  # TODO(tikuta): Remove use_compile_py=True after removing compile.py.
   def compile(self, targets=None, name=None, out_dir=None,
-              target=None, use_goma_module=False, **kwargs):
+              target=None, use_goma_module=False,
+              use_compile_py=True, **kwargs):
     """Return a compile.py invocation."""
     targets = targets or self.c.compile_py.default_targets.as_jsonish()
     assert isinstance(targets, (list, tuple))
 
     if self.c.gyp_env.GYP_DEFINES.get('clang', 0) == 1:
       # Get the Clang revision before compiling.
       self._clang_version = self.get_clang_version()
 
     goma_env = self.get_env()
     goma_env.update(kwargs.get('env', {}))
@@ skipping 82 matching lines @@
       args += ['--cros-board', self.c.TARGET_CROS_BOARD]
 
     args.append('--')
     args.extend(targets)
 
     if self.c.TARGET_CROS_BOARD:
       # Wrap 'compile' through 'cros chrome-sdk'
       kwargs['wrapper'] = self.get_cros_chrome_sdk_wrapper()
       kwargs.setdefault('cwd', self.m.path['checkout'])
 
-    if use_goma_module:
+    if use_goma_module or not use_compile_py:
       # TODO(tikuta):
       # Make it possible to build with TARGET_CROS_BOARD using goma module.
       assert not self.c.TARGET_CROS_BOARD
 
       if out_dir is None:
         out_dir = 'out'
 
       target_output_dir = self.m.path.abspath(
           self.m.path.join(self.m.path['checkout'], out_dir,
                            target or self.c.build_config_fs))
 
       command = [str(self.m.depot_tools.ninja_path), '-w', 'dupbuild=err',
                  '-C', target_output_dir]
 
       if self.c.compile_py.build_args:
         command.extend(self.c.compile_py.build_args)
 
-      # Set -j just before 'with self.m.goma.build_with_goma('
-      # for ninja_log_command being set correctly if starting goma
-      # fails.
-      command += ['-j', self.m.goma.recommended_goma_jobs]
+      if use_goma_module:
+        # Set -j just before 'with self.m.goma.build_with_goma('
+        # for ninja_log_command being set correctly if starting goma
+        # fails.
+        command += ['-j', self.m.goma.recommended_goma_jobs]
 
       if targets is not None:
         # Add build targets to command ('All', 'chrome' etc).
         command += targets
 
+      kwargs.pop('env', {})
+
       # TODO(tikuta): Set disable_local_fallback option appropriately.
-      with self.m.goma.build_with_goma(
-          env=goma_env,
-          ninja_log_outdir=target_output_dir,
-          ninja_log_compiler=self.c.compile_py.compiler or 'goma',
-          ninja_log_command=command,
-          allow_build_without_goma=allow_build_without_goma):
-        if 'GOMA_DISABLED' in goma_env:
-          self.m.goma.remove_j_flag(command)
+      if use_goma_module:
+        with self.m.goma.build_with_goma(
+            env=goma_env,
+            ninja_log_outdir=target_output_dir,
+            ninja_log_compiler=self.c.compile_py.compiler or 'goma',
+            ninja_log_command=command,
+            allow_build_without_goma=allow_build_without_goma):
+          if 'GOMA_DISABLED' in goma_env:
+            self.m.goma.remove_j_flag(command)
 
-          if self.m.platform.is_win:
-            self.m.python('update windows env',
-                          script=self.package_repo_resource(
-                              'scripts', 'slave', 'update_windows_env.py'),
-                          args=['--envfile-dir', str(target_output_dir)],
-                          env=goma_env)
-        ninja_env.update(kwargs.pop('env', {}))

[shinyak, 2016/11/01 04:37:09]

Could you explain why we don't need to do `ninja_env.update(kwargs.pop('env',
{}))` in new code?

[tikuta, 2016/11/01 04:45:05]

I noticed that same thing is done in line 172 and 173.
So, let me remove this line.

[shinyak, 2016/11/01 04:55:22]

Oh, I see. Then, maybe it would be good to use kwargs.pop in L172 (right) and
remove L293 (right)? 

[tikuta, 2016/11/01 05:08:59]

kwargs['env'] is used in line 324.
I will use kwargs.pop in L172 after removing compile.py.

[shinyak (Google), 2016/11/01 05:20:06]

Hmm, sorry. if-block from L265 is too large ><

+            if self.m.platform.is_win:
+              self.m.python('update windows env',
+                            script=self.package_repo_resource(
+                                'scripts', 'slave', 'update_windows_env.py'),
+                            args=['--envfile-dir', str(target_output_dir)],
+                            env=goma_env)
+          self.m.step(name or 'compile with ninja',
+                      command,
+                      env=ninja_env,

[ukai, 2016/11/02 01:22:07]

ninja_env will have GOMA_DISABLED if failed to start goma?

GOMA_DISABLED should be passed to gomacc, so it must be in ninja_env?

maybe, L303 should check ninja_env, instead of goma_env?

[tikuta, 2016/11/02 04:51:30]

'GOMA_DISABLED' is set in goma_env passed to goma.build_with_goma(...).
So, I added 'GOMA_DISABLED' in ninja_env if 'GOMA_DISABLED' is set in goma_env.

+                      **kwargs)
+      else:
         self.m.step(name or 'compile with ninja',
                     command,
                     env=ninja_env,
                     **kwargs)
       return
 
     env = self.get_env()
     env.update(kwargs.pop('env', {}))
 
     try:
@@ skipping 587 matching lines @@
       }))
 
   def get_annotate_by_test_name(self, test_name):
     return 'graphing'
 
   def download_lto_plugin(self):
     return self.m.python(
         name='download LTO plugin',
         script=self.m.path['checkout'].join(
             'build', 'download_gold_plugin.py'))