Change git_cl.py to use subprocess2

git_cl.py

 #!/usr/bin/env python
 # Copyright (c) 2011 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.
 
 # Copyright (C) 2008 Evan Martin <martine@danga.com>
 
 """A git-command for integrating reviews on Rietveld."""
 
-import errno
 import logging
 import optparse
 import os
 import re
-import subprocess
 import sys
 import tempfile
 import textwrap
 import urlparse
 import urllib2
 
 try:
   import readline  # pylint: disable=F0401,W0611
 except ImportError:
   pass
 
 try:
   import simplejson as json  # pylint: disable=F0401
 except ImportError:
   try:
     import json  # pylint: disable=F0401
   except ImportError:
     # Fall back to the packaged version.
     sys.path.append(os.path.join(os.path.dirname(__file__), 'third_party'))
     import simplejson as json  # pylint: disable=F0401
 
 
 from third_party import upload
 import breakpad  # pylint: disable=W0611
 import fix_encoding
 import presubmit_support
 import rietveld
 import scm
+import subprocess2
 import watchlists
 
 
-
 DEFAULT_SERVER = 'http://codereview.appspot.com'
 POSTUPSTREAM_HOOK_PATTERN = '.git/hooks/post-cl-%s'
 DESCRIPTION_BACKUP_FILE = '~/.git_cl_description_backup'
 
 
 def DieWithError(message):
   print >> sys.stderr, message
   sys.exit(1)
 
 
-def Popen(cmd, **kwargs):
-  """Wrapper for subprocess.Popen() that logs and watch for cygwin issues"""
-  logging.debug('Popen: ' + ' '.join(cmd))
+def RunCommand(args, error_ok=False, error_message=None, **kwargs):
   try:
-    return subprocess.Popen(cmd, **kwargs)
-  except OSError, e:
-    if e.errno == errno.EAGAIN and sys.platform == 'cygwin':
+    return subprocess2.check_output(args, **kwargs)
+  except subprocess2.CalledProcessError, e:
+    if not error_ok:
       DieWithError(
-          'Visit '
-          'http://code.google.com/p/chromium/wiki/CygwinDllRemappingFailure to '
-          'learn how to fix this error; you need to rebase your cygwin dlls')
-    raise
-
-
-def RunCommand(cmd, error_ok=False, error_message=None,
-               redirect_stdout=True, swallow_stderr=False, **kwargs):
-  if redirect_stdout:
-    stdout = subprocess.PIPE
-  else:
-    stdout = None
-  if swallow_stderr:
-    stderr = subprocess.PIPE
-  else:
-    stderr = None
-  proc = Popen(cmd, stdout=stdout, stderr=stderr, **kwargs)
-  output = proc.communicate()[0]
-  if not error_ok and proc.returncode != 0:
-    DieWithError('Command "%s" failed.\n' % (' '.join(cmd)) +
-                 (error_message or output or ''))
-  return output
+          'Command "%s" failed.\n%s' % (
+            ' '.join(args), error_message or e.stdout or ''))
+    return e.stdout
 
 
 def RunGit(args, **kwargs):
-  cmd = ['git'] + args
-  return RunCommand(cmd, **kwargs)
+  """Returns stdout."""
+  return RunCommand(['git'] + args, **kwargs)
 
 
 def RunGitWithCode(args):
-  proc = Popen(['git'] + args, stdout=subprocess.PIPE)
-  output = proc.communicate()[0]
-  return proc.returncode, output
+  """Returns return code and stdout."""
+  out, code = subprocess2.communicate(['git'] + args, stdout=subprocess2.PIPE)
+  return code, out[0]
 
 
 def usage(more):
   def hook(fn):
     fn.usage_more = more
     return fn
   return hook
 
 
 def ask_for_data(prompt):
@@ skipping 47 matching lines @@
 
   # Parse specs like "trunk/src:refs/remotes/origin/trunk".
   if fetch_suburl:
     full_url = base_url + '/' + fetch_suburl
   else:
     full_url = base_url
   if full_url == url:
     return as_ref
   return None
 
+
 class Settings(object):
   def __init__(self):
     self.default_server = None
     self.cc = None
     self.root = None
     self.is_git_svn = None
     self.svn_branch = None
     self.tree_status_url = None
     self.viewvc_url = None
     self.updated = False
@@ skipping 43 matching lines @@
       # 1) iterate through our branch history and find the svn URL.
       # 2) find the svn-remote that fetches from the URL.
 
       # regexp matching the git-svn line that contains the URL.
       git_svn_re = re.compile(r'^\s*git-svn-id: (\S+)@', re.MULTILINE)
 
       # We don't want to go through all of history, so read a line from the
       # pipe at a time.
       # The -100 is an arbitrary limit so we don't search forever.
       cmd = ['git', 'log', '-100', '--pretty=medium']
-      proc = Popen(cmd, stdout=subprocess.PIPE)
+      proc = subprocess2.Popen(cmd, stdout=subprocess2.PIPE)
       url = None
       for line in proc.stdout:
         match = git_svn_re.match(line)
         if match:
           url = match.group(1)
           proc.stdout.close()  # Cut pipe.
           break
 
       if url:
         svn_remote_re = re.compile(r'^svn-remote\.([^.]+)\.url (.*)$')
@@ skipping 243 matching lines @@
         self.patchset = None
       self.has_patchset = True
     return self.patchset
 
   def SetPatchset(self, patchset):
     """Set this branch's patchset.  If patchset=0, clears the patchset."""
     if patchset:
       RunGit(['config', self._PatchsetSetting(), str(patchset)])
     else:
       RunGit(['config', '--unset', self._PatchsetSetting()],
-             swallow_stderr=True, error_ok=True)
+             stderr=subprocess2.PIPE, error_ok=True)
     self.has_patchset = False
 
   def GetPatchSetDiff(self, issue):
     patchset = self.RpcServer().get_issue_properties(
         int(issue), False)['patchsets'][-1]
     return self.RpcServer().get(
         '/download/issue%s_%s.diff' % (issue, patchset))
 
   def SetIssue(self, issue):
     """Set this branch's issue.  If issue=0, clears the issue."""
@@ skipping 340 matching lines @@
   fileobj.close()
 
   # Open up the default editor in the system to get the CL description.
   try:
     cmd = '%s %s' % (editor, filename)
     if sys.platform == 'win32' and os.environ.get('TERM') == 'msys':
       # Msysgit requires the usage of 'env' to be present.
       cmd = 'env ' + cmd
     # shell=True to allow the shell to handle all forms of quotes in $EDITOR.
     try:
-      subprocess.check_call(cmd, shell=True)
-    except subprocess.CalledProcessError, e:
+      subprocess2.check_call(cmd, shell=True)
+    except subprocess2.CalledProcessError, e:
       DieWithError('Editor returned %d' % e.returncode)
     fileobj = open(filename)
     text = fileobj.read()
     fileobj.close()
   finally:
     os.remove(filename)
 
   if not text:
     return
 
@@ skipping 82 matching lines @@
     if not options.reviewers and hook_results.reviewers:
       options.reviewers = hook_results.reviewers
 
 
   # --no-ext-diff is broken in some versions of Git, so try to work around
   # this by overriding the environment (but there is still a problem if the
   # git config key "diff.external" is used).
   env = os.environ.copy()
   if 'GIT_EXTERNAL_DIFF' in env:
     del env['GIT_EXTERNAL_DIFF']
-  subprocess.call(['git', 'diff', '--no-ext-diff', '--stat', '-M'] + args,
-                  env=env)
+  subprocess2.call(
+      ['git', 'diff', '--no-ext-diff', '--stat', '-M'] + args, env=env)
 
   upload_args = ['--assume_yes']  # Don't ask about untracked files.
   upload_args.extend(['--server', cl.GetRietveldServer()])
   if options.emulate_svn_auto_props:
     upload_args.append('--emulate_svn_auto_props')
   if options.send_mail:
     if not options.reviewers:
       DieWithError("Must specify reviewers to send email.")
     upload_args.append('--send_mail')
   if options.from_logs and not options.message:
@@ skipping 159 matching lines @@
 
   if cl.GetIssue():
     description += "\n\nReview URL: %s" % cl.GetIssueURL()
 
   if options.contributor:
     description += "\nPatch from %s." % options.contributor
   print 'Description:', repr(description)
 
   branches = [base_branch, cl.GetBranchRef()]
   if not options.force:
