Commit f61de2f1 authored by dpranke's avatar dpranke Committed by Commit bot

Use a response file for gn refs when running 'analyze' on gn bots.

Previously we would invoke 'gn refs' once for each file modified in
a CL; this was terribly inefficient. Switching to a response file
makes things run much faster. This is a precursor to fixing
the actual problem in bug 487035, because we will be doubling the
number of times we need to call gn refs in the fix.

R=scottmg@chromium.org
BUG=487035
CQ_EXTRA_TRYBOTS=tryserver.chromium.mac:mac_chromium_gn_rel;tryserver.chromium.win:win8_chromium_gn_rel

Review URL: https://codereview.chromium.org/1131623006

Cr-Commit-Position: refs/heads/master@{#329799}
parent ab29c168
...@@ -20,6 +20,7 @@ import shlex ...@@ -20,6 +20,7 @@ import shlex
import shutil import shutil
import sys import sys
import subprocess import subprocess
import tempfile
def main(args): def main(args):
...@@ -415,36 +416,41 @@ class MetaBuildWrapper(object): ...@@ -415,36 +416,41 @@ class MetaBuildWrapper(object):
self.WriteJSON({'status': 'Found dependency (all)'}, output_path) self.WriteJSON({'status': 'Found dependency (all)'}, output_path)
return 0 return 0
# TODO: Because of the --type=executable filter below, we don't detect # Bail out early if 'all' was asked for, since 'gn refs' won't recognize it.
# when files will cause 'all' or 'gn_all' or similar targets to be if 'all' in inp['targets']:
# dirty. We need to figure out how to handle that properly, but for
# now we can just bail out early.
if 'gn_all' in inp['targets'] or 'all' in inp['targets']:
self.WriteJSON({'status': 'Found dependency (all)'}, output_path) self.WriteJSON({'status': 'Found dependency (all)'}, output_path)
return 0 return 0
all_needed_targets = set()
ret = 0 ret = 0
for f in inp['files']: response_file = self.TempFile()
response_file.write('\n'.join(inp['files']) + '\n')
response_file.close()
matching_targets = []
try:
cmd = self.GNCmd('refs', self.args.path[0]) + [ cmd = self.GNCmd('refs', self.args.path[0]) + [
'//' + f, '--type=executable', '--all', '--as=output'] '@%s' % response_file.name,
'--type=executable', '--all', '--as=output'
]
ret, out, _ = self.Run(cmd) ret, out, _ = self.Run(cmd)
if ret and not 'The input matches no targets' in out: if ret and not 'The input matches no targets' in out:
self.WriteFailureAndRaise('gn refs returned %d: %s' % (ret, out), self.WriteFailureAndRaise('gn refs returned %d: %s' % (ret, out),
output_path) output_path)
build_dir = self.ToSrcRelPath(self.args.path[0]) + os.sep
rpath = self.ToSrcRelPath(self.args.path[0]) + os.sep for output in out.splitlines():
needed_targets = [t.replace(rpath, '') for t in out.splitlines()] build_output = output.replace(build_dir, '')
needed_targets = [nt for nt in needed_targets if nt in inp['targets']] if build_output in inp['targets']:
all_needed_targets.update(set(needed_targets)) matching_targets.append(build_output)
finally:
if all_needed_targets: self.RemoveFile(response_file.name)
if matching_targets:
# TODO: it could be that a target X might depend on a target Y # TODO: it could be that a target X might depend on a target Y
# and both would be listed in the input, but we would only need # and both would be listed in the input, but we would only need
# to specify target X as a build_target (whereas both X and Y are # to specify target X as a build_target (whereas both X and Y are
# targets). I'm not sure if that optimization is generally worth it. # targets). I'm not sure if that optimization is generally worth it.
self.WriteJSON({'targets': sorted(all_needed_targets), self.WriteJSON({'targets': sorted(matching_targets),
'build_targets': sorted(all_needed_targets), 'build_targets': sorted(matching_targets),
'status': 'Found dependency'}, output_path) 'status': 'Found dependency'}, output_path)
else: else:
self.WriteJSON({'targets': [], self.WriteJSON({'targets': [],
...@@ -536,11 +542,20 @@ class MetaBuildWrapper(object): ...@@ -536,11 +542,20 @@ class MetaBuildWrapper(object):
with open(path) as fp: with open(path) as fp:
return fp.read() return fp.read()
def RemoveFile(self, path):
# This function largely exists so it can be overriden for testing.
os.remove(path)
def TempFile(self, mode='w'):
# This function largely exists so it can be overriden for testing.
return tempfile.NamedTemporaryFile(mode=mode, delete=False)
def WriteFile(self, path, contents): def WriteFile(self, path, contents):
# This function largely exists so it can be overriden for testing. # This function largely exists so it can be overriden for testing.
with open(path, 'w') as fp: with open(path, 'w') as fp:
return fp.write(contents) return fp.write(contents)
class MBErr(Exception): class MBErr(Exception):
pass pass
......
...@@ -47,6 +47,26 @@ class FakeMBW(mb.MetaBuildWrapper): ...@@ -47,6 +47,26 @@ class FakeMBW(mb.MetaBuildWrapper):
else: else:
self.out += sep.join(args) + end self.out += sep.join(args) + end
def TempFile(self):
return FakeFile(self.files)
def RemoveFile(self, path):
del self.files[path]
class FakeFile(object):
def __init__(self, files):
self.name = '/tmp/file'
self.buf = ''
self.files = files
def write(self, contents):
self.buf += contents
def close(self):
self.files[self.name] = self.buf
class IntegrationTest(unittest.TestCase): class IntegrationTest(unittest.TestCase):
def test_validate(self): def test_validate(self):
# Note that this validates that the actual mb_config.pyl is valid. # Note that this validates that the actual mb_config.pyl is valid.
...@@ -126,6 +146,7 @@ class UnitTest(unittest.TestCase): ...@@ -126,6 +146,7 @@ class UnitTest(unittest.TestCase):
"files": ["foo/foo_unittest.cc"], "files": ["foo/foo_unittest.cc"],
"targets": ["foo_unittests", "bar_unittests"] "targets": ["foo_unittests", "bar_unittests"]
}"""} }"""}
mbw = self.fake_mbw(files) mbw = self.fake_mbw(files)
mbw.Call = lambda cmd: (0, 'out/Default/foo_unittests\n', '') mbw.Call = lambda cmd: (0, 'out/Default/foo_unittests\n', '')
......
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