Commit 24146be8 authored by Edward Lemur's avatar Edward Lemur Committed by Commit Bot

depot_tools: Simplify CheckCallAndFilter[AndHeader]

- Merge CheckCallAndFilter[AndHeader]
- print_stdout will cause command output to be redirected to sys.stdout.
- Made compatible with Python 3.

Bug: 984182
Change-Id: Ida30e295b872c8c1a1474a376a90aecea924f307
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1727404
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: 's avatarRobbie Iannucci <iannucci@chromium.org>
parent 447b45d4
......@@ -252,8 +252,9 @@ class Hook(object):
try:
start_time = time.time()
gclient_utils.CheckCallAndFilterAndHeader(
cmd, cwd=self.effective_cwd, always=self._verbose)
gclient_utils.CheckCallAndFilter(
cmd, cwd=self.effective_cwd, print_stdout=True, show_header=True,
always_show_header=self._verbose)
except (gclient_utils.Error, subprocess2.CalledProcessError) as e:
# Use a discrete exit status code of 2 to indicate that a hook action
# failed. Users of this script may wish to treat hook action failures
......
......@@ -912,7 +912,7 @@ class GitWrapper(SCMWrapper):
merge_base = []
self._Run(
['-c', 'core.quotePath=false', 'diff', '--name-status'] + merge_base,
options, stdout=self.out_fh, always=options.verbose)
options, always_show_header=options.verbose)
if file_list is not None:
files = self._GetDiffFilenames(merge_base[0] if merge_base else None)
file_list.extend([os.path.join(self.checkout_path, f) for f in files])
......@@ -1037,12 +1037,12 @@ class GitWrapper(SCMWrapper):
clone_cmd.append(tmp_dir)
if self.print_outbuf:
print_stdout = True
stdout = gclient_utils.WriteToStdout(self.out_fh)
filter_fn = None
else:
print_stdout = False
stdout = self.out_fh
filter_fn = self.filter
self._Run(clone_cmd, options, cwd=self._root_dir, retry=True,
print_stdout=print_stdout, stdout=stdout)
print_stdout=print_stdout, filter_fn=filter_fn)
gclient_utils.safe_makedirs(self.checkout_path)
gclient_utils.safe_rename(os.path.join(tmp_dir, '.git'),
os.path.join(self.checkout_path, '.git'))
......@@ -1358,18 +1358,14 @@ class GitWrapper(SCMWrapper):
revision = self._Capture(['rev-parse', 'FETCH_HEAD'])
return revision
def _Run(self, args, options, show_header=True, **kwargs):
def _Run(self, args, options, **kwargs):
# Disable 'unused options' warning | pylint: disable=unused-argument
kwargs.setdefault('cwd', self.checkout_path)
kwargs.setdefault('stdout', self.out_fh)
kwargs['filter_fn'] = self.filter
kwargs.setdefault('print_stdout', False)
kwargs.setdefault('filter_fn', self.filter)
kwargs.setdefault('show_header', True)
env = scm.GIT.ApplyEnvVars(kwargs)
cmd = ['git'] + args
if show_header:
gclient_utils.CheckCallAndFilterAndHeader(cmd, env=env, **kwargs)
else:
gclient_utils.CheckCallAndFilter(cmd, env=env, **kwargs)
gclient_utils.CheckCallAndFilter(cmd, env=env, **kwargs)
class CipdPackage(object):
......@@ -1480,7 +1476,8 @@ class CipdRoot(object):
'-root', self.root_dir,
'-ensure-file', ensure_file,
]
gclient_utils.CheckCallAndFilterAndHeader(cmd)
gclient_utils.CheckCallAndFilter(
cmd, print_stdout=True, show_header=True)
def run(self, command):
if command == 'update':
......@@ -1565,8 +1562,7 @@ class CipdWrapper(SCMWrapper):
'-version', self._package.version,
'-json-output', describe_json_path
]
gclient_utils.CheckCallAndFilter(
cmd, filter_fn=lambda _line: None, print_stdout=False)
gclient_utils.CheckCallAndFilter(cmd)
with open(describe_json_path) as f:
describe_json = json.load(f)
return describe_json.get('result', {}).get('pin', {}).get('instance_id')
......
......@@ -312,38 +312,6 @@ def CommandToStr(args):
return ' '.join(pipes.quote(arg) for arg in args)
def CheckCallAndFilterAndHeader(args, always=False, header=None, **kwargs):
"""Adds 'header' support to CheckCallAndFilter.
If |always| is True, a message indicating what is being done
is printed to stdout all the time even if not output is generated. Otherwise
the message header is printed only if the call generated any ouput.
"""
stdout = kwargs.setdefault('stdout', sys.stdout)
if header is None:
# The automatically generated header only prepends newline if always is
# false: always is usually set to false if there's an external progress
# display, and it's better not to clobber it in that case.
header = "%s________ running '%s' in '%s'\n" % (
'' if always else '\n',
' '.join(args), kwargs.get('cwd', '.'))
if always:
stdout.write(header)
else:
filter_fn = kwargs.get('filter_fn')
def filter_msg(line):
if line is None:
stdout.write(header)
elif filter_fn:
filter_fn(line)
kwargs['filter_fn'] = filter_msg
kwargs['call_filter_on_first_line'] = True
# Obviously.
kwargs.setdefault('print_stdout', True)
return CheckCallAndFilter(args, **kwargs)
class Wrapper(object):
"""Wraps an object, acting as a transparent proxy for all properties by
default.
......@@ -355,22 +323,6 @@ class Wrapper(object):
return getattr(self._wrapped, name)
class WriteToStdout(Wrapper):
"""Creates a file object clone to also print to sys.stdout."""
def __init__(self, wrapped):
super(WriteToStdout, self).__init__(wrapped)
if not hasattr(self, 'lock'):
self.lock = threading.Lock()
def write(self, out, *args, **kwargs):
self._wrapped.write(out, *args, **kwargs)
self.lock.acquire()
try:
sys.stdout.write(out, *args, **kwargs)
finally:
self.lock.release()
class AutoFlush(Wrapper):
"""Creates a file object clone to automatically flush after N seconds."""
def __init__(self, wrapped, delay):
......@@ -536,9 +488,9 @@ class GClientChildren(object):
print(' ', zombie.pid, file=sys.stderr)
def CheckCallAndFilter(args, stdout=None, filter_fn=None,
print_stdout=None, call_filter_on_first_line=False,
retry=False, **kwargs):
def CheckCallAndFilter(args, print_stdout=False, filter_fn=None,
show_header=False, always_show_header=False, retry=False,
**kwargs):
"""Runs a command and calls back a filter function if needed.
Accepts all subprocess2.Popen() parameters plus:
......@@ -546,28 +498,68 @@ def CheckCallAndFilter(args, stdout=None, filter_fn=None,
filter_fn: A function taking a single string argument called with each line
of the subprocess2's output. Each line has the trailing newline
character trimmed.
stdout: Can be any bufferable output.
show_header: Whether to display a header before the command output.
always_show_header: Show header even when the command produced no output.
retry: If the process exits non-zero, sleep for a brief interval and try
again, up to RETRY_MAX times.
stderr is always redirected to stdout.
Returns the output of the command as a binary string.
"""
assert print_stdout or filter_fn
stdout = stdout or sys.stdout
output = io.BytesIO()
filter_fn = filter_fn or (lambda x: None)
def show_header_if_necessary(needs_header, attempt):
"""Show the header at most once."""
if not needs_header[0]:
return
needs_header[0] = False
# Automatically generated header. We only prepend a newline if
# always_show_header is false, since it usually indicates there's an
# external progress display, and it's better not to clobber it in that case.
header = '' if always_show_header else '\n'
header += '________ running \'%s\' in \'%s\'' % (
' '.join(args), kwargs.get('cwd', '.'))
if attempt:
header += ' attempt %s / %s' % (attempt + 1, RETRY_MAX + 1)
header += '\n'
if print_stdout:
sys.stdout.write(header)
if filter_fn:
filter_fn(header)
def filter_line(command_output, line_start):
"""Extract the last line from command output and filter it."""
if not filter_fn or line_start is None:
return
command_output.seek(line_start)
filter_fn(command_output.read().decode('utf-8'))
# Initialize stdout writer if needed. On Python 3, sys.stdout does not accept
# byte inputs and sys.stdout.buffer must be used instead.
if print_stdout:
sys.stdout.flush()
stdout_write = getattr(sys.stdout, 'buffer', sys.stdout).write
else:
stdout_write = lambda _: None
sleep_interval = RETRY_INITIAL_SLEEP
run_cwd = kwargs.get('cwd', os.getcwd())
for _ in range(RETRY_MAX + 1):
for attempt in range(RETRY_MAX + 1):
kid = subprocess2.Popen(
args, bufsize=0, stdout=subprocess2.PIPE, stderr=subprocess2.STDOUT,
**kwargs)
GClientChildren.add(kid)
# Do a flush of stdout before we begin reading from the subprocess2's stdout
stdout.flush()
# Store the output of the command regardless of the value of print_stdout or
# filter_fn.
command_output = io.BytesIO()
# Passed as a list for "by ref" semantics.
needs_header = [show_header]
if always_show_header:
show_header_if_necessary(needs_header, attempt)
# Also, we need to forward stdout to prevent weird re-ordering of output.
# This has to be done on a per byte basis to make sure it is not buffered:
......@@ -575,25 +567,29 @@ def CheckCallAndFilter(args, stdout=None, filter_fn=None,
# input, no end-of-line character is output after the prompt and it would
# not show up.
try:
in_byte = kid.stdout.read(1)
if in_byte:
if call_filter_on_first_line:
filter_fn(None)
in_line = b''
while in_byte:
output.write(in_byte)
if print_stdout:
stdout.write(in_byte)
if in_byte not in ['\r', '\n']:
in_line += in_byte
else:
filter_fn(in_line)
in_line = b''
in_byte = kid.stdout.read(1)
# Flush the rest of buffered output. This is only an issue with
# stdout/stderr not ending with a \n.
if len(in_line):
filter_fn(in_line)
line_start = None
while True:
in_byte = kid.stdout.read(1)
is_newline = in_byte in (b'\n', b'\r')
if not in_byte:
break
show_header_if_necessary(needs_header, attempt)
if is_newline:
filter_line(command_output, line_start)
line_start = None
elif line_start is None:
line_start = command_output.tell()
stdout_write(in_byte)
command_output.write(in_byte)
# Flush the rest of buffered output.
sys.stdout.flush()
if line_start is not None:
filter_line(command_output, line_start)
rv = kid.wait()
kid.stdout.close()
......@@ -606,13 +602,16 @@ def CheckCallAndFilter(args, stdout=None, filter_fn=None,
raise
if rv == 0:
return output.getvalue()
return command_output.getvalue()
if not retry:
break
print("WARNING: subprocess '%s' in %s failed; will retry after a short "
'nap...' % (' '.join('"%s"' % x for x in args), run_cwd))
time.sleep(sleep_interval)
sleep_interval *= 2
raise subprocess2.CalledProcessError(
rv, args, kwargs.get('cwd', None), None, None)
......@@ -635,6 +634,7 @@ class GitFilter(object):
The line will be skipped if predicate(line) returns False.
out_fh: File handle to write output to.
"""
self.first_line = True
self.last_time = 0
self.time_throttle = time_throttle
self.predicate = predicate
......@@ -656,7 +656,9 @@ class GitFilter(object):
elif now - self.last_time < self.time_throttle:
return
self.last_time = now
self.out_fh.write('[%s] ' % Elapsed())
if not self.first_line:
self.out_fh.write('[%s] ' % Elapsed())
self.first_line = False
print(line, file=self.out_fh)
......
......@@ -60,8 +60,6 @@ class BaseTestCase(GCBaseTestCase, SuperMoxTestBase):
def setUp(self):
SuperMoxTestBase.setUp(self)
self.mox.StubOutWithMock(gclient_scm.gclient_utils, 'CheckCallAndFilter')
self.mox.StubOutWithMock(gclient_scm.gclient_utils,
'CheckCallAndFilterAndHeader')
self.mox.StubOutWithMock(gclient_scm.gclient_utils, 'FileRead')
self.mox.StubOutWithMock(gclient_scm.gclient_utils, 'FileWrite')
self.mox.StubOutWithMock(gclient_scm.gclient_utils, 'rmtree')
......@@ -347,7 +345,7 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase):
self.assertEquals(file_list, [file_path])
self.checkstdout(
('\n________ running \'git -c core.quotePath=false diff --name-status '
'069c602044c5388d2d15c3f875b057c852003458\' in \'%s\'\nM\ta\n') %
'069c602044c5388d2d15c3f875b057c852003458\' in \'%s\'\n\nM\ta\n') %
join(self.root_dir, '.'))
def testStatus2New(self):
......@@ -367,8 +365,8 @@ class ManagedGitWrapperTestCase(BaseGitWrapperTestCase):
self.assertEquals(sorted(file_list), expected_file_list)
self.checkstdout(
('\n________ running \'git -c core.quotePath=false diff --name-status '
'069c602044c5388d2d15c3f875b057c852003458\' in \'%s\'\nM\ta\nM\tb\n') %
join(self.root_dir, '.'))
'069c602044c5388d2d15c3f875b057c852003458\' in \'%s\'\n\nM\ta\nM\tb\n')
% join(self.root_dir, '.'))
def testUpdateUpdate(self):
if not self.enabled:
......@@ -1024,8 +1022,7 @@ class CipdWrapperTestCase(BaseTestCase):
self.mox.StubOutWithMock(tempfile, 'mkdtemp')
tempfile.mkdtemp().AndReturn(self._workdir)
gclient_scm.gclient_utils.CheckCallAndFilter(
cmd, filter_fn=mox.IgnoreArg(), print_stdout=False)
gclient_scm.gclient_utils.CheckCallAndFilter(cmd)
gclient_scm.gclient_utils.rmtree(self._workdir)
self.mox.ReplayAll()
......
......@@ -4,8 +4,10 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
from __future__ import unicode_literals
import io
import os
import StringIO
import sys
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
......@@ -27,19 +29,19 @@ class GclientUtilBase(SuperMoxTestBase):
class CheckCallAndFilterTestCase(GclientUtilBase):
class ProcessIdMock(object):
def __init__(self, test_string):
self.stdout = StringIO.StringIO(test_string)
def __init__(self, test_string, return_code=0):
self.stdout = io.BytesIO(test_string.encode('utf-8'))
self.pid = 9284
# pylint: disable=no-self-use
self.return_code = return_code
def wait(self):
return 0
return self.return_code
def _inner(self, args, test_string):
def testCheckCallAndFilter(self):
cwd = 'bleh'
gclient_utils.sys.stdout.write(
'________ running \'boo foo bar\' in \'bleh\'\n')
for i in test_string:
gclient_utils.sys.stdout.write(i)
args = ['boo', 'foo', 'bar']
test_string = 'ahah\naccb\nallo\naddb\n✔'
# pylint: disable=no-member
subprocess2.Popen(
args,
......@@ -50,30 +52,70 @@ class CheckCallAndFilterTestCase(GclientUtilBase):
os.getcwd()
self.mox.ReplayAll()
compiled_pattern = gclient_utils.re.compile(r'a(.*)b')
line_list = []
capture_list = []
def FilterLines(line):
line_list.append(line)
assert isinstance(line, str), type(line)
match = compiled_pattern.search(line)
if match:
capture_list.append(match.group(1))
gclient_utils.CheckCallAndFilterAndHeader(
args, cwd=cwd, always=True, filter_fn=FilterLines)
self.assertEquals(line_list, ['ahah', 'accb', 'allo', 'addb', '✔'])
self.assertEquals(capture_list, ['cc', 'dd'])
def testCheckCallAndFilter(self):
result = gclient_utils.CheckCallAndFilter(
args, cwd=cwd, show_header=True, always_show_header=True,
filter_fn=line_list.append)
self.assertEqual(result, test_string.encode('utf-8'))
self.assertEqual(line_list, [
'________ running \'boo foo bar\' in \'bleh\'\n',
'ahah',
'accb',
'allo',
'addb',
'✔'])
def testCheckCallAndFilter_RetryOnce(self):
cwd = 'bleh'
args = ['boo', 'foo', 'bar']
test_string = 'ahah\naccb\nallo\naddb\n\n'
self._inner(args, test_string)
test_string = 'ahah\naccb\nallo\naddb\n✔'
# pylint: disable=no-member
subprocess2.Popen(
args,
cwd=cwd,
stdout=subprocess2.PIPE,
stderr=subprocess2.STDOUT,
bufsize=0).AndReturn(self.ProcessIdMock(test_string, 1))
os.getcwd()
# pylint: disable=no-member
subprocess2.Popen(
args,
cwd=cwd,
stdout=subprocess2.PIPE,
stderr=subprocess2.STDOUT,
bufsize=0).AndReturn(self.ProcessIdMock(test_string, 0))
self.mox.ReplayAll()
line_list = []
result = gclient_utils.CheckCallAndFilter(
args, cwd=cwd, show_header=True, always_show_header=True,
filter_fn=line_list.append, retry=True)
self.assertEqual(result, test_string.encode('utf-8'))
self.assertEqual(line_list, [
'________ running \'boo foo bar\' in \'bleh\'\n',
'ahah',
'accb',
'allo',
'addb',
'✔',
'________ running \'boo foo bar\' in \'bleh\' attempt 2 / 4\n',
'ahah',
'accb',
'allo',
'addb',
'✔',
])
self.checkstdout(
'________ running \'boo foo bar\' in \'bleh\'\n'
'ahah\naccb\nallo\naddb\n\n'
'________ running \'boo foo bar\' in \'bleh\'\n'
'ahah\naccb\nallo\naddb\n✔'
'\n')
'WARNING: subprocess \'"boo" "foo" "bar"\' in bleh failed; will retry '
'after a short nap...\n')
class SplitUrlRevisionTestCase(GclientUtilBase):
......@@ -81,60 +123,60 @@ class SplitUrlRevisionTestCase(GclientUtilBase):
url = "ssh://test@example.com/test.git"
rev = "ac345e52dc"
out_url, out_rev = gclient_utils.SplitUrlRevision(url)
self.assertEquals(out_rev, None)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, None)
self.assertEqual(out_url, url)
out_url, out_rev = gclient_utils.SplitUrlRevision("%s@%s" % (url, rev))
self.assertEquals(out_rev, rev)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, rev)
self.assertEqual(out_url, url)
url = "ssh://example.com/test.git"
out_url, out_rev = gclient_utils.SplitUrlRevision(url)
self.assertEquals(out_rev, None)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, None)
self.assertEqual(out_url, url)
out_url, out_rev = gclient_utils.SplitUrlRevision("%s@%s" % (url, rev))
self.assertEquals(out_rev, rev)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, rev)
self.assertEqual(out_url, url)
url = "ssh://example.com/git/test.git"
out_url, out_rev = gclient_utils.SplitUrlRevision(url)
self.assertEquals(out_rev, None)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, None)
self.assertEqual(out_url, url)
out_url, out_rev = gclient_utils.SplitUrlRevision("%s@%s" % (url, rev))
self.assertEquals(out_rev, rev)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, rev)
self.assertEqual(out_url, url)
rev = "test-stable"
out_url, out_rev = gclient_utils.SplitUrlRevision("%s@%s" % (url, rev))
self.assertEquals(out_rev, rev)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, rev)
self.assertEqual(out_url, url)
url = "ssh://user-name@example.com/~/test.git"
out_url, out_rev = gclient_utils.SplitUrlRevision(url)
self.assertEquals(out_rev, None)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, None)
self.assertEqual(out_url, url)
out_url, out_rev = gclient_utils.SplitUrlRevision("%s@%s" % (url, rev))
self.assertEquals(out_rev, rev)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, rev)
self.assertEqual(out_url, url)
url = "ssh://user-name@example.com/~username/test.git"
out_url, out_rev = gclient_utils.SplitUrlRevision(url)
self.assertEquals(out_rev, None)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, None)
self.assertEqual(out_url, url)
out_url, out_rev = gclient_utils.SplitUrlRevision("%s@%s" % (url, rev))
self.assertEquals(out_rev, rev)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, rev)
self.assertEqual(out_url, url)
url = "git@github.com:dart-lang/spark.git"
out_url, out_rev = gclient_utils.SplitUrlRevision(url)
self.assertEquals(out_rev, None)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, None)
self.assertEqual(out_url, url)
out_url, out_rev = gclient_utils.SplitUrlRevision("%s@%s" % (url, rev))
self.assertEquals(out_rev, rev)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, rev)
self.assertEqual(out_url, url)
def testSVNUrl(self):
url = "svn://example.com/test"
rev = "ac345e52dc"
out_url, out_rev = gclient_utils.SplitUrlRevision(url)
self.assertEquals(out_rev, None)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, None)
self.assertEqual(out_url, url)
out_url, out_rev = gclient_utils.SplitUrlRevision("%s@%s" % (url, rev))
self.assertEquals(out_rev, rev)
self.assertEquals(out_url, url)
self.assertEqual(out_rev, rev)
self.assertEqual(out_url, url)
class GClientUtilsTest(trial_dir.TestCase):
......@@ -171,7 +213,7 @@ class GClientUtilsTest(trial_dir.TestCase):
['foo:', 'https://foo:'],
]
for content, expected in values:
self.assertEquals(
self.assertEqual(
expected, gclient_utils.UpgradeToHttps(content))
def testParseCodereviewSettingsContent(self):
......@@ -191,7 +233,7 @@ class GClientUtilsTest(trial_dir.TestCase):
['VIEW_VC:http://r/s', {'VIEW_VC': 'https://r/s'}],
]
for content, expected in values:
self.assertEquals(
self.assertEqual(
expected, gclient_utils.ParseCodereviewSettingsContent(content))
......
......@@ -37,8 +37,6 @@ class BaseTestCase(SuperMoxTestBase):
class BaseSCMTestCase(BaseTestCase):
def setUp(self):
BaseTestCase.setUp(self)
self.mox.StubOutWithMock(scm.gclient_utils, 'CheckCallAndFilter')
self.mox.StubOutWithMock(scm.gclient_utils, 'CheckCallAndFilterAndHeader')
self.mox.StubOutWithMock(subprocess2, 'Popen')
self.mox.StubOutWithMock(subprocess2, 'communicate')
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment