Only apply DEPS patches before gclient sync, apply all other patches afterwards

scripts/slave/bot_update.py

 #!/usr/bin/env python
 # Copyright 2014 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 codecs
 import copy
 import cStringIO
 import ctypes
 import json
@@ skipping 206 matching lines @@
 
 class SVNRevisionNotFound(Exception):
   pass
 
 
 def call(*args, **kwargs):
   """Interactive subprocess call."""
   kwargs['stdout'] = subprocess.PIPE
   kwargs['stderr'] = subprocess.STDOUT
   stdin_data = kwargs.pop('stdin_data', None)
+  tries = kwargs.pop('tries', RETRIES)
   if stdin_data:
     kwargs['stdin'] = subprocess.PIPE
   out = cStringIO.StringIO()
   new_env = kwargs.get('env', {})
   env = copy.copy(os.environ)
   env.update(new_env)
   kwargs['env'] = env
-  for attempt in xrange(RETRIES):
+  for attempt in range(tries):

[iannucci, 2014/05/08 22:17:01]

hm... why not xrange?

[Ryan Tseng, 2014/05/09 20:59:42]

"range" sounds cleaner (xrange isn't even a word!), and at O(3) the use of a
generator is hardly relevant to performance.

     attempt_msg = ' (retry #%d)' % attempt if attempt else ''
     if new_env:
       print '===Injecting Environment Variables==='
       for k, v in sorted(new_env.items()):
         print '%s: %s' % (k, v)
     print '===Running %s%s===' % (' '.join(args), attempt_msg)
     start_time = time.time()
     proc = subprocess.Popen(args, **kwargs)
     if stdin_data:
       proc.stdin.write(stdin_data)
@@ skipping 17 matching lines @@
     code = proc.wait()
     elapsed_time = ((time.time() - start_time) / 60.0)
     if not code:
       print '===Succeeded in %.1f mins===' % elapsed_time
       print
       return out.getvalue()
     print '===Failed in %.1f mins===' % elapsed_time
     print
 
   raise SubprocessFailed('%s failed with code %d in %s after %d attempts.' %
-                         (' '.join(args), code, os.getcwd(), RETRIES), code)
+                         (' '.join(args), code, os.getcwd(), tries), code)
 
 
 def git(*args, **kwargs):
   """Wrapper around call specifically for Git commands."""
   if args and args[0] == 'cache':
     # Rewrite "git cache" calls into "python git_cache.py".
     cmd = (sys.executable, '-u', GIT_CACHE_PATH) + args[1:]
   else:
     git_executable = 'git'
     # On windows, subprocess doesn't fuzzy-match 'git' to 'git.bat', so we
@@ skipping 288 matching lines @@
 
 
 def _last_commit_for_file(filename, repo_base):
   cmd = ['log', '--format=%H', '--max-count=1', '--', filename]
   return git(*cmd, cwd=repo_base).strip()
 
 
 def need_to_run_deps2git(repo_base, deps_file, deps_git_file):
   """Checks to see if we need to run deps2git.
 
-  Returns True if there was a DEPS change after the last .DEPS.git update.
+  Returns True if there was a DEPS change after the last .DEPS.git update
+  or if DEPS has local modifications.
   """
   print 'Checking if %s exists' % deps_git_file
   if not path.isfile(deps_git_file):
     # .DEPS.git doesn't exist but DEPS does? We probably want to generate one.
-    print 'it exists!'
+    print 'it doesn\'t exist!'
+    return True
+
+  if git('diff', deps_file, cwd=repo_base).strip():
     return True
 
   last_known_deps_ref = _last_commit_for_file(deps_file, repo_base)
   last_known_deps_git_ref = _last_commit_for_file(deps_git_file, repo_base)
   merge_base_ref = git('merge-base', last_known_deps_ref,
                        last_known_deps_git_ref, cwd=repo_base).strip()
 
   # If the merge base of the last DEPS and last .DEPS.git file is not
   # equivilent to the hash of the last DEPS file, that means the DEPS file
   # was committed after the last .DEPS.git file.
@@ skipping 47 matching lines @@
   version = m.group(1)
   git_buildspec = get_git_buildspec(version)
   with open(deps_git_file, 'wb') as f:
     f.write(git_buildspec)
 
 
 def ensure_deps2git(sln_dir, shallow):
   repo_base = path.join(os.getcwd(), sln_dir)
   deps_file = path.join(repo_base, 'DEPS')
   deps_git_file = path.join(repo_base, '.DEPS.git')
-  print 'Checking if %s is newer than %s' % (deps_file, deps_git_file)
   if not path.isfile(deps_file):
     return
 
+  print 'Checking if %s is newer than %s' % (deps_file, deps_git_file)
   if not need_to_run_deps2git(repo_base, deps_file, deps_git_file):
     return
 
   print '===DEPS file modified, need to run deps2git==='
   # Magic to get deps2git to work with internal DEPS.
   shutil.copyfile(S2G_INTERNAL_FROM_PATH, S2G_INTERNAL_DEST_PATH)
 
   # TODO(hinoka): This might need to be smarter if we need to deal with
   #               DEPS changes that are in an internal repository.
   repo_type = 'public'
@@ skipping 127 matching lines @@
 def _download(url):
   """Fetch url and return content, with retries for flake."""
   for attempt in xrange(RETRIES):
     try:
       return urllib2.urlopen(url).read()
     except Exception:
       if attempt == RETRIES - 1:
         raise
 
 
-def apply_issue_svn(root, patch_url):
-  patch_data = call('svn', 'cat', patch_url)
-  call(PATCH_TOOL, '-p0', '--remove-empty-files', '--force', '--forward',
-       stdin_data=patch_data, cwd=root)
+def parse_diff(diff):
+  """Takes a unified diff and returns a list of diffed files and their diffs.
+
+  The return format is a list of pairs of:
+    (<filename>, <diff contents>)
+  <diff contents> is inclusive of the diff line.
+  """
+  result = []
+  current_diff = ''
+  current_header = None
+  for line in diff.splitlines():
+    # "diff" is for git style patches, and "Index: " is for SVN style patches.
+    if line.startswith('diff') or line.startswith('Index: '):
+      if current_header:
+        # If we are in a diff portion, then save the diff.
+        result.append((current_header, '%s\n' % current_diff))
+      git_header_match = re.match(r'diff (?:--git )?(\S+) (\S+)', line)
+      svn_header_match = re.match(r'Index: (.*)', line)
+
+      if git_header_match:
+        # First, see if its a git style header.
+        from_file = git_header_match.group(1)
+        to_file = git_header_match.group(2)
+        if from_file != to_file and from_file.startswith('a/'):
+          # Sometimes git prepends 'a/' and 'b/' in front of file paths.
+          from_file = from_file[2:]
+        current_header = from_file
+
+      elif svn_header_match:
+        # Otherwise, check if its an SVN style header.
+        current_header = svn_header_match.group(1)
+
+      else:
+        # Otherwise... I'm not really sure what to do with this.

[iannucci, 2014/05/08 22:17:01]

probably should abort loudly

[Ryan Tseng, 2014/05/09 20:59:42]

Done.

+        current_header = line
+
+      current_diff = ''
+    current_diff += '%s\n' % line
+  if current_header:
+    # We hit EOF, gotta save the last diff.
+    result.append((current_header, current_diff))
+  return result
 
 
-def apply_issue_rietveld(issue, patchset, root, server, rev_map, revision):
+def get_svn_patch(patch_url):
+  """Fetch patch from patch_url, return list of (filename, diff)"""
+  patch_data = call('svn', 'cat', patch_url)
+  return parse_diff(patch_data)
+
+
+def apply_svn_patch(patch_root, patches, whitelist=None, blacklist=None):
+  """Expects a list of (filename, diff), applies it on top of patch_root."""
+  if whitelist:
+    patches = [(name, diff) for name, diff in patches if name in whitelist]
+  elif blacklist:
+    patches = [(name, diff) for name, diff in patches if name not in blacklist]
+  diffs = [diff for _, diff in patches]
+  patch = ''.join(diffs)
+
+  if patch:
+    print '===Patching files==='
+    for filename, _ in patches:
+      print 'Patching %s' % filename
+    call(PATCH_TOOL, '-p0', '--remove-empty-files', '--force', '--forward',
+        stdin_data=patch, cwd=patch_root, tries=1)
+
+
+def apply_rietveld_issue(issue, patchset, root, server, rev_map, revision,
+                         whitelist=None, blacklist=None):
   apply_issue_bin = ('apply_issue.bat' if sys.platform.startswith('win')
                      else 'apply_issue')
-  call(apply_issue_bin,
-       '--root_dir', root,
-       '--issue', issue,
-       '--patchset', patchset,
-       '--no-auth',
-       '--server', server,
-       '--base_ref', revision,
-       '--force',
-       '--ignore_deps')
+  cmd = [apply_issue_bin,
+         # The patch will be applied on top of this directory.
+         '--root_dir', root,
+         # Tell apply_issue how to fetch the patch.
+         '--issue', issue,
+         '--patchset', patchset,
+         '--no-auth',
+         '--server', server,
+         # Always run apply_issue.py, otherwise it would see update.flag
+         # and then bail out.
+         '--force',
+         # Don't run gclient sync when it sees a DEPS change.
+         '--ignore_deps',
+         # Don't commit the patch or add it to the index.
+         '--no_commit',
+  ]
+  if whitelist:
+    for item in whitelist:
+      cmd.extend(['--whitelist', item])
+  elif blacklist:
+    for item in blacklist:
+      cmd.extend(['--blacklist', item])
+
+  # Only try once, since subsequent failures hide the real failure.
+  call(*cmd, tries=1)
 
 
 def check_flag(flag_file):
   """Returns True if the flag file is present."""
   return os.path.isfile(flag_file)
 
 
 def delete_flag(flag_file):
   """Remove bot update flag."""
   if os.path.isfile(flag_file):
     os.remove(flag_file)
 
 
 def emit_flag(flag_file):
   """Deposit a bot update flag on the system to tell gclient not to run."""
   print 'Emitting flag file at %s' % flag_file
   with open(flag_file, 'wb') as f:
     f.write('Success!')
 
 
-def get_real_git_hash(git_hash, dir_name):
-  """Return git_hash or the parent of git_hash if its a patched hash."""
-  log = git('log', '-1', '--format=%B', cwd=dir_name).strip()
-  if 'committed patch' in log.lower():
-    return git('log', '-1', '--format=%H', 'HEAD^', cwd=dir_name).strip()
-  return git_hash
-
-
 def parse_got_revision(gclient_output, got_revision_mapping, use_svn_revs):
   """Translate git gclient revision mapping to build properties.
 
   If use_svn_revs is True, then translate git hashes in the revision mapping
   to svn revision numbers.
   """
   properties = {}
   solutions_output = gclient_output['solutions']
   for dir_name, property_name in got_revision_mapping.iteritems():
     if dir_name not in solutions_output:
       continue
     solution_output = solutions_output[dir_name]
     if solution_output.get('scm') is None:
       # This is an ignored DEPS, so the output got_revision should be 'None'.
       git_revision = revision = None
     else:
       # Since we are using .DEPS.git, everything had better be git.
       assert solution_output.get('scm') == 'git'
-      git_revision = get_real_git_hash(solution_output['revision'], dir_name)
+      git_revision = solution_output['revision']
       if use_svn_revs:
         revision = get_svn_rev(git_revision, dir_name)
         if not revision:
           revision = git_revision
       else:
         revision = git_revision
 
     properties[property_name] = revision
     if revision != git_revision:
       properties['%s_git' % property_name] = git_revision
@@ skipping 13 matching lines @@
 
 def ensure_checkout(solutions, revision, first_sln, target_os, target_os_only,
                     root, issue, patchset, patch_url, rietveld_server,
                     revision_mapping, buildspec_name, gyp_env, shallow):
   # Get a checkout of each solution, without DEPS or hooks.
   # Calling git directly because there is no way to run Gclient without
   # invoking DEPS.
   print 'Fetching Git checkout'
   git_ref = git_checkout(solutions, revision, shallow)
 
-  # If either patch_url or issue is passed in, then we need to apply a patch.
+  patches = None
   if patch_url:
-    # patch_url takes precidence since its only passed in on gcl try/git try.
-    apply_issue_svn(root, patch_url)
-  elif issue:
-    apply_issue_rietveld(issue, patchset, root, rietveld_server,
-                         revision_mapping, git_ref)
+    patches = get_svn_patch(patch_url)
+
+  if root == first_sln:
+    # Only top level DEPS patching is supported right now.
+    if patches:
+      apply_svn_patch(root, patches, whitelist=['DEPS'])
+    elif issue:
+      apply_rietveld_issue(issue, patchset, root, rietveld_server,
+                          revision_mapping, git_ref, whitelist=['DEPS'])
+
 
   if buildspec_name:
     buildspecs2git(root, buildspec_name)
-  elif first_sln == root:
-    # Run deps2git if there is a DEPS commit after the last .DEPS.git commit.
-    # We only need to ensure deps2git if the root is not overridden, since
-    # if the root is overridden, it means we are working with a sub repository
-    # patch, which means its impossible for it to touch DEPS.
+  else:
+    # Run deps2git if there is a DEPS change after the last .DEPS.git commit.
     ensure_deps2git(root, shallow)
 
   # Ensure our build/ directory is set up with the correct .gclient file.
   gclient_configure(solutions, target_os, target_os_only)
 
   # Let gclient do the DEPS syncing.
   gclient_output = gclient_sync(buildspec_name)
   if buildspec_name:
     # Run gclient runhooks if we're on an official builder.
     # TODO(hinoka): Remove this when the official builders run their own
     #               runhooks step.
     gclient_runhooks(gyp_env)
+
+  # Apply the rest of the patch here (sans DEPS)
+  if patches:
+    apply_svn_patch(root, patches, blacklist=['DEPS'])
+  elif issue:
+    apply_rietveld_issue(issue, patchset, root, rietveld_server,
+                         revision_mapping, git_ref, blacklist=['DEPS'])
+
   return gclient_output
 
 
 def parse_args():
   parse = optparse.OptionParser()
 
   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('--patch_url', help='Optional URL to SVN patch.')
@@ skipping 148 matching lines @@
               patch_root=options.root,
               step_text=step_text,
               properties=got_revisions)
   else:
     # If we're not on recipes, tell annotator about our got_revisions.
     emit_properties(got_revisions)
 
 
 if __name__ == '__main__':
   sys.exit(main())