-    subprocess.call(['git', 'diff', '--stat'] + branches)
+    subprocess2.call(['git', 'diff', '--stat'] + branches)
     ask_for_data('About to commit; enter to confirm.')
 
   # We want to squash all this branch's commits into one commit with the
   # proper description.
   # We do this by doing a "reset --soft" to the base branch (which keeps
   # the working copy the same), then dcommitting that.
   MERGE_BRANCH = 'git-cl-commit'
   # Delete the merge branch if it already exists.
   if RunGitWithCode(['show-ref', '--quiet', '--verify',
                      'refs/heads/' + MERGE_BRANCH])[0] == 0:
@@ skipping 116 matching lines @@
     match = re.match(r'.*?/issue(\d+)_\d+.diff', issue_url)
     if not match:
       DieWithError('Must pass an issue ID or full URL for '
           '\'Download raw patch set\'')
     issue = match.group(1)
     patch_data = urllib2.urlopen(issue_arg).read()
 
   if options.newbranch:
     if options.force:
       RunGit(['branch', '-D', options.newbranch],
-         swallow_stderr=True, error_ok=True)
+          stderr=subprocess2.PIPE, error_ok=True)
     RunGit(['checkout', '-b', options.newbranch,
             Changelist().GetUpstreamBranch()])
 
   # Switch up to the top-level directory, if necessary, in preparation for
   # applying the patch.
   top = RunGit(['rev-parse', '--show-cdup']).strip()
   if top:
     os.chdir(top)
 
   # Git patches have a/ at the beginning of source paths.  We strip that out
   # with a sed script rather than the -p flag to patch so we can feed either
   # Git or svn-style patches into the same apply command.
   # re.sub() should be used but flags=re.MULTILINE is only in python 2.7.
-  sed_proc = Popen(['sed', '-e', 's|^--- a/|--- |; s|^+++ b/|+++ |'],
-      stdin=subprocess.PIPE, stdout=subprocess.PIPE)
-  patch_data = sed_proc.communicate(patch_data)[0]
-  if sed_proc.returncode:
+  try:
+    patch_data = subprocess2.check_output(
+        ['sed', '-e', 's|^--- a/|--- |; s|^+++ b/|+++ |'],
+        stdin=patch_data)
+  except subprocess2.CalledProcessError:
     DieWithError('Git patch mungling failed.')
   logging.info(patch_data)
   # We use "git apply" to apply the patch instead of "patch" so that we can
   # pick up file adds.
   # The --index flag means: also insert into the index (so we catch adds).
   cmd = ['git', 'apply', '--index', '-p0']
   if options.reject:
     cmd.append('--reject')
-  patch_proc = Popen(cmd, stdin=subprocess.PIPE)
-  patch_proc.communicate(patch_data)
-  if patch_proc.returncode:
+  try:
+    subprocess2.check_call(cmd, stdin=patch_data)
+  except subprocess2.CalledProcessError:
     DieWithError('Failed to apply the patch')
 
   # If we had an issue, commit the current state and register the issue.
   if not options.nocommit:
     RunGit(['commit', '-m', 'patch from issue %s' % issue])
     cl = Changelist()
     cl.SetIssue(issue)
     print "Committed patch."
   else:
     print "Patch applied to index."
   return 0
 
 
 def CMDrebase(parser, args):
   """rebase current branch on top of svn repo"""
   # Provide a wrapper for git svn rebase to help avoid accidental
   # git svn dcommit.
   # It's the only command that doesn't use parser at all since we just defer
   # execution to git-svn.
-  RunGit(['svn', 'rebase'] + args, redirect_stdout=False)
+  subprocess2.check_call(['git', 'svn', 'rebase'] + args)
   return 0
 
 
 def GetTreeStatus():
   """Fetches the tree status and returns either 'open', 'closed',
   'unknown' or 'unset'."""
   url = settings.GetTreeStatusUrl(error_ok=True)
   if url:
     status = urllib2.urlopen(url).read().lower()
     if status.find('closed') != -1 or status == '0':
@@ skipping 116 matching lines @@
              'and retry or visit go/isgaeup.\n%s') % (e.code, str(e)))
 
   # Not a known command. Default to help.
   GenUsage(parser, 'help')
   return CMDhelp(parser, argv)
 
 
 if __name__ == '__main__':
   fix_encoding.fix_encoding()
   sys.exit(main(sys.argv[1:]))