From 97335088c69362e6b9558d43767bead0e7dd2189 Mon Sep 17 00:00:00 2001 From: "maruel@chromium.org" Date: Thu, 20 Oct 2011 15:00:16 +0000 Subject: [PATCH] Revert r106358 "Get rid of RunShell*() functions in gcl.py to finish the conversion to subprocess2" This eats stdout while gcl is committing, which is annoying. It needs tee-like support first. Will revisit this change later, reverting in the meantime. TBR=dpranke@chromium.org BUG= TEST= Review URL: http://codereview.chromium.org/8344085 git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@106498 0039d316-1c4b-4281-b951-d872f2087c98 --- gcl.py | 66 ++++++++++++++++++++++++++++--------------- tests/gcl_unittest.py | 11 ++++---- 2 files changed, 48 insertions(+), 29 deletions(-) diff --git a/gcl.py b/gcl.py index 171017e12..8df5de1bc 100755 --- a/gcl.py +++ b/gcl.py @@ -38,7 +38,7 @@ from scm import SVN import subprocess2 from third_party import upload -__version__ = '1.2.2' +__version__ = '1.2.1' CODEREVIEW_SETTINGS = { @@ -69,7 +69,6 @@ DEFAULT_LINT_IGNORE_REGEX = r"$^" REVIEWERS_REGEX = r'\s*R=(.+)' - def CheckHomeForFile(filename): """Checks the users home dir for the existence of the given file. Returns the path to the file if it's there, or None if it is not. @@ -228,6 +227,33 @@ def ErrorExit(msg): sys.exit(1) +def RunShellWithReturnCode(command, print_output=False): + """Executes a command and returns the output and the return code.""" + p = subprocess2.Popen( + command, stdout=subprocess2.PIPE, + stderr=subprocess2.STDOUT, universal_newlines=True) + if print_output: + output_array = [] + while True: + line = p.stdout.readline() + if not line: + break + if print_output: + print line.strip('\n') + output_array.append(line) + output = "".join(output_array) + else: + output = p.stdout.read() + p.wait() + p.stdout.close() + return output, p.returncode + + +def RunShell(command, print_output=False): + """Executes a command and returns the output.""" + return RunShellWithReturnCode(command, print_output)[0] + + def FilterFlag(args, flag): """Returns True if the flag is present in args list. @@ -988,25 +1014,19 @@ def CMDcommit(change_info, args): handle, commit_filename = tempfile.mkstemp(text=True) os.write(handle, commit_message) os.close(handle) - try: - handle, targets_filename = tempfile.mkstemp(text=True) - os.write(handle, "\n".join(change_info.GetFileNames())) - os.close(handle) - try: - commit_cmd += ['--file=' + commit_filename] - commit_cmd += ['--targets=' + targets_filename] - # Change the current working directory before calling commit. - previous_cwd = os.getcwd() - os.chdir(change_info.GetLocalRoot()) - output = '' - try: - output = subprocess2.check_output(commit_cmd) - except subprocess2.CalledProcessError, e: - ErrorExit('Commit failed.\n%s' % e) - finally: - os.remove(commit_filename) - finally: - os.remove(targets_filename) + + handle, targets_filename = tempfile.mkstemp(text=True) + os.write(handle, "\n".join(change_info.GetFileNames())) + os.close(handle) + + commit_cmd += ['--file=' + commit_filename] + commit_cmd += ['--targets=' + targets_filename] + # Change the current working directory before calling commit. + previous_cwd = os.getcwd() + os.chdir(change_info.GetLocalRoot()) + output = RunShell(commit_cmd, True) + os.remove(commit_filename) + os.remove(targets_filename) if output.find("Committed revision") != -1: change_info.Delete() @@ -1284,7 +1304,7 @@ def CMDdiff(args): cmd = ['svn', 'diff'] cmd.extend([os.path.join(root, x) for x in files]) cmd.extend(args) - return subprocess2.call(cmd) + return RunShellWithReturnCode(cmd, print_output=True)[1] @no_args @@ -1367,7 +1387,7 @@ def CMDpassthru(args): root = GetRepositoryRoot() change_info = ChangeInfo.Load(args[1], root, True, True) args.extend([os.path.join(root, x) for x in change_info.GetFileNames()]) - return subprocess2.call(args) + return RunShellWithReturnCode(args, print_output=True)[1] def Command(name): diff --git a/tests/gcl_unittest.py b/tests/gcl_unittest.py index 5b3893478..e013df2e2 100755 --- a/tests/gcl_unittest.py +++ b/tests/gcl_unittest.py @@ -20,8 +20,7 @@ class GclTestsBase(SuperMoxTestBase): def setUp(self): SuperMoxTestBase.setUp(self) self.fake_root_dir = self.RootDir() - self.mox.StubOutWithMock(gcl.subprocess2, 'call') - self.mox.StubOutWithMock(gcl.subprocess2, 'check_output') + self.mox.StubOutWithMock(gcl, 'RunShell') self.mox.StubOutWithMock(gcl.SVN, 'CaptureInfo') self.mox.StubOutWithMock(gcl.SVN, 'GetCheckoutRoot') self.mox.StubOutWithMock(gcl, 'tempfile') @@ -89,7 +88,8 @@ class GclUnittest(GclTestsBase): 'GetModifiedFiles', 'GetRepositoryRoot', 'ListFiles', 'LoadChangelistInfoForMultiple', 'MISSING_TEST_MSG', 'OptionallyDoPresubmitChecks', 'REPOSITORY_ROOT', 'REVIEWERS_REGEX', - 'SVN', 'TryChange', 'UnknownFiles', 'Warn', + 'RunShell', 'RunShellWithReturnCode', 'SVN', + 'TryChange', 'UnknownFiles', 'Warn', 'attrs', 'breakpad', 'defer_attributes', 'fix_encoding', 'gclient_utils', 'json', 'main', 'need_change', 'need_change_and_args', 'no_args', 'optparse', 'os', 'presubmit_support', 'random', 're', @@ -527,9 +527,8 @@ class CMDCommitUnittest(GclTestsBase): gcl.os.getcwd().AndReturn('prev') gcl.os.chdir(change_info.GetLocalRoot()) - gcl.subprocess2.check_output( - ['svn', 'commit', '--file=commit', '--targets=files'] - ).AndReturn(shell_output) + gcl.RunShell(['svn', 'commit', '--file=commit', '--targets=files'], + True).AndReturn(shell_output) if 'Committed' in shell_output: self.mox.StubOutWithMock(gcl, 'GetCodeReviewSetting') gcl.GetCodeReviewSetting('VIEW_VC').AndReturn('http://view/')