Now trychange can store patches in a Git repo

trychange.py

 #!/usr/bin/env python
 # Copyright (c) 2012 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.
 
 """Client-side script to send a try job to the try server. It communicates to
-the try server by either writting to a svn repository or by directly connecting
-to the server by HTTP.
+the try server by either writting to a svn/git repository or by directly
+connecting to the server by HTTP.
 """
 
+
+import contextlib
 import datetime
 import errno
 import getpass
 import itertools
 import json
 import logging
 import optparse
 import os
 import posixpath
 import re
@@ skipping 42 matching lines @@
   premade diff file on the local drive:
     %(prog)s --email user@example.com
             --svn_repo svn://svn.chromium.org/chrome-try/try --diff foo.diff
 
   Running only on a 'mac' slave with revision 123 and clobber first; specify
   manually the 3 source files to use for the try job:
     %(prog)s --bot mac --revision 123 --clobber -f src/a.cc -f src/a.h
             -f include/b.h
 """
 
+GIT_PATCH_DIR_BASENAME = os.path.join('git-try', 'patches-git')
 
 def DieWithError(message):
   print >> sys.stderr, message
   sys.exit(1)
 
 
 def RunCommand(args, error_ok=False, error_message=None, **kwargs):
   try:
     return subprocess2.check_output(args, shell=False, **kwargs)
   except subprocess2.CalledProcessError, e:
@@ skipping 74 matching lines @@
     """Set default settings based on the gcl-style settings from the repository.
 
     The settings in the self.options object will only be set if no previous
     value exists (i.e. command line flags to the try command will override the
     settings in codereview.settings).
     """
     settings = {
       'port': self.GetCodeReviewSetting('TRYSERVER_HTTP_PORT'),
       'host': self.GetCodeReviewSetting('TRYSERVER_HTTP_HOST'),
       'svn_repo': self.GetCodeReviewSetting('TRYSERVER_SVN_URL'),
+      'git_repo': self.GetCodeReviewSetting('TRYSERVER_GIT_URL'),
       'project': self.GetCodeReviewSetting('TRYSERVER_PROJECT'),
+      # primarily for revision=auto
+      'revision': self.GetCodeReviewSetting('TRYSERVER_REVISION'),
       'root': self.GetCodeReviewSetting('TRYSERVER_ROOT'),
       'patchlevel': self.GetCodeReviewSetting('TRYSERVER_PATCHLEVEL'),
     }
     logging.info('\n'.join(['%s: %s' % (k, v)
                             for (k, v) in settings.iteritems() if v]))
     for (k, v) in settings.iteritems():
       # Avoid overwriting options already set using command line flags.
       if v and getattr(self.options, k) is None:
         setattr(self.options, k, v)
 
@@ skipping 101 matching lines @@
 
     The files in the list should either be absolute paths or relative to the
     given root.
     """
     return scm.SVN.GenerateDiff(self.files, self.checkout_root, full_move=True,
                                 revision=self.diff_against)
 
 
 class GIT(SCM):
   """Gathers the options and diff for a git checkout."""
+
   def __init__(self, *args, **kwargs):
     SCM.__init__(self, *args, **kwargs)
     self.checkout_root = scm.GIT.GetCheckoutRoot(self.checkout_root)
     if not self.options.name:
       self.options.name = scm.GIT.GetPatchName(self.checkout_root)
     if not self.options.email:
       self.options.email = scm.GIT.GetEmail(self.checkout_root)
     if not self.diff_against:
       self.diff_against = scm.GIT.GetUpstreamBranch(self.checkout_root)
       if not self.diff_against:
@@ skipping 104 matching lines @@
 
       bot_spec.extend(_ApplyTestFilter(options.testfilter, new_style))
 
   except ImportError:
     pass
 
   return bot_spec
 
 
 def _ParseSendChangeOptions(bot_spec, options):
-  """Parse common options passed to _SendChangeHTTP and _SendChangeSVN."""
+  """Parse common options passed to _SendChangeHTTP, _SendChangeSVN and
+  _SendChangeGit.
+  """
   values = [
     ('user', options.user),
     ('name', options.name),
   ]
   if options.email:
     values.append(('email', options.email))
   if options.revision:
     values.append(('revision', options.revision))
   if options.clobber:
     values.append(('clobber', 'true'))
