Add a git-drover.

git_drover.py

Index: git_drover.py
diff --git a/git_drover.py b/git_drover.py
new file mode 100755
index 0000000000000000000000000000000000000000..21f0f7993956e1338a852d54f66ba6815ec38d32
--- /dev/null
+++ b/git_drover.py
@@ -0,0 +1,281 @@
+#!/usr/bin/env python
+# Copyright 2015 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.
+"""git drover: A tool for merging changes to release branches."""
+
+import argparse
+import logging
+import os
+import shutil
+import subprocess
+import sys
+import tempfile
+
+
+class Error(Exception):
+  pass
+
+
+class _Drover(object):
+
+  def __init__(self, branch, cl, parent_repo, dry_run):
+    self._branch = branch
+    self._cl = cl
+    self._parent_repo = os.path.abspath(parent_repo)
+    self._dry_run = dry_run
+    self._workdir = None
+    self._branch_name = None
+    self._dev_null_file = None
+    self._cherry_pick_in_progress = False
+    self._cwd = os.getcwd()
+
+  def run(self):
+    """Runs this Drover instance.
+
+    Raises:
+      Error: An error occurred while attempting to merge this change.
+    """
+    try:
+      self._run_internal()
+    finally:
+      self._cleanup()
+
+  def _run_internal(self):
+    self._check_inputs()
+    if not self._confirm('Going to merge this to %s.' % self._branch):
+      return
+    self._create_checkout()
+    self._prepare_merge()
+    if self._dry_run:
+      logging.info('--dry_run enabled; not landing.')
+      return
+
+    self._run_git_command(['cl', 'upload'], error_message='Upload failed')
+
+    if not self._confirm('About to land on %s.' % self._branch):
+      return
+    self._run_git_command(['cl', 'land', '--bypass-hooks'])
+
+  def _cleanup(self):
+    if self._cherry_pick_in_progress:
+      self._run_git_command(['cherry-pick', '--abort'])

[iannucci, 2015/09/22 04:18:11]

you could also just run this always and swallow the error

[Sam McNally, 2015/09/23 01:16:07]

Done.

+    if self._branch_name:
+      self._run_git_command(['checkout', '--detach'], quiet=True)
+      self._run_git_command(['branch', '-D', self._branch_name], quiet=True)
+    os.chdir(self._cwd)

[iannucci, 2015/09/22 04:18:11]

I'd very strongly advise to avoid chdir'ing (except possibly once in main()). It
can introduce an entire class of ugly bugs that you don't want :(

[Sam McNally, 2015/09/23 01:16:07]

Done.

+    if self._workdir:
+      logging.debug('Deleting %s', self._workdir)
+      shutil.rmtree(self._workdir)
+    if self._dev_null_file:
+      self._dev_null_file.close()
+      self._dev_null_file = None
+
+  @staticmethod
+  def _confirm(message):
+    """Show a confirmation prompt with the given message.
+
+    Returns:
+      A bool representing whether the user wishes to continue.
+    """
+    result = ''
+    while result not in ('y', 'n'):
+      try:
+        result = raw_input('%s Continue (y/n)? ' % message)
+      except EOFError:
+        result = 'n'
+    return result == 'y'
+
+  def _check_inputs(self):
+    """Check the input arguments and ensure the parent repo is up to date."""
+
+    if not os.path.isdir(self._parent_repo):
+      raise Error('Invalid parent repo path %r' % self._parent_repo)
+    if not hasattr(os, 'symlink'):
+      raise Error('Symlink support is required')
+
+    os.chdir(self._parent_repo)
+    self._run_git_command(['--help'],
+                          error_message='Unable to run git',
+                          quiet=True)
+    self._run_git_command(['status'],
+                          quiet=True,
+                          error_message='%r is not a valid git repo' %
+                          os.path.abspath(self._parent_repo))
+    self._run_git_command(['fetch', 'origin'],
+                          error_message='Failed to fetch origin')
+    self._run_git_command(
+        ['rev-parse', 'branch-heads/%s' % self._branch],

[iannucci, 2015/09/22 04:18:11]

let's use absolute refs (so: `refs/branch-heads/___`). Unless git sees the
`refs/` portion, it will 'search' for the most likely branch. This can end up
picking something weird if the user has more than one remote, or has copied refs
somewhere, or has ever run 'filter-branch', or....

[Sam McNally, 2015/09/23 01:16:07]

Done. I had to use refs/remotes/branch-heads/...

+        quiet=True,
+        error_message='Branch %s not found' % self._branch)
+    self._run_git_command(['rev-parse', self._cl],
+                          quiet=True,
+                          error_message='Revision "%s" not found' % self._cl)
+    self._run_git_command(['show', '-s', self._cl])
+
+  @staticmethod
+  def _process_files(source_dir, target_dir, filenames, operation):
+    """Performs a given operation on a list of files.
+
+    Args:
+      source_dir: A string containing the source directory.
+      source_dir: A string containing the target directory.
+      filenames: A list of strings containing the files within |source_dir| to
+          be processed.
+      operation: A binary function to be called for each file in |filenames|, in
+          the form operation(|source_dir|/file, |target_dir|/file).
+    """
+    for filename in filenames:
+      if not isinstance(filename, tuple):
+        filename = (filename,)
+      target = os.path.join(target_dir, *filename)
+      source = os.path.join(source_dir, *filename)
+      if not os.path.exists(os.path.dirname(target)):
+        os.makedirs(os.path.dirname(target))
+      logging.debug('Cloning %r to %r', source, target)
+      operation(source, target)
+
+  FILES_TO_LINK = [
+      'refs',
+      ('logs', 'refs'),
+      'objects',
+      ('info', 'refs'),
+      ('info', 'exclude'),
+      'hooks',
+      'packed-refs',
+      'remotes',
+      'rr-cache',
+      'svn',
+  ]
+  FILES_TO_COPY = ['config', 'HEAD']
+
+  def _create_checkout(self):
+    """Creates a checkout to use for cherry-picking.
+
+    This creates a checkout similarly to git-new-workdir. Most of the .git
+    directory is shared with the |self._parent_repo| using symlinks. This
+    differs from git-new-workdir in that the config is forked instead of shared.
+    This is so the new workdir can be a sparse checkout without affecting
+    |self._parent_repo|.
+    """
+    self._workdir = tempfile.mkdtemp(prefix='drover_%s_' % self._branch)
+    logging.debug('Creating checkout in %s', self._workdir)
+    os.chdir(self._workdir)
+    parent_git_dir = os.path.join(self._parent_repo, '.git')

