Add option to not use compile.py in chromium recipe_module

scripts/slave/recipe_modules/chromium/api.py

Index: scripts/slave/recipe_modules/chromium/api.py
diff --git a/scripts/slave/recipe_modules/chromium/api.py b/scripts/slave/recipe_modules/chromium/api.py
index 777ca5ba21cea7ad8fdbb81d31c6368a55f2b2a7..d3a4c6600a8a9d7bdc88eab216388882f42df4b5 100644
--- a/scripts/slave/recipe_modules/chromium/api.py
+++ b/scripts/slave/recipe_modules/chromium/api.py
@@ -156,8 +156,10 @@ class ChromiumApi(recipe_api.RecipeApi):
 
     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))
@@ -260,7 +262,7 @@ class ChromiumApi(recipe_api.RecipeApi):
       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
@@ -278,32 +280,40 @@ class ChromiumApi(recipe_api.RecipeApi):
       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 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 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)
+          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,