Bot update!

scripts/slave/bot_update.py

Index: scripts/slave/bot_update.py
diff --git a/scripts/slave/bot_update.py b/scripts/slave/bot_update.py
index 2e16e4cd7c7366ff710d7bdea1866cd7e8eaee5e..bc0e84624ab1c6fa2b6403e4ac70ddcc5d3ccb90 100644
--- a/scripts/slave/bot_update.py
+++ b/scripts/slave/bot_update.py
@@ -3,13 +3,20 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-
+import codecs
 import copy
 import optparse
 import os
+import pprint
+import shutil
+import socket
+import subprocess
 import sys
+import time
 import urlparse
 
+import os.path as path
+
 
 RECOGNIZED_PATHS = {
     # If SVN path matches key, the entire URL is rewritten to the Git url.
@@ -51,10 +58,14 @@ This step does nothing. You actually want to look at the "update" step.
 
 """
 
+
+GCLIENT_TEMPLATE = """solutions = %(solutions)s
+
+cache_dir = %(cache_dir)s
+"""
+
 ENABLED_MASTERS = ['chromium.git']
-# Master: Builders dict.
 ENABLED_BUILDERS = {}
-# Master: Slaves dict.
 ENABLED_SLAVES = {}
 
 # Disabled filters get run AFTER enabled filters, so for example if a builder
@@ -63,6 +74,61 @@ ENABLED_SLAVES = {}
 DISABLED_BUILDERS = {}
 DISABLED_SLAVES = {}
 
+# How many times to retry failed subprocess calls.
+RETRIES = 3
+
+SCRIPTS_PATH = path.dirname(path.dirname(path.abspath(__file__)))
+DEPS2GIT_DIR_PATH = path.join(SCRIPTS_PATH, 'tools', 'deps2git')
+DEPS2GIT_PATH = path.join(DEPS2GIT_DIR_PATH, 'deps2git.py')
+S2G_INTERNAL_FROM_PATH = path.join(SCRIPTS_PATH, 'tools', 'deps2git_internal',
+                                   'svn_to_git_internal.py')
+S2G_INTERNAL_DEST_PATH = path.join(DEPS2GIT_DIR_PATH, 'svn_to_git_internal.py')
+
+# ../../cache_dir aka /b/build/slave/cache_dir
+THIS_DIR = path.abspath(os.getcwd())
+BUILDER_DIR = path.dirname(THIS_DIR)
+SLAVE_DIR = path.dirname(BUILDER_DIR)
+CACHE_DIR = path.join(SLAVE_DIR, 'cache_dir')
+
+
+class SubprocessFailed(Exception):
+  pass
+
+
+def call(*args):
+  """Interactive subprocess call."""
+  for attempt in xrange(RETRIES):

[pgervais, 2014/02/12 17:46:26]

Passing RETRIES as a global variable could be avoided by changing the function
signature like this:

def call(cmd, retries=3)

This is more consistent with the Popen signature and cleaner (*args arguments
tend to be hard to document).

[Ryan Tseng, 2014/02/12 18:56:53]

We just overloaded **kwargs in this function to pass directly to
subprocess.Popen() in another CL, so that won't work as well :(

The *arg construct was intentional (despite going against pythonic conventions)
because our calls are generally short and call('a', 'b', 'c', 'd') looks more
readable than call(['a', 'b', 'c', 'd']) by a tiny margin.

[pgervais, 2014/02/12 19:17:22]

There are at least two solutions to that problem:
- def call(*args, retries=3, **kwargs):
OR
- def call(*args, **kwargs): and use kwargs.pop('retries') to remove the extra
key prior to calling Popen. 

Plus Popen doesn't have that many keyword parameters, and they're not likely to
change. Listing all of them in the function definition is thus possible.
I have often noticed that this tiny margin is not worth it, but it's my personal
experience. You inevitably end up writing things like

cmd = ['call', 'me']
print 'command: %s ' % ' '.join(cmd)
call(*cmd)

Thus you're forced to add an extra *, which I don't like (but again, this is me)

[agable, 2014/02/12 19:59:11]

FWIW, the construct being used here (pass all **kwargs straight through to
subprocess.Popen) is the same system used throughout recipes. We're definitely
not ever going to list all the arguments to Popen (there are 13 of them).

We do use kwargs.pop() in a bunch of places, so we could/should probably use
that here too. I don't feel strongly either way.

As for call(1, 2, 3) vs call([1, 2, 3])... recipes uses both. I think we ended
up standardizing on the latter (since it was effectively necessary for
api.python) but again i don't feel strongly either way.

+    attempt_msg = '(retry #%d)' % attempt if attempt else ''
+    print '===Running %s%s===' % (' '.join(args), attempt_msg)
+    start_time = time.time()
+    proc = subprocess.Popen(args, stdout=subprocess.PIPE,
+                            stderr=subprocess.STDOUT)
+    # This is here because passing 'sys.stdout' into stdout for proc will
+    # produce out of order output.
+    while True:
+      buf = proc.stdout.read(1)
+      if not buf:
+        break
+      sys.stdout.write(buf)

[pgervais, 2014/02/12 17:46:26]

Isn't that horribly inefficient? I trust that you checked that stdout=sys.stdout
and stderr=subprocess.STDOUT does produce ooo output, but I'm still surprised.

[Ryan Tseng, 2014/02/12 18:56:53]

You'd think so, but its a pretty simple loop and our machines have at least 8
cores nominally, so this is pretty negligible, despite how inefficient it looks.

Also its the only thing that works all the time, since stdout=sys.stdout will
produce pretty undesireable results, where the "print" statements in main() gets
interleaved/printed after the subprocess because the subprocess outputs to
sys.stdout unbuffered but the python process itself buffers it outputs whenever
it feels like it.

We already do this in many places in various slave scripts because of the very
issue.  In fact it generally looks even more complicated, with threads and
stuff.

Basically pipes are weird.

+    code = proc.wait()
+    elapsed_time = ((time.time() - start_time) / 60.0)
+    if not code:
+      print '===Succeeded in %.1f mins===' % elapsed_time

[pgervais, 2014/02/12 17:46:26]

nit: units have no plural. Use 'min' instead of 'mins'

[Ryan Tseng, 2014/02/12 18:56:53]

I intentionally said "mins" because everything except 1.0min should be plural
(0.0 mins, 0.5 mins, 2.0mins), so it would be right most of the time.

[pgervais, 2014/02/12 19:17:22]

My point was that you should write '2 min' not '2 mins', because 'min' is a
unit, not an abbreviation. It's the same with other units: 2 m, 2 mph.

[agable, 2014/02/12 19:59:11]

+1. Any of 'min' or 'minutes' or 'minute(s)' are correct, 'mins' isn't really.

+      print
+      return 0
+    print '===Failed in %.1f mins===' % elapsed_time
+    print

[pgervais, 2014/02/12 17:46:26]

You could have added \n in the previous line.

[Ryan Tseng, 2014/02/12 18:56:53]

Noted, will fix in another CL

+
+  raise SubprocessFailed('%s failed with code %d in %s after %d attempts.' %
+                         (' '.join(args), code, os.getcwd(), RETRIES))
+
+
+def get_gclient_spec(solutions):
+  return GCLIENT_TEMPLATE % {
+      'solutions': pprint.pformat(solutions, indent=4),
+      'cache_dir': '"%s"' % CACHE_DIR
+  }
+
 
 def check_enabled(master, builder, slave):
   if master in ENABLED_MASTERS:
@@ -88,7 +154,8 @@ def check_disabled(master, builder, slave):
 
 
 def check_valid_host(master, builder, slave):
-  return False
+  return (check_enabled(master, builder, slave)
+          and not check_disabled(master, builder, slave))
 
 
 def solutions_printer(solutions):
@@ -99,6 +166,10 @@ def solutions_printer(solutions):
     name = solution.get('name')
     url = solution.get('url')
     print '%s (%s)' % (name, url)
+    if solution.get('deps_file'):
+      print '  Dependencies file is %s' % solution['deps_file']
+    if 'managed' in solution:
+      print '  Managed mode is %s' % ('ON' if solution['managed'] else 'OFF')
     custom_vars = solution.get('custom_vars')
     if custom_vars:
       print '  Custom Variables:'
@@ -112,10 +183,6 @@ def solutions_printer(solutions):
           print '    %s -> %s' % (deps_name, deps_value)
         else:
           print '    %s: Ignore' % deps_name
-    if solution.get('deps_file'):
-      print '  Dependencies file is %s' % solution['deps_file']
-    if 'managed' in solution:
-      print '  Managed mode is %s' % ('ON' if solution['managed'] else 'OFF')
     print
 
 
@@ -125,76 +192,149 @@ def solutions_to_git(input_solutions):
   for solution in solutions:
     original_url = solution['url']
     parsed_url = urlparse.urlparse(original_url)
-    path = parsed_url.path
-    if path in RECOGNIZED_PATHS:
-      solution['url'] = RECOGNIZED_PATHS[path]
+    parsed_path = parsed_url.path
+    if parsed_path in RECOGNIZED_PATHS:
+      solution['url'] = RECOGNIZED_PATHS[parsed_path]
     else:
-      print 'Warning: path %s not recognized' % path
+      print 'Warning: path %s not recognized' % parsed_path
     if solution.get('deps_file', 'DEPS') == 'DEPS':
       solution['deps_file'] = '.DEPS.git'
     solution['managed'] = False
   return solutions
 
 
-def ensure_no_git_checkout():
+def ensure_no_checkout(dir_names, scm_dirname):
   """Ensure that there is no git checkout under build/.
 
-  If there is a git checkout under build/, then move build/ to build.dead/
+  If there is an incorrect checkout under build/, then
+  move build/ to build.dead/
+  This function will check each directory in dir_names.
+
+  scm_dirname is expected to be either ['.svn', '.git']
   """
-  pass
+  assert scm_dirname in ['.svn', '.git']
+  has_checkout = any(map(lambda dir_name: path.exists(
+      path.join(os.getcwd(), dir_name, scm_dirname)), dir_names))
 
+  if has_checkout:
+    # cd .. && rm -rf ./build && mkdir ./build && cd build
+    build_dir = os.getcwd()
 
-def ensure_no_svn_checkout():
-  """Ensure that there is no svn checkout under build/.
+    os.chdir(path.dirname(os.getcwd()))
+    print '%s detected in checkout, deleting %s...' % (scm_dirname, build_dir),
+    shutil.rmtree(build_dir)
+    print 'done'
+    os.mkdir(build_dir)
+    os.chdir(build_dir)
 
-  If there is a svn checkout under build/, then move build/ to build.dead/
-  """
-  pass
 
 
 def gclient_configure(solutions):
-  pass
+  """Should do the same thing as gclient --spec='...'."""
+  with codecs.open('.gclient', mode='w', encoding='utf-8') as f:
+    f.write(get_gclient_spec(solutions))
 
 
-def gclient_shallow_sync():
-  pass
-
+def gclient_sync():
+  call('gclient', 'sync', '--verbose', '--reset', '--force',
+       '--nohooks', '--noprehooks')
+
+
+def get_git_hash(revision, dir_name):
+  match = "^git-svn-id: [^ ]*@%d" % revision
+  cmd = ['git', 'log', '--grep', match, '--format=%H', dir_name]
+  return subprocess.check_output(cmd).strip() or None
+
+
+def deps2git(sln_dirs):
+  for sln_dir in sln_dirs:
+    deps_file = path.join(os.getcwd(), sln_dir, 'DEPS')
+    deps_git_file = path.join(os.getcwd(), sln_dir, '.DEPS.git')
+    if not path.isfile(deps_file):
+      return
+    # Do we have a better way of doing this....?
+    repo_type = 'internal' if 'internal' in sln_dir else 'public'
+    # TODO(hinoka): This will be what populates the git caches on the first
+    #               run for all of the bots.  Since deps2git is single threaded,
+    #               all of this will run in a single threaded context and be
+    #               super slow.  Should make deps2git multithreaded.
+    call(sys.executable, DEPS2GIT_PATH, '-t', repo_type,
+         '--cache_dir=%s' % CACHE_DIR,
+         '--deps=%s' % deps_file, '--out=%s' % deps_git_file)
+
+
+def git_checkout(solutions, revision):
+  build_dir = os.getcwd()
+  # Revision only applies to the first solution.
+  first_solution = True
+  for sln in solutions:
+    name = sln['name']
+    url = sln['url']
+    sln_dir = path.join(build_dir, name)
+    if not path.isdir(sln_dir):
+      call('git', 'clone', url, sln_dir)
+
+    # Clean out .DEPS.git changes first.
+    call('git', '-C', sln_dir, 'reset', '--hard')
+    call('git', '-C', sln_dir, 'clean', '-df')
+    call('git', '-C', sln_dir, 'pull', 'origin', 'master')
+    # TODO(hinoka): We probably have to make use of revision mapping.
+    if first_solution and revision and revision.lower() != 'head':
+      if revision and revision.isdigit() and len(revision) < 40:
+        # rev_num is really a svn revision number, convert it into a git hash.
+        git_ref = get_git_hash(revision, name)
+      else:
+        # rev_num is actually a git hash or ref, we can just use it.
+        git_ref = revision
+      call('git', '-C', sln_dir, 'checkout', git_ref)
+    else:
+      call('git', '-C', sln_dir, 'checkout', 'origin/master')
 
-def git_pull_and_clean():
-  pass
+    first_solution = False
 
 
 def apply_issue(issue, patchset, root, server):
   pass
 
 
-def deps2git():
-  pass
+def delete_flag(flag_file):
+  """Remove bot update flag."""
+  if os.path.exists(flag_file):
+    os.remove(flag_file)
 
 
-def gclient_sync():
-  pass
-
-
-def deposit_bot_update_flag():
+def emit_flag(flag_file):
   """Deposit a bot update flag on the system to tell gclient not to run."""
-  pass
+  print 'Emitting flag file at %s' % flag_file
+  with open(flag_file, 'wb') as f:
+    f.write('Success!')
 
 
 def parse_args():
   parse = optparse.OptionParser()
 
-  parse.add_option('-i', '--issue', help='Issue number to patch from.')
-  parse.add_option('-p', '--patchset',
+  parse.add_option('--issue', help='Issue number to patch from.')
+  parse.add_option('--patchset',
                    help='Patchset from issue to patch from, if applicable.')
-  parse.add_option('-r', '--root', help='Repository root.')
-  parse.add_option('-c', '--server', help='Rietveld server.')
-  parse.add_option('-s', '--specs', help='Gcilent spec.')
-  parse.add_option('-m', '--master', help='Master name.')
+  parse.add_option('--patch_url', help='Optional URL to SVN patch.')
+  parse.add_option('--root', help='Repository root.')
+  parse.add_option('--rietveld_server', help='Rietveld server.')
+  parse.add_option('--specs', help='Gcilent spec.')
+  parse.add_option('--master', help='Master name.')
   parse.add_option('-f', '--force', action='store_true',
                    help='Bypass check to see if we want to be run. '
                         'Should ONLY be used locally.')
-  parse.add_option('-e', '--revision-mapping')
+  # TODO(hinoka): We don't actually use this yet, we should factor this in.
+  parse.add_option('--revision-mapping')
+  parse.add_option('--revision')
+  parse.add_option('--slave_name', default=socket.getfqdn().split('.')[0],
+                   help='Hostname of the current machine, '
+                   'used for determining whether or not to activate.')
+  parse.add_option('--builder_name', help='Name of the builder, '
+                   'used for determining whether or not to activate.')
+  parse.add_option('--build_dir', default=os.getcwd())
+  parse.add_option('--flag_file', default=path.join(os.getcwd(),
+                                                          'update.flag'))
 
   return parse.parse_args()
 
@@ -202,8 +342,8 @@ def parse_args():
 def main():
   # Get inputs.
   options, _ = parse_args()
-  builder = os.environ.get('BUILDBOT_BUILDERNAME', None)
-  slave = os.environ.get('BUILDBOT_SLAVENAME', None)
+  builder = options.builder_name
+  slave = options.slave_name
   master = options.master
 
   # Check if this script should activate or not.
@@ -220,21 +360,37 @@ def main():
 
   # Parse, munipulate, and print the gclient solutions.
   specs = {}
-  exec(options.specs, specs)  # TODO(hinoka): LOL this is terrible.
-  solutions = specs.get('solutions', [])
-  git_solutions = solutions_to_git(solutions)
+  exec(options.specs, specs)
+  svn_solutions = specs.get('solutions', [])
+  git_solutions = solutions_to_git(svn_solutions)
   solutions_printer(git_solutions)
 
-  # Do the checkout.
-  # TODO(hinoka): Uncomment these once they're implemented.
-  # ensure_no_svn_checkout()
-  # gclient_configure(git_solutions)
-  # gclient_shallow_sync()
-  # git_pull_and_clean()
+  # Cleanup svn checkout if active, otherwise remove git checkout and exit.
+  dir_names = [sln.get('name') for sln in svn_solutions if 'name' in sln]
+  if active:
+    ensure_no_checkout(dir_names, '.svn')
+    emit_flag(options.flag_file)
+  else:
+    ensure_no_checkout(dir_names, '.git')
+    delete_flag(options.flag_file)
+    return
+
+  # Get a checkout of each solution, without DEPS or hooks.
+  # Calling git directory because there is no way to run Gclient without
+  # invoking DEPS.
+  print 'Fetching Git checkout'
+  git_checkout(git_solutions, options.revision)
+
+  # TODO(hinoka): This must be implemented before we can turn this on for TS.
   # if options.issue:
   #   apply_issue(options.issue, options.patchset, options.root, options.server)
-  # deps2git()
-  # gclient_sync()
+
+  # Magic to get deps2git to work with internal DEPS.
+  shutil.copyfile(S2G_INTERNAL_FROM_PATH, S2G_INTERNAL_DEST_PATH)
+  deps2git(dir_names)
+
+  gclient_configure(git_solutions)
+  gclient_sync()
 
 
 if __name__ == '__main__':