[iannucci, 2015/09/22 04:18:11]

you can use `git rev-parse --git-dir` to get this in case they're not in the
root of the repo.

[Sam McNally, 2015/09/23 01:16:07]

Done.

+    git_dir = os.path.join(self._workdir, '.git')
+    os.mkdir(git_dir)
+    logging.debug('Creating symlinks')
+    self._process_files(parent_git_dir, git_dir, self.FILES_TO_LINK, os.symlink)
+    logging.debug('Creating copies')
+    self._process_files(parent_git_dir, git_dir, self.FILES_TO_COPY,
+                        shutil.copy)
+    self._run_git_command(['config', 'core.sparsecheckout', 'true'])
+    with open(os.path.join(git_dir, 'info', 'sparse-checkout'), 'w') as f:
+      f.write('codereview.settings')
+
+    branch_name = os.path.split(self._workdir)[-1]
+    self._run_git_command(
+        ['checkout', '-b', branch_name, 'branch-heads/%s' % self._branch],

[iannucci, 2015/09/22 04:18:11]

absolute refs

[Sam McNally, 2015/09/23 01:16:07]

Done.

+        quiet=True)
+    self._branch_name = branch_name

[iannucci, 2015/09/22 04:18:11]

Is there any way we can use the actual git workdir script for this? Or share
this code in git_common.py or something?

Also, have you considered using `git clone --shared` or `git clone --local` to
do this operation?

[Sam McNally, 2015/09/23 01:16:07]

Moved it from gclient-new-workdir.py. This has to be slightly different; it sets
the workdir to do a sparse checkout and we don't want this to leak into the
parent checkout so .git/config and .git/info/sparse-checkout aren't shared.
I hadn't, but I think this might be simpler than those. Doing a clone would
require copying across the right remote and creating a sparse clone is a bit
fiddly.

+
+  def _prepare_merge(self):

[iannucci, 2015/09/22 04:18:11]

s/merge/cherry_pick/ everywhere (mentioned below) 

[Sam McNally, 2015/09/23 01:16:07]

Done.

+    self._cherry_pick_in_progress = True
+    self._run_git_command(['cherry-pick', '-x', self._cl],
+                          quiet=True,
+                          error_message='Patch failed to apply')
+    self._cherry_pick_in_progress = False

[iannucci, 2015/09/22 04:18:11]

fwiw, git tracks this bit of information in a file in .git... maybe good to use
that instead of duplicating it (thinking that it could get out of sync on an
edge case error)?

[Sam McNally, 2015/09/23 01:16:07]

Removed this.

+    self._run_git_command(['reset', '--hard'], quiet=True)
+
+  def _run_git_command(self, args, quiet=False, error_message=None):
+    """Runs a git command.
+
+    Args:
+      args: A list of strings containing the args to pass to git.
+      quiet: A bool containing whether to redirect output to /dev/null.
+      error_message: A string containing the error message to report if the
+          command fails.
+
+    Raises:
+      Error: The command failed to complete successfully.
+    """
+    logging.debug('Running git %s', ' '.join('%s' % arg for arg in args))
+    output = None
+    if quiet:
+      if self._dev_null_file is None:
+        self._dev_null_file = open(os.devnull, 'w')

[iannucci, 2015/09/22 04:18:11]

I'd probably do this in __init__, since it's essentially zero-cost, and if quiet
is true (which it is for the very first git invocation), then you'll end up
doing it anyway.

[Sam McNally, 2015/09/23 01:16:07]

Done.

+      output = self._dev_null_file
+
+    try:
+      subprocess.check_call(
+          ['git'] + args,
+          stdout=output,
+          stderr=output,
+          shell=False)
+    except (OSError, subprocess.CalledProcessError) as e:
+      if error_message:
+        raise Error(error_message)
+      else:
+        raise Error('Command %r failed: %s' % (' '.join(args), e))
+
+
+def merge_change(branch, cl, parent_repo, dry_run):
+  """Merges a change into a branch.
+
+  Args:
+    branch: A string containing the release branch number to which to merge.

[iannucci, 2015/09/22 04:18:11]

Does it work with 'mini' branches?

[Sam McNally, 2015/09/23 01:16:07]

I'm not sure what 'mini' branches are.

+    cl: A string containing the git hash of the patch to merge.

[iannucci, 2015/09/22 04:18:11]

maybe 'revision'?

Is it required to be the full git hash?

[Sam McNally, 2015/09/23 01:16:07]

Done.
No, it can be anything that git-rev-parse likes.

+    parent_repo: A string containing the path to the parent repo to use for this
+        merge.
+    dry_run: A boolean containing whether to stop before uploading the merge cl.
+
+  Raises:
+    Error: An error occurred while attempting to merge |cl| to |branch|.
+  """
+  drover = _Drover(branch, cl, parent_repo, dry_run)
+  drover.run()
+
+
+def main():
+  parser = argparse.ArgumentParser(
+      description='Merge a change into a release branch.')
+  parser.add_argument('--branch',
+                      type=str,
+                      required=True,
+                      metavar='<branch>',
+                      help='the name of the branch to which to merge')

[iannucci, 2015/09/22 04:18:11]

full name or just the bit after `refs/branch-heads/`? May want to supply an
example, and make sure that if they provide `1234` it works as well as
`refs/branch-heads/1234` (or errors, or something... just so it's well-defined).

[Sam McNally, 2015/09/23 01:16:07]

Just the bit after.

+  parser.add_argument('--merge',
+                      type=str,
+                      required=True,
+                      metavar='<change>',
+                      help='the hash of the change to merge')

[iannucci, 2015/09/22 04:18:11]

s/merge/cherry-pick/ since 'merge' means something very specific in git, and
we're not using it that way here.

[Sam McNally, 2015/09/23 01:16:07]

Done.

+  parser.add_argument(
+      '--parent_checkout',
+      type=str,
+      default=os.path.abspath('.'),
+      metavar='<path_to_parent_checkout>',
+      help=('the path to the chromium checkout to use; '

[iannucci, 2015/09/22 04:18:11]

definitely want to explain what 'use' means here (perhaps in the man page?). I'm
assuming it's just for git objects?

[Sam McNally, 2015/09/23 01:16:07]

Done.

+            'if unspecified, the current directory is used'))
+  parser.add_argument('--dry-run',
+                      action='store_true',
+                      default=False,
+                      help=("Don't actually upload and land. "
+                            'Just check that merging would succeed'))

[iannucci, 2015/09/22 04:18:11]

mixed ' and " is weird... I see why you did it, but maybe switch both of them on
this 
line to " just to avoid the weirdness :)

[Sam McNally, 2015/09/23 01:16:07]

Done.

+  parser.add_argument('-v',
+                      '--verbose',
+                      action='store_true',
+                      default=False,
+                      help='Show verbose logging')
+  options = parser.parse_args()
+  if options.verbose:
+    logging.getLogger().setLevel(logging.DEBUG)
+  try:
+    merge_change(options.branch, options.merge, options.parent_checkout,
+                 options.dry_run)
+  except Error as e:
+    logging.error(e.message)
+    sys.exit(128)
+
+
+if __name__ == '__main__':
+  main()