From d4b4e11328d979a4a1a89e7db449101505cb98a2 Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Fri, 9 Sep 2011 11:44:10 +0000 Subject: [PATCH] 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 --- git_cl.py | 83 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 52 insertions(+), 31 deletions(-) diff --git a/git_cl.py b/git_cl.py index ebb39c1ac2..42afb630d7 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 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' @@ -53,26 +55,47 @@ def DieWithError(message): 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 @@ def MatchSvnGlob(url, base_url, glob_spec, allow_wildcards): return as_ref return None - class Settings(object): def __init__(self): self.default_server = None @@ -204,7 +226,7 @@ class Settings(object): # 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 @@ or verify this branch is set up to track another (via the --track argument to 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 @@ def UserEditedLog(starting_text): 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 @@ def CMDupload(parser, args): 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 @@ def SendUpstream(parser, args, cmd): 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 @@ def CMDpatch(parser, args): 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 @@ def CMDpatch(parser, args): # 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 @@ def CMDpatch(parser, args): 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 @@ def CMDrebase(parser, args): # 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