Selective build clobbering feature (compile.py).

scripts/slave/compile.py

Index: scripts/slave/compile.py
diff --git a/scripts/slave/compile.py b/scripts/slave/compile.py
index e051e22cc6f48889bd956f47f8a487566db25c66..35f847c065c47ed1c60152f930cc5765e2bff17a 100755
--- a/scripts/slave/compile.py
+++ b/scripts/slave/compile.py
@@ -435,9 +435,8 @@ def main_xcode(options, args):
 
   # Note: this clobbers all targets, not just Debug or Release.
   if options.clobber:
-    build_output_dir = os.path.join(os.path.dirname(options.build_dir),
-        'xcodebuild')
-    print('Removing %s' % build_output_dir)
+    clobber_dir = os.path.dirname(options.target_build_dir)
+    print('Removing %s' % clobber_dir)
     # Deleting output_dir would also delete all the .ninja files. iOS builds
     # generates ninja configuration inside the xcodebuild directory to be able
     # to run sub builds. crbug.com/138950 is tracking this issue.
@@ -445,7 +444,7 @@ def main_xcode(options, args):
     # .ninja files). For now, only delete all non-.ninja files.
     # TODO(thakis): Make "clobber" a step that runs before "runhooks". Once the
     # master has been restarted, remove all clobber handling from compile.py.
-    ninja_clobber(build_output_dir)
+    ninja_clobber(clobber_dir)
 
   common_xcode_settings(command, options, env, options.compiler)
 
@@ -560,9 +559,8 @@ def common_make_settings(
       assert options.goma_dir
       env['CC'] = 'clang'
       env['CXX'] = 'clang++'
-      clang_dir = os.path.abspath(os.path.join(
-          slave_utils.SlaveBaseDir(options.build_dir), 'build', 'src',
-          'third_party', 'llvm-build', 'Release+Asserts', 'bin'))
+      clang_dir = os.path.abspath(os.path.join(options.src_dir,

[M-A Ruel, 2012/11/06 15:20:01]

I guess adding "options.src_dir = os.path.abspath(options.src_dir)" in main
would be a good idea.

[iannucci, 2012/11/13 20:09:51]

I believe this is already essentially implemented in main. I'll take out all the
unnecessary abspaths.

+        'third_party', 'llvm-build', 'Release+Asserts', 'bin'))
       env['PATH'] = ':'.join([options.goma_dir, clang_dir, env['PATH']])
     else:  # jsonclang
       env['CC'] = os.path.join(SLAVE_SCRIPTS_DIR, 'chromium', 'jsonclang')
@@ -570,9 +568,8 @@ def common_make_settings(
       command.append('-r')
       command.append('-k')
       # 'jsonclang' assumes the clang binary is in the path.
-      clang_dir = os.path.abspath(os.path.join(
-          slave_utils.SlaveBaseDir(options.build_dir), 'build', 'src',
-          'third_party', 'llvm-build', 'Release+Asserts', 'bin'))
+      clang_dir = os.path.abspath(os.path.join(options.src_dir,
+        'third_party', 'llvm-build', 'Release+Asserts', 'bin'))
       if options.goma_dir:
         env['PATH'] = ':'.join([options.goma_dir, clang_dir, env['PATH']])
       else:
@@ -599,8 +596,7 @@ def common_make_settings(
     return
 
   if compiler == 'clang':
-    clang_dir = os.path.abspath(os.path.join(
-        slave_utils.SlaveBaseDir(options.build_dir), 'build', 'src',
+    clang_dir = os.path.abspath(os.path.join(options.src_dir,
         'third_party', 'llvm-build', 'Release+Asserts', 'bin'))
     env['CC'] = os.path.join(clang_dir, 'clang')
     env['CXX'] = os.path.join(clang_dir, 'clang++')
@@ -612,8 +608,7 @@ def common_make_settings(
     # See
     # http://dev.chromium.org/developers/how-tos/using-valgrind/threadsanitizer/gcc-tsan
     # for build instructions.
-    tsan_base = os.path.abspath(os.path.join(
-        slave_utils.SlaveBaseDir(options.build_dir), 'build', 'src',
+    tsan_base = os.path.abspath(os.path.join(options.src_dir,
         'third_party', 'compiler-tsan'))
 
     tsan_gcc_bin = os.path.abspath(os.path.join(
@@ -638,8 +633,7 @@ def common_make_settings(
     env['GCCTSAN_GCC_DIR'] = gcctsan_gcc_dir
     env['GCCTSAN_GCC_VER'] = 'current'
     env['GCCTSAN_IGNORE'] = os.path.abspath(os.path.join(
-        slave_utils.SlaveBaseDir(options.build_dir),
-        'build', 'src', 'tools', 'valgrind', 'tsan', 'ignores.txt'))
+        options.src_dir, 'tools', 'valgrind', 'tsan', 'ignores.txt'))
     env['GCCTSAN_ARGS'] = (
         '-DADDRESS_SANITIZER -DWTF_USE_DYNAMIC_ANNOTATIONS=1 '
         '-DWTF_USE_DYNAMIC_ANNOTATIONS_NOIMPL=1' )
@@ -662,8 +656,6 @@ def main_make(options, args):
     assert options.goma_dir is None
 
   options.build_dir = os.path.abspath(options.build_dir)
-  src_dir = os.path.join(slave_utils.SlaveBaseDir(options.build_dir), 'build',
-                         'src')
   # TODO(mmoss) Temporary hack to ignore the Windows --solution flag that is
   # passed to all builders. This can be taken out once the master scripts are
   # updated to only pass platform-appropriate --solution values.
@@ -681,7 +673,7 @@ def main_make(options, args):
     if os.path.isfile(os.path.join(options.build_dir, 'Makefile')):
       working_dir = options.build_dir
     else:
-      working_dir = src_dir
+      working_dir = options.src_dir
 
   # Lots of test-execution scripts hard-code 'sconsbuild' as the output
   # directory.  Accomodate them.
@@ -713,22 +705,21 @@ def main_make(options, args):
   env.print_overrides()
   result = 0
 
-  def clobber(target):
-    build_output_dir = os.path.join(working_dir, 'out', target)
-    print('Removing %s' % build_output_dir)
-    chromium_utils.RemoveDirectory(build_output_dir)
+  def clobber():
+    print('Removing %s' % options.target_output_dir)
+    chromium_utils.RemoveDirectory(options.target_output_dir)
 
-  for target in options.target.split(','):
-    if options.clobber:
-      clobber(target)
+  assert ',' not in options.target, (
+   'Used to allow multiple comma-separated targets for make. This should not be'
+   ' in use any more. Asserting from orbit. It\'s the only way to be sure')
+
+  if options.clobber:
+    clobber()
 
-    target_command = command + ['BUILDTYPE=' + target]
-    this_result = chromium_utils.RunCommand(target_command, env=env)
-    if this_result and not options.clobber:
-      clobber(target)
-    # Keep the first non-zero return code as overall result.
-    if this_result and not result:
-      result = this_result
+  target_command = command + ['BUILDTYPE=' + options.target]
+  result = chromium_utils.RunCommand(target_command, env=env)
+  if result and not options.clobber:
+    clobber()
 
   goma_teardown(options, env)
 
@@ -749,24 +740,19 @@ def main_ninja(options, args):
   # 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.
-  src_dir = os.path.join(
-      slave_utils.SlaveBaseDir(os.path.abspath(options.build_dir)),
-      'build',
-      'src')
-  print 'chdir to %s' % src_dir
-  os.chdir(src_dir)
+  print 'chdir to %s' % options.src_dir
+  os.chdir(options.src_dir)
 
-  output_dir = os.path.join('out', options.target)
-  command = ['ninja', '-C', output_dir]
+  command = ['ninja', '-C', options.target_output_dir]
 
   if options.clobber:
-    print('Removing %s' % output_dir)
+    print('Removing %s' % options.target_output_dir)
     # Deleting output_dir would also delete all the .ninja files necessary to
     # build. Clobbering should run before runhooks (which creates .ninja files).
     # For now, only delete all non-.ninja files. TODO(thakis): Make "clobber" a
     # step that runs before "runhooks". Once the master has been restarted,
     # remove all clobber handling from compile.py.
-    ninja_clobber(output_dir)
+    ninja_clobber(options.target_output_dir)
 
   if options.verbose:
     command.append('-v')
@@ -799,8 +785,9 @@ def main_ninja(options, args):
       # Note that, on other platform, ninja doesn't use ninja -t msvc
       # (it just simply run $cc/$cxx), so modifying PATH can work to run
       # gomacc without this hack.
-      orig_build = open(os.path.join(output_dir, 'build.ninja'))
-      new_build = open(os.path.join(output_dir, 'build.ninja.goma'), 'w')
+      manifest = os.path.join(options.target_output_dir, 'build.ninja')
+      orig_build = open(manifest)
+      new_build = open(manifest+'.goma', 'w')

[M-A Ruel, 2012/11/06 15:20:01]

spacing

[iannucci, 2012/11/13 20:09:51]

Done.

       for line in orig_build:
         if line.startswith('cc '):
           new_build.write('cc = %s cl\n' % (
@@ -859,10 +846,8 @@ def main_scons(options, args):
   """
   options.build_dir = os.path.abspath(options.build_dir)
   if options.clobber:
-    build_output_dir = os.path.join(os.path.dirname(options.build_dir),
-                                    'sconsbuild', options.target)
-    print('Removing %s' % build_output_dir)
-    chromium_utils.RemoveDirectory(build_output_dir)
+    print('Removing %s' % options.target_output_dir)
+    chromium_utils.RemoveDirectory(options.target_output_dir)
 
   os.chdir(options.build_dir)
 
@@ -951,22 +936,21 @@ def main_win(options, args):
       tool_options.extend(['/Project', options.project])
 
   options.build_dir = os.path.abspath(options.build_dir)
-  build_output_dir = os.path.join(options.build_dir, options.target)
 
   def clobber():
-    print('Removing %s' % build_output_dir)
-    chromium_utils.RemoveDirectory(build_output_dir)
+    print('Removing %s' % options.target_output_dir)
+    chromium_utils.RemoveDirectory(options.target_output_dir)
 
   if options.clobber:
     clobber()
   else:
     # Remove the log file so it doesn't grow without limit,
-    chromium_utils.RemoveFile(build_output_dir, 'debug.log')
+    chromium_utils.RemoveFile(options.target_output_dir, 'debug.log')
     # Remove the chrome.dll version resource so it picks up the new svn
     # revision, unless user explicitly asked not to remove it. See
     # Bug 1064677 for more details.
     if not options.keep_version_file:
-      chromium_utils.RemoveFile(build_output_dir, 'obj', 'chrome_dll',
+      chromium_utils.RemoveFile(options.target_output_dir, 'obj', 'chrome_dll',
                                 'chrome_dll_version.rc')
 
   env = EchoDict(os.environ)
@@ -1067,10 +1051,10 @@ def main_win(options, args):
   if errors:
     print('\n\nRetrying a clobber build because of:')
     print('\n'.join(('  ' + l for l in errors)))
-    print('Removing %s' % build_output_dir)
+    print('Removing %s' % options.target_output_dir)
     for _ in range(3):
       try:
-        chromium_utils.RemoveDirectory(build_output_dir)
+        chromium_utils.RemoveDirectory(options.target_output_dir)
         break
       except OSError, e:
         print(e)
@@ -1088,6 +1072,32 @@ def main_win(options, args):
 
   return result
 

[M-A Ruel, 2012/11/06 15:20:01]

2 lines between file-level symbols

[iannucci, 2012/11/13 20:09:51]

Done.

+def landmines_triggered(build_dir):
+  trigger_file = os.path.join(build_dir, '.landmines_triggered')
+  if os.path.exists(trigger_file):
+    print "Setting clobber due to triggered landmines:"

[M-A Ruel, 2012/11/06 15:20:01]

The file use single-quotes

[iannucci, 2012/11/13 20:09:51]

Done.

+    with open(trigger_file) as f:
+      sys.stdout.writelines(f.readlines())

[M-A Ruel, 2012/11/06 15:20:01]

You are doing that because you want to strip CR, right?

[iannucci, 2012/11/13 20:09:51]

Doing this to print the file as-is (and it's more efficient than
write(f.read())). It will only ever be generated on the same system, so the line
endings should always match.

+    return True
+  return False
+
+def get_target_build_dir(build_tool, src_dir, target, is_iphone=False):

[M-A Ruel, 2012/11/06 15:20:01]

"""Keep in sync with foo_bar.py"""

[iannucci, 2012/11/13 20:09:51]

Done. I also updated the other function to refer to this file.

+  ret = None
+  if build_tool == 'xcode':
+    ret = os.path.join(src_dir, 'xcodebuild',
+        target + ('-iphoneos' if is_iphone else ''))
+  elif build_tool == 'make':
+    ret = os.path.join(src_dir, 'out', target)
+  elif build_tool == 'ninja':
+    ret = os.path.join(src_dir, 'out', target)
+  elif build_tool == 'msvs':
+    ret = os.path.join(src_dir, 'build', target)
+  elif build_tool == 'scons':
+    ret = os.path.join(src_dir, 'sconsbuild', target)
+  else:
+    raise NotImplementedError()
+  return os.path.abspath(ret)
+
 
 def real_main():
   option_parser = optparse.OptionParser()
@@ -1147,10 +1157,13 @@ def real_main():
   if options.build_tool is None:
     if chromium_utils.IsWindows():
       main = main_win
+      options.build_tool = 'msvs'
     elif chromium_utils.IsMac():
       main = main_xcode
+      options.build_tool = 'xcode'
     elif chromium_utils.IsLinux():
       main = main_make
+      options.build_tool = 'make'
     else:
       print('Please specify --build-tool.')
       return 1
@@ -1168,6 +1181,13 @@ def real_main():
       sys.stderr.write('Unknown build tool %s.\n' % repr(options.build_tool))
       return 2
 

[M-A Ruel, 2012/11/06 15:20:01]

options.build_dir = os.path.abspath(options.build_dir) ?

[iannucci, 2012/11/13 20:09:51]

Sure, why not! :D

Would be useful for optparse to have a 'path' type which did this
automatically...

+  options.src_dir = os.path.join(slave_utils.SlaveBaseDir(
+        os.path.abspath(options.build_dir)), 'build', 'src')
+  options.target_output_dir = get_target_build_dir(options.build_tool,

[M-A Ruel, 2012/11/06 15:20:01]

This alignment hurts. Please everything at +4 or at (.

[iannucci, 2012/11/13 20:09:51]

Done.

+      options.src_dir, options.target, 'iphoneos' in args)
+  options.clobber = (options.clobber or
+                     landmines_triggered(options.target_output_dir))
+
   return main(options, args)