Enable goma on pdfium buildbots.

scripts/slave/recipes/pdfium.py

Index: scripts/slave/recipes/pdfium.py
diff --git a/scripts/slave/recipes/pdfium.py b/scripts/slave/recipes/pdfium.py
index 6d7ee54a1db52c361e5de277aa1feba031dba1fa..351a1302d5c8f33649c63be833906ff4bf7e226a 100644
--- a/scripts/slave/recipes/pdfium.py
+++ b/scripts/slave/recipes/pdfium.py
@@ -5,6 +5,7 @@
 DEPS = [
   'depot_tools/gclient',
   'depot_tools/bot_update',
+  'goma',
   'recipe_engine/path',
   'recipe_engine/platform',
   'recipe_engine/properties',
@@ -24,6 +25,7 @@ PROPERTIES = {
   'rel': Property(default=False, kind=bool),
   'skip_test': Property(default=False, kind=bool),
   'target_os': Property(default=None, kind=str),
+  'goma': Property(default=True, kind=bool),

[Paweł Hajdan Jr., 2016/09/07 16:02:10]

Why is goma being added as build property?

If it's intended as temporary, please add a TODO.

[tikuta, 2016/09/13 09:30:11]

Removed

 }
 
 def _CheckoutSteps(api, memory_tool, skia, xfa, v8, target_cpu, clang,
@@ -38,7 +40,7 @@ def _CheckoutSteps(api, memory_tool, skia, xfa, v8, target_cpu, clang,
 
 
 def _GNGenBuilds(api, memory_tool, skia, xfa, v8, target_cpu, clang, rel,
-                 target_os, out_dir):
+                 target_os, goma, out_dir):
   gn_bool = {True: 'true', False: 'false'}
   # Generate build files by GN.
   checkout = api.path['checkout']
@@ -52,6 +54,7 @@ def _GNGenBuilds(api, memory_tool, skia, xfa, v8, target_cpu, clang, rel,
       'pdf_enable_xfa=%s' % gn_bool[xfa],
       'pdf_use_skia=%s' % gn_bool[skia],
       'pdf_is_standalone=true',
+      'use_goma=%s' % gn_bool[goma],
   ]
   if api.platform.is_win and not memory_tool:
     args.append('symbol_level=1')
@@ -71,10 +74,25 @@ def _GNGenBuilds(api, memory_tool, skia, xfa, v8, target_cpu, clang, rel,
               '--args=' + ' '.join(args)],
              cwd=checkout)
 
-def _BuildSteps(api, out_dir):
+def _BuildSteps(api, goma, clang, out_dir):
+  if goma:
+    api.goma.ensure_goma()
+    api.goma.start()
+
   # Build sample file using Ninja
   debug_path = api.path['checkout'].join('out', out_dir)
-  api.step('compile with ninja', ['ninja', '-C', debug_path])
+  ninja_cmd = ['ninja', '-C', debug_path]
+  if goma:
+    ninja_cmd.extend(['-j', api.goma.recommended_goma_jobs])
+  step_result = api.step('compile with ninja', ninja_cmd)
+
+  if goma:

[Paweł Hajdan Jr., 2016/09/07 16:02:10]

Instead of two "if goma" blocks, could you introduce a context manager? "with
goma.(pick-a-name):" that would ensure goma is stopped on exiting the block.

[Dirk Pranke, 2016/09/08 22:01:56]

Paweł, as far as I know we don't do this anywhere else (or, generally, worry
about stopping goma at all).

Am I wrong?

[Paweł Hajdan Jr., 2016/09/09 08:56:55]

I was mostly thinking about duplicated "if goma" and the interface than really
ensuring goma is stopped.

I'd be technically fine with this, it's more of a nit really. I'm more concerned
about adding a "goma" build property above.

[tikuta, 2016/09/13 09:30:11]

use build_with_goma context

+    api.goma.stop(
+        ninja_log_outdir=debug_path,
+        ninja_log_compiler= 'clang' if clang else 'unknown',
+        ninja_log_command=ninja_cmd,
+        ninja_log_exit_status=step_result.retcode,
+    )
 
 
 def _RunDrMemoryTests(api, v8):
@@ -145,15 +163,15 @@ def _RunTests(api, memory_tool, v8, out_dir):
 
 
 def RunSteps(api, memory_tool, skia, xfa, v8, target_cpu, clang, rel, skip_test,
-             target_os):
+             target_os, goma):
   _CheckoutSteps(api, memory_tool, skia, xfa, v8, target_cpu, clang, target_os)
 
   out_dir = 'Release' if rel else 'Debug'
 
   _GNGenBuilds(api, memory_tool, skia, xfa, v8, target_cpu, clang, rel,
-               target_os, out_dir)
+               target_os, goma, out_dir)
 
-  _BuildSteps(api, out_dir)
+  _BuildSteps(api, goma, clang, out_dir)
 
   if skip_test:
     return