@@ skipping 50 matching lines @@
                                                                   str(e.args)))
   if not connection:
     raise NoTryServerAccess('%s is unaccessible.' % url)
   logging.info('Reading response...')
   response = connection.read()
   logging.info('Done')
   if response != 'OK':
     raise NoTryServerAccess('%s is unaccessible. Got:\n%s' % (url, response))
 
 
+@contextlib.contextmanager
+def _PrepareDescriptionAndPatchFiles(description, options):
+  """Create temporary files with description and patch, yield execution,
+  and delete the files.
+
+
+  __enter__ called on the return value returns a tuple of patch_filename and
+  description_filename.
+
+  Args:
+    description: contents of description file.
+    options: patchset options object. Must have attributes: user,
+        name (of patch) and diff (contents of patch).
+  """
+
+  # Create a temporary file and put description into it.
+  with tempfile.NamedTemporaryFile() as description_file:
+    description_file.write(description)
+    description_file.flush()
+
+    # Create a temporary directory, put a uniquely named file in it with the
+    # diff content
+    temp_dir = tempfile.mkdtemp()
+    try:
+      current_time = str(datetime.datetime.now()).replace(':', '.')
+      file_name = '%s.%s.%s.diff' % (Escape(options.user),
+                                     Escape(options.name), current_time)
+      full_patch_filename = os.path.join(temp_dir, file_name)
+      with open(full_patch_filename, 'wb') as f:
+        f.write(options.diff)
+
+      yield full_patch_filename, description_file.name

[M-A Ruel, 2014/03/25 01:42:56]

If description_file.name is to be opened by another process, it'll likely won't
work on Windows due to the way NamedTemporaryFile() works (it doesn't allow file
share). In this case you'll have to revert to mkstemp(). Note that the previous
code seemed to work so it may be fine.

Use a sensible prefix= value for it and also for mkdtemp call.

[nodir, 2014/03/26 05:26:52]

Done.

+    finally:
+      shutil.rmtree(temp_dir, True)
+
+
 def _SendChangeSVN(bot_spec, options):
   """Send a change to the try server by committing a diff file on a subversion
   server."""
   if not options.svn_repo:
     raise NoTryServerAccess('Please use the --svn_repo option to specify the'
                             ' try server svn repository to connect to.')
 
   values = _ParseSendChangeOptions(bot_spec, options)
   description = ''.join("%s=%s\n" % (k, v) for k, v in values)
   logging.info('Sending by SVN')
   logging.info(description)
   logging.info(options.svn_repo)
   logging.info(options.diff)
   if options.dry_run:
     return
 
-  # Create a temporary directory, put a uniquely named file in it with the diff
-  # content and svn import that.
-  temp_dir = tempfile.mkdtemp()
-  temp_file = tempfile.NamedTemporaryFile()
-  try:
+  with _PrepareDescriptionAndPatchFiles(description, options) as (
+       patch_filename, description_filename):
+    if sys.platform == "cygwin":
+      # Small chromium-specific issue here:
+      # git-try uses /usr/bin/python on cygwin but svn.bat will be used
+      # instead of /usr/bin/svn by default. That causes bad things(tm) since
+      # Windows' svn.exe has no clue about cygwin paths. Hence force to use
+      # the cygwin version in this particular context.
+      exe = "/usr/bin/svn"
+    else:
+      exe = "svn"
+    patch_dir = os.path.dirname(patch_filename)
+    command = [exe, 'import', '-q', patch_dir, options.svn_repo, '--file',
+               description_filename]
+    if scm.SVN.AssertVersion("1.5")[0]:
+      command.append('--no-ignore')
+
     try:
-      # Description
-      temp_file.write(description)
-      temp_file.flush()
-
-      # Diff file
-      current_time = str(datetime.datetime.now()).replace(':', '.')
-      file_name = (Escape(options.user) + '.' + Escape(options.name) +
-                   '.%s.diff' % current_time)
-      full_path = os.path.join(temp_dir, file_name)
-      with open(full_path, 'wb') as f:
-        f.write(options.diff)
-
-      # Committing it will trigger a try job.
-      if sys.platform == "cygwin":
-        # Small chromium-specific issue here:
-        # git-try uses /usr/bin/python on cygwin but svn.bat will be used
-        # instead of /usr/bin/svn by default. That causes bad things(tm) since
-        # Windows' svn.exe has no clue about cygwin paths. Hence force to use
-        # the cygwin version in this particular context.
-        exe = "/usr/bin/svn"
-      else:
-        exe = "svn"
-      command = [exe, 'import', '-q', temp_dir, options.svn_repo, '--file',
-                 temp_file.name]
-      if scm.SVN.AssertVersion("1.5")[0]:
-        command.append('--no-ignore')
-
       subprocess2.check_call(command)
     except subprocess2.CalledProcessError, e:
       raise NoTryServerAccess(str(e))
-  finally:
-    temp_file.close()
-    shutil.rmtree(temp_dir, True)
+
+
+def _GetPatchGitRepo(git_url):
+  """Get a path to a Git repo with patches with latest changes,

[M-A Ruel, 2014/03/25 01:42:56]

Imperattive everywhere.

[nodir, 2014/03/26 05:26:52]

Not sure I got it. Isn't "Get a patch..." imperative?

+  associated with the given url.
+
+  Store patches in .git/git-try/patches-git directory, a git repo. If it
+  doesn't exist yet or its origin URL is different, then clean up and clone it.
+  If it existed before, then pull changes.
+
+  Does not support SVN repo.
+
+  Returns a path to the directory with patches.
+  """
+  git_dir = scm.GIT.GetGitDir(os.getcwd())
+  patch_dir = os.path.join(git_dir, GIT_PATCH_DIR_BASENAME)
+
+  logging.info('Looking for git repo for patches')
+  # Is there already a repo with the expected url or should we clone?
+  clone = True
+  if os.path.exists(patch_dir) and scm.GIT.IsInsideWorkTree(patch_dir):
+    existing_url = scm.GIT.Capture(
+        ['config', '--local', 'remote.origin.url'],
+        cwd=patch_dir)
+    clone = existing_url != git_url
+
+  if clone:
+    if os.path.exists(patch_dir):
+      logging.info('Cleaning up')
+      shutil.rmtree(patch_dir, True)
+    logging.info('Cloning patch repo')
+    scm.GIT.Capture(['clone', git_url, GIT_PATCH_DIR_BASENAME], cwd=git_dir)
+    email = scm.GIT.GetEmail(cwd=os.getcwd())
+    scm.GIT.Capture(['config', '--local', 'user.email', email], patch_dir)
+  else:
+    if scm.GIT.IsWorkTreeDirty(patch_dir):
+      logging.info('Work dir is dirty: hard reset!')
+      scm.GIT.Capture(['reset', '--hard'], cwd=patch_dir)
+    logging.info('Updating patch repo')
+    scm.GIT.Capture(['pull', 'origin', 'master'], cwd=patch_dir)

[M-A Ruel, 2014/03/25 01:42:56]

Technically, you don't want to pull other's patches ?

[nodir, 2014/03/26 05:26:52]

Probably this is safer because the push won't be fast-forward

+
+  return patch_dir
+
+
+def _SendChangeGit(bot_spec, options):
+  """Send a change to the try server by committing a diff file to a GIT repo"""
+  if not options.git_repo:
+    raise NoTryServerAccess('Please use the --git_repo option to specify the '
+                            'try server git repository to connect to.')
+
+  values = _ParseSendChangeOptions(bot_spec, options)
+  comment_subject = '%s.%s' % (options.user, options.name)
+  comment_body = ''.join("%s=%s\n" % (k, v) for k, v in values)
+  description = '%s\n\n%s' % (comment_subject, comment_body)
+  logging.info('Sending by GIT')
+  logging.info(description)
+  logging.info(options.git_repo)
+  logging.info(options.diff)
+  if options.dry_run:
+    return
+
+  patch_dir = _GetPatchGitRepo(options.git_repo)
+  assert scm.GIT.IsInsideWorkTree(patch_dir)
+  assert not scm.GIT.IsWorkTreeDirty(patch_dir)
+
+  with _PrepareDescriptionAndPatchFiles(description, options) as (
+       patch_filename, description_filename):
+    logging.info('Committing patch')
+    target_filename = os.path.abspath(
+        os.path.join(patch_dir, os.path.basename(patch_filename)))
+    shutil.copyfile(patch_filename, target_filename)
+    try:
+      scm.GIT.Capture(['add', target_filename], cwd=patch_dir)
+      scm.GIT.Capture(['commit', '-F', description_filename], cwd=patch_dir)
+      assert not scm.GIT.IsWorkTreeDirty(patch_dir)
+      logging.info('Pushing patch')
+      scm.GIT.Capture(['push', 'origin', 'HEAD'], cwd=patch_dir)

[M-A Ruel, 2014/03/25 01:42:56]

Pushing on origin/HEAD? Usually you push to a branch otherwise the change may be
lost.

[nodir, 2014/03/26 05:26:52]

Done.

+    except subprocess2.CalledProcessError, e:
+      scm.GIT.Capture(['reset', '--hard', 'origin/master'], cwd=patch_dir)
+      raise NoTryServerAccess(str(e))
+
 
 
 def PrintSuccess(bot_spec, options):
   if not options.dry_run:
     text = 'Patch \'%s\' sent to try server' % options.name
     if bot_spec:
       text += ': %s' % ', '.join(
           '%s:%s' % (b[0], ','.join(b[1])) for b in bot_spec)
     print(text)
 
@@ skipping 95 matching lines @@
             "times to specify multiple builders. ex: "
             "'-bwin_rel:ui_tests,webkit_unit_tests -bwin_layout'. See "
             "the try server waterfall for the builders name and the tests "
             "available. Can also be used to specify gtest_filter, e.g. "
             "-bwin_rel:base_unittests:ValuesTest.*Value"))
   group.add_option("-B", "--print_bots", action="store_true",
                     help="Print bots we would use (e.g. from PRESUBMIT.py)"
                          " and exit.  Do not send patch.  Like --dry_run"
                          " but less verbose.")
   group.add_option("-r", "--revision",
-                    help="Revision to use for the try job; default: the "
+                    help="Revision to use for the try job. If 'auto' is "
+                         "specified, it is resolved to the revision a patch is "
+                         "generated against (Git only). Default: the "
                          "revision will be determined by the try server; see "
                          "its waterfall for more info")
   group.add_option("-c", "--clobber", action="store_true",
                     help="Force a clobber before building; e.g. don't do an "
                          "incremental build")
   # TODO(maruel): help="Select a specific configuration, usually 'debug' or "
   #                    "'release'"
   group.add_option("--target", help=optparse.SUPPRESS_HELP)
 
   group.add_option("--project",
@@ skipping 65 matching lines @@
   group.add_option("--use_svn",
                    action="store_const",
                    const=_SendChangeSVN,
                    dest="send_patch",
                    help="Use SVN to talk to the try server")
   group.add_option("-S", "--svn_repo",
                    metavar="SVN_URL",
                    help="SVN url to use to write the changes in; --use_svn is "
                         "implied when using --svn_repo")
   parser.add_option_group(group)
+
+  group = optparse.OptionGroup(parser, "Access the try server with Git")
+  group.add_option("--use_git",
+                   action="store_const",
+                   const=_SendChangeGit,
+                   dest="send_patch",
+                   help="Use GIT to talk to the try server")
+  group.add_option("-G", "--git_repo",
+                   metavar="GIT_URL",
+                   help="GIT url to use to write the changes in; --use_git is "
+                        "implied when using --git_repo")
+  parser.add_option_group(group)
   return parser
 
 
 def TryChange(argv,
               change,
               swallow_exception,
               prog=None,
               extra_epilog=None):
   """
   Args:
@@ skipping 48 matching lines @@
     match = re.match(r'^(.*)/(\d+)/?$', options.rietveld_url)
     if match:
       if options.issue or options.patchset:
         parser.error('Cannot use both --issue and use a review number url')
       options.issue = int(match.group(2))
       options.rietveld_url = match.group(1)
 
   try:
     changed_files = None
     # Always include os.getcwd() in the checkout settings.
-    checkouts = []
     path = os.getcwd()
 
     file_list = []
     if options.files:
       file_list = options.files
     elif change:
       file_list = [f.LocalPath() for f in change.AffectedFiles()]
 
     if options.upstream_branch:
       path += '@' + options.upstream_branch
       # Clear file list so that the correct list will be retrieved from the
       # upstream branch.
       file_list = []
-    checkouts.append(GuessVCS(options, path, file_list))
-    checkouts[0].AutomagicalSettings()
+
+    current_vcs = GuessVCS(options, path, file_list)
+    current_vcs.AutomagicalSettings()
+    options = current_vcs.options
+    vcs_is_git = type(current_vcs) is GIT
+
+    # So far, git_repo doesn't work with SVN
+    if options.git_repo and not vcs_is_git:
+      parser.error('--git_repo option is supported only for GIT repositories')
+
+    # If revision==auto, resolve it
+    if options.revision and options.revision.lower() == 'auto':
+      if not vcs_is_git:
+        parser.error('--revision=auto is supported only for GIT repositories')
+      options.revision = scm.GIT.Capture(
+          ['rev-parse', current_vcs.diff_against],
+          cwd=path)
+
+    checkouts = [current_vcs]
     for item in options.sub_rep:
       # Pass file_list=None because we don't know the sub repo's file list.
       checkout = GuessVCS(options,
-                          os.path.join(checkouts[0].checkout_root, item),
+                          os.path.join(current_vcs.checkout_root, item),
                           None)
       if checkout.checkout_root in [c.checkout_root for c in checkouts]:
         parser.error('Specified the root %s two times.' %
                      checkout.checkout_root)
       checkouts.append(checkout)
 
     can_http = options.port and options.host
     can_svn = options.svn_repo
+    can_git = options.git_repo
     # If there was no transport selected yet, now we must have enough data to
     # select one.
-    if not options.send_patch and not (can_http or can_svn):
+    if not options.send_patch and not (can_http or can_svn or can_git):
       parser.error('Please specify an access method.')
 
     # Convert options.diff into the content of the diff.
     if options.url:
       if options.files:
         parser.error('You cannot specify files and --url at the same time.')
       options.diff = urllib2.urlopen(options.url).read()
     elif options.diff:
       if options.files:
         parser.error('You cannot specify files and --diff at the same time.')
@@ skipping 57 matching lines @@
 
     if options.print_bots:
       print 'Bots which would be used:'
       for bot in bot_spec:
         if bot[1]:
           print '  %s:%s' % (bot[0], ','.join(bot[1]))
         else:
           print '  %s' % (bot[0])
       return 0
 
-    # Send the patch.
+    # Determine sending protocol
     if options.send_patch:
       # If forced.
-      options.send_patch(bot_spec, options)
-      PrintSuccess(bot_spec, options)
-      return 0
-    try:
-      if can_http:
-        _SendChangeHTTP(bot_spec, options)
+      senders = [options.send_patch]
+    else:
+      # Try sending patch using avaialble protocols
+      all_senders = [
+        (_SendChangeHTTP, can_http),
+        (_SendChangeSVN, can_svn),
+        (_SendChangeGit, can_git)
+      ]
+      senders = [sender for sender, can in all_senders if can]
+
+    # Send the patch.
+    for sender in senders:
+      try:
+        sender(bot_spec, options)
         PrintSuccess(bot_spec, options)
         return 0
-    except NoTryServerAccess:
-      if not can_svn:
-        raise
-    _SendChangeSVN(bot_spec, options)
-    PrintSuccess(bot_spec, options)
-    return 0
+      except NoTryServerAccess:
+        is_last = sender == senders[-1]
+        if is_last:
+          raise
+    assert False, "Unreachable code"
   except (InvalidScript, NoTryServerAccess), e:
     if swallow_exception:
       return 1
     print >> sys.stderr, e
     return 1
   except (gclient_utils.Error, subprocess2.CalledProcessError), e:
     print >> sys.stderr, e
     return 1
   return 0
 
 
 if __name__ == "__main__":
   fix_encoding.fix_encoding()
   sys.exit(TryChange(None, None, False))