Revert r100190 "Change git_cl.py to use subprocess2"
Revert r100267 "git cl assumes no shell is used."
It's causing exceptions, r100190 still need some work.
TBR=dpranke@chromium.org
BUG=
TEST=
Review URL: http://codereview.chromium.org/7858029
git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@100390 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/git_cl.py b/git_cl.py
index ebb39c1..42afb63 100755
--- a/git_cl.py
+++ b/git_cl.py
@@ -7,10 +7,12 @@
"""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
@@ -39,10 +41,10 @@
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'
@@ -53,26 +55,47 @@
sys.exit(1)
-def RunCommand(args, error_ok=False, error_message=None, **kwargs):
+def Popen(cmd, **kwargs):
+ """Wrapper for subprocess.Popen() that logs and watch for cygwin issues"""
+ logging.debug('Popen: ' + ' '.join(cmd))
try:
- return subprocess2.check_output(args, shell=False, **kwargs)
- except subprocess2.CalledProcessError, e:
- if not error_ok:
+ return subprocess.Popen(cmd, **kwargs)
+ except OSError, e:
+ if e.errno == errno.EAGAIN and sys.platform == 'cygwin':
DieWithError(
- 'Command "%s" failed.\n%s' % (
- ' '.join(args), error_message or e.stdout or ''))
- return e.stdout
+ '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
def RunGit(args, **kwargs):
- """Returns stdout."""
- return RunCommand(['git'] + args, **kwargs)
+ cmd = ['git'] + args
+ return RunCommand(cmd, **kwargs)
def RunGitWithCode(args):
- """Returns return code and stdout."""
- out, code = subprocess2.communicate(['git'] + args, stdout=subprocess2.PIPE)
- return code, out[0]
+ proc = Popen(['git'] + args, stdout=subprocess.PIPE)
+ output = proc.communicate()[0]
+ return proc.returncode, output
def usage(more):
@@ -140,7 +163,6 @@
return as_ref
return None
-
class Settings(object):
def __init__(self):
self.default_server = None
@@ -204,7 +226,7 @@
# pipe at a time.
# The -100 is an arbitrary limit so we don't search forever.
cmd = ['git', 'log', '-100', '--pretty=medium']
- proc = subprocess2.Popen(cmd, stdout=subprocess2.PIPE)
+ proc = Popen(cmd, stdout=subprocess.PIPE)
url = None
for line in proc.stdout:
match = git_svn_re.match(line)
@@ -468,7 +490,7 @@
RunGit(['config', self._PatchsetSetting(), str(patchset)])
else:
RunGit(['config', '--unset', self._PatchsetSetting()],
- stderr=subprocess2.PIPE, error_ok=True)
+ swallow_stderr=True, error_ok=True)
self.has_patchset = False
def GetPatchSetDiff(self, issue):
@@ -829,8 +851,8 @@
cmd = 'env ' + cmd
# shell=True to allow the shell to handle all forms of quotes in $EDITOR.
try:
- subprocess2.check_call(cmd, shell=True)
- except subprocess2.CalledProcessError, e:
+ subprocess.check_call(cmd, shell=True)
+ except subprocess.CalledProcessError, e:
DieWithError('Editor returned %d' % e.returncode)
fileobj = open(filename)
text = fileobj.read()
@@ -933,8 +955,8 @@
env = os.environ.copy()
if 'GIT_EXTERNAL_DIFF' in env:
del env['GIT_EXTERNAL_DIFF']
- subprocess2.call(
- ['git', 'diff', '--no-ext-diff', '--stat', '-M'] + args, env=env)
+ subprocess.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()])
@@ -1114,7 +1136,7 @@
branches = [base_branch, cl.GetBranchRef()]
if not options.force:
- subprocess2.call(['git', 'diff', '--stat'] + branches)
+ subprocess.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
@@ -1251,7 +1273,7 @@
if options.newbranch:
if options.force:
RunGit(['branch', '-D', options.newbranch],
- stderr=subprocess2.PIPE, error_ok=True)
+ swallow_stderr=True, error_ok=True)
RunGit(['checkout', '-b', options.newbranch,
Changelist().GetUpstreamBranch()])
@@ -1265,11 +1287,10 @@
# 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.
- try:
- patch_data = subprocess2.check_output(
- ['sed', '-e', 's|^--- a/|--- |; s|^+++ b/|+++ |'],
- stdin=patch_data)
- except subprocess2.CalledProcessError:
+ 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:
DieWithError('Git patch mungling failed.')
logging.info(patch_data)
# We use "git apply" to apply the patch instead of "patch" so that we can
@@ -1278,9 +1299,9 @@
cmd = ['git', 'apply', '--index', '-p0']
if options.reject:
cmd.append('--reject')
- try:
- subprocess2.check_call(cmd, stdin=patch_data)
- except subprocess2.CalledProcessError:
+ patch_proc = Popen(cmd, stdin=subprocess.PIPE)
+ patch_proc.communicate(patch_data)
+ if patch_proc.returncode:
DieWithError('Failed to apply the patch')
# If we had an issue, commit the current state and register the issue.
@@ -1300,7 +1321,7 @@
# git svn dcommit.
# It's the only command that doesn't use parser at all since we just defer
# execution to git-svn.
- subprocess2.check_call(['git', 'svn', 'rebase'] + args)
+ RunGit(['svn', 'rebase'] + args, redirect_stdout=False)
return 0