Create reclient specific ninja wrapper to properly handle reproxy lifecyle

This is required as it is impossible to catch a keyboard interrupt in a
windows batch file and we dont want zombie reproxy instances running on
developer's machines for performance and metric collection reasons.

Bug: b/264405266
Change-Id: I00f036c8f14451cdb1b99a5cad1c2af03dd57d57
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4171506
Reviewed-by: Dirk Pranke <dpranke@google.com>
Auto-Submit: Ben Segall <bentekkie@google.com>
Commit-Queue: Ben Segall <bentekkie@google.com>
changes/06/4171506/13
Ben Segall 3 years ago committed by LUCI CQ
parent 7c6ebe7fd1
commit eb2866e654

@ -11,10 +11,11 @@ per-file ninja*=thakis@chromium.org
per-file autoninja*=brucedawson@chromium.org
per-file post_build_ninja_summary.py=brucedawson@chromium.org
per-file ninja_reclient.py=file://GOMA_OWNERS
per-file presubmit*.py=brucedawson@chromium.org
per-file ninjalog*=tikuta@chromium.org
per-file cipd_manifest*=file://GOMA_OWNERS
per-file pylint*=vapier@chromium.org

@ -68,15 +68,6 @@ def main(args):
use_remoteexec = False
use_rbe = False
# Currently get reclient binary and config dirs relative to output_dir. If
# they exist and using remoteexec, then automatically call bootstrap to start
# reproxy. This works under the current assumption that the output
# directory is two levels up from chromium/src.
reclient_bin_dir = os.path.join(output_dir, '..', '..', 'buildtools',
'reclient')
reclient_cfg = os.path.join(output_dir, '..', '..', 'buildtools',
'reclient_cfgs', 'reproxy.cfg')
# Attempt to auto-detect remote build acceleration. We support gn-based
# builds, where we look for args.gn in the build tree, and cmake-based builds
# where we look for rules.ninja.
@ -116,29 +107,6 @@ def main(args):
use_goma = True
break
# If use_remoteexec is set, but the reclient binaries or configs don't
# exist, display an error message and stop. Otherwise, the build will
# attempt to run with rewrapper wrapping actions, but will fail with
# possible non-obvious problems.
# As of August 2022, dev builds with reclient are not supported, so
# indicate that use_goma should be swapped for use_remoteexec. This
# message will be changed when dev builds are fully supported.
if (not offline and use_remoteexec and (
not os.path.exists(reclient_bin_dir) or not os.path.exists(reclient_cfg))
):
print(("Build is configured to use reclient but necessary binaries "
"or config files can't be found. Developer builds with "
"reclient are not yet supported. Try regenerating your "
"build with use_goma in place of use_remoteexec for now."),
file=sys.stderr)
if sys.platform.startswith('win'):
# Set an exit code of 1 in the batch file.
print('cmd "/c exit 1"')
else:
# Set an exit code of 1 by executing 'false' in the bash script.
print('false')
sys.exit(1)
# If GOMA_DISABLED is set to "true", "t", "yes", "y", or "1"
# (case-insensitive) then gomacc will use the local compiler instead of doing
# a goma compile. This is convenient if you want to briefly disable goma. It
@ -185,6 +153,10 @@ def main(args):
# Call ninja.py so that it can find ninja binary installed by DEPS or one in
# PATH.
ninja_path = os.path.join(SCRIPT_DIR, 'ninja.py')
# If using remoteexec, use ninja_reclient.py which wraps ninja.py with
# starting and stopping reproxy.
if not offline and use_remoteexec:
ninja_path = os.path.join(SCRIPT_DIR, 'ninja_reclient.py')
args = prefix_args + [sys.executable, ninja_path] + input_args[1:]
num_cores = multiprocessing.cpu_count()
@ -234,22 +206,6 @@ def main(args):
if os.environ.get('NINJA_SUMMARIZE_BUILD', '0') == '1':
args += ['-d', 'stats']
# If using remoteexec and the necessary environment variables are set,
# also start reproxy (via bootstrap) before running ninja.
if (not offline and use_remoteexec and os.path.exists(reclient_bin_dir)
and os.path.exists(reclient_cfg)):
bootstrap = os.path.join(reclient_bin_dir, 'bootstrap')
setup_args = [
bootstrap, '--cfg=' + reclient_cfg,
'--re_proxy=' + os.path.join(reclient_bin_dir, 'reproxy')
]
teardown_args = [bootstrap, '--cfg=' + reclient_cfg, '--shutdown']
cmd_sep = '\n' if sys.platform.startswith('win') else '&&'
cmd_always_sep = '\n' if sys.platform.startswith('win') else '; '
args = setup_args + [cmd_sep] + args + [cmd_always_sep] + teardown_args
if offline and not sys.platform.startswith('win'):
# Tell goma or reclient to do local compiles. On Windows these environment
# variables are set by the wrapper batch file.

@ -0,0 +1,91 @@
#!/usr/bin/env python3
# Copyright 2023 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
"""This script is a wrapper around the ninja.py script that also
handles the client lifecycle safely. It will automatically start
reproxy before running ninja and stop reproxy when ninja stops
for any reason eg. build completes, keyboard interupt etc."""
import os
import subprocess
import sys
import ninja
import gclient_paths
def find_reclient_bin_dir():
tools_path = gclient_paths.GetBuildtoolsPath()
if not tools_path:
return None
reclient_bin_dir = os.path.join(tools_path, 'reclient')
if os.path.isdir(reclient_bin_dir):
return reclient_bin_dir
return None
def find_reclient_cfg():
tools_path = gclient_paths.GetBuildtoolsPath()
if not tools_path:
return None
reclient_cfg = os.path.join(tools_path, 'reclient_cfgs', 'reproxy.cfg')
if os.path.isfile(reclient_cfg):
return reclient_cfg
return None
def run(cmd_args):
if os.environ.get('NINJA_SUMMARIZE_BUILD') == '1':
print(' '.join(cmd_args))
return subprocess.call(cmd_args)
def start_reproxy(reclient_cfg, reclient_bin_dir):
return run([
os.path.join(reclient_bin_dir, 'bootstrap'),
'--re_proxy=' + os.path.join(reclient_bin_dir, 'reproxy'),
'--cfg=' + reclient_cfg
])
def stop_reproxy(reclient_cfg, reclient_bin_dir):
return run([
os.path.join(reclient_bin_dir, 'bootstrap'), '--shutdown',
'--cfg=' + reclient_cfg
])
def main(argv):
# If use_remoteexec is set, but the reclient binaries or configs don't
# exist, display an error message and stop. Otherwise, the build will
# attempt to run with rewrapper wrapping actions, but will fail with
# possible non-obvious problems.
# As of January 2023, dev builds with reclient are not supported, so
# indicate that use_goma should be swapped for use_remoteexec. This
# message will be changed when dev builds are fully supported.
reclient_bin_dir = find_reclient_bin_dir()
reclient_cfg = find_reclient_cfg()
if reclient_bin_dir is None or reclient_cfg is None:
print(("Build is configured to use reclient but necessary binaries "
"or config files can't be found. Developer builds with "
"reclient are not yet supported. Try regenerating your "
"build with use_goma in place of use_remoteexec for now."),
file=sys.stderr)
return 1
reproxy_ret_code = start_reproxy(reclient_cfg, reclient_bin_dir)
if reproxy_ret_code != 0:
return reproxy_ret_code
try:
return ninja.main(argv)
except KeyboardInterrupt:
# Suppress python stack trace if ninja is interrupted
return 1
finally:
stop_reproxy(reclient_cfg, reclient_bin_dir)
if __name__ == '__main__':
sys.exit(main(sys.argv))

@ -1,3 +1,4 @@
per-file autoninja_test.py=brucedawson@chromium.org
per-file autoninja_test.py=tikuta@chromium.org
per-file ninjalog_uploader_test.py=tikuta@chromium.org
per-file ninja_reclient_test.py=file://GOMA_OWNERS

@ -14,26 +14,66 @@ ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
sys.path.insert(0, ROOT_DIR)
import autoninja
from testing_support import trial_dir
class AutoninjaTest(unittest.TestCase):
def write(filename, content):
"""Writes the content of a file and create the directories as needed."""
filename = os.path.abspath(filename)
dirname = os.path.dirname(filename)
if not os.path.isdir(dirname):
os.makedirs(dirname)
with open(filename, 'w') as f:
f.write(content)
class AutoninjaTest(trial_dir.TestCase):
def setUp(self):
super(AutoninjaTest, self).setUp()
self.previous_dir = os.getcwd()
os.chdir(self.root_dir)
def tearDown(self):
os.chdir(self.previous_dir)
super(AutoninjaTest, self).tearDown()
def test_autoninja(self):
autoninja.main([])
def test_autoninja_goma(self):
with unittest.mock.patch(
'os.path.exists',
return_value=True) as mock_exists, unittest.mock.patch(
'autoninja.open', unittest.mock.mock_open(
read_data='use_goma=true')) as mock_open, unittest.mock.patch(
'subprocess.call', return_value=0):
args = autoninja.main([]).split()
mock_exists.assert_called()
mock_open.assert_called_once()
'subprocess.call',
return_value=0) as mock_call, unittest.mock.patch.dict(
os.environ, {"GOMA_DIR": os.path.join(self.root_dir, 'goma_dir')}):
out_dir = os.path.join('out', 'dir')
write(os.path.join(out_dir, 'args.gn'), 'use_goma=true')
write(
os.path.join(
'goma_dir',
'gomacc.exe' if sys.platform.startswith('win') else 'gomacc'),
'content')
args = autoninja.main(['autoninja.py', '-C', out_dir]).split()
mock_call.assert_called_once()
self.assertIn('-j', args)
parallel_j = int(args[args.index('-j') + 1])
self.assertGreater(parallel_j, multiprocessing.cpu_count())
self.assertIn(os.path.join(autoninja.SCRIPT_DIR, 'ninja.py'), args)
def test_autoninja_reclient(self):
out_dir = os.path.join('out', 'dir')
write(os.path.join(out_dir, 'args.gn'), 'use_remoteexec=true')
write('.gclient', '')
write('.gclient_entries', 'entries = {"buildtools": "..."}')
write(os.path.join('buildtools', 'reclient_cfgs', 'reproxy.cfg'), 'RBE_v=2')
write(os.path.join('buildtools', 'reclient', 'version.txt'), '0.0')
args = autoninja.main(['autoninja.py', '-C', out_dir]).split()
self.assertIn('-j', args)
parallel_j = int(args[args.index('-j') + 1])
self.assertGreater(parallel_j, multiprocessing.cpu_count())
self.assertIn(os.path.join(autoninja.SCRIPT_DIR, 'ninja_reclient.py'), args)
if __name__ == '__main__':

@ -0,0 +1,124 @@
#!/usr/bin/env python3
# Copyright (c) 2023 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import os
import os.path
import sys
import unittest
import unittest.mock
ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
sys.path.insert(0, ROOT_DIR)
import ninja_reclient
from testing_support import trial_dir
def write(filename, content):
"""Writes the content of a file and create the directories as needed."""
filename = os.path.abspath(filename)
dirname = os.path.dirname(filename)
if not os.path.isdir(dirname):
os.makedirs(dirname)
with open(filename, 'w') as f:
f.write(content)
class NinjaReclientTest(trial_dir.TestCase):
def setUp(self):
super(NinjaReclientTest, self).setUp()
self.previous_dir = os.getcwd()
os.chdir(self.root_dir)
def tearDown(self):
os.chdir(self.previous_dir)
super(NinjaReclientTest, self).tearDown()
@unittest.mock.patch('subprocess.call', return_value=0)
@unittest.mock.patch('ninja.main', return_value=0)
def test_ninja_reclient(self, mock_ninja, mock_call):
reclient_bin_dir = os.path.join('src', 'buildtools', 'reclient')
reclient_cfg = os.path.join('src', 'buildtools', 'reclient_cfgs',
'reproxy.cfg')
write('.gclient', '')
write('.gclient_entries', 'entries = {"buildtools": "..."}')
write(os.path.join(reclient_bin_dir, 'version.txt'), '0.0')
write(reclient_cfg, '0.0')
argv = ["ninja_reclient.py", "-C", "out/a", "chrome"]
self.assertEqual(0, ninja_reclient.main(argv))
mock_ninja.assert_called_once_with(argv)
mock_call.assert_has_calls([
unittest.mock.call([
os.path.join(self.root_dir, reclient_bin_dir,
'bootstrap'), "--re_proxy=" +
os.path.join(self.root_dir, reclient_bin_dir, 'reproxy'),
"--cfg=" + os.path.join(self.root_dir, reclient_cfg)
]),
unittest.mock.call([
os.path.join(self.root_dir, reclient_bin_dir, 'bootstrap'),
"--shutdown", "--cfg=" + os.path.join(self.root_dir, reclient_cfg)
]),
])
@unittest.mock.patch('subprocess.call', return_value=0)
@unittest.mock.patch('ninja.main', side_effect=KeyboardInterrupt())
def test_ninja_reclient_ninja_interrupted(self, mock_ninja, mock_call):
reclient_bin_dir = os.path.join('src', 'buildtools', 'reclient')
reclient_cfg = os.path.join('src', 'buildtools', 'reclient_cfgs',
'reproxy.cfg')
write('.gclient', '')
write('.gclient_entries', 'entries = {"buildtools": "..."}')
write(os.path.join(reclient_bin_dir, 'version.txt'), '0.0')
write(reclient_cfg, '0.0')
argv = ["ninja_reclient.py", "-C", "out/a", "chrome"]
self.assertEqual(1, ninja_reclient.main(argv))
mock_ninja.assert_called_once_with(argv)
mock_call.assert_has_calls([
unittest.mock.call([
os.path.join(self.root_dir, reclient_bin_dir,
'bootstrap'), "--re_proxy=" +
os.path.join(self.root_dir, reclient_bin_dir, 'reproxy'),
"--cfg=" + os.path.join(self.root_dir, reclient_cfg)
]),
unittest.mock.call([
os.path.join(self.root_dir, reclient_bin_dir, 'bootstrap'),
"--shutdown", "--cfg=" + os.path.join(self.root_dir, reclient_cfg)
]),
])
@unittest.mock.patch('subprocess.call', return_value=0)
@unittest.mock.patch('ninja.main', return_value=0)
def test_ninja_reclient_cfg_not_found(self, mock_ninja, mock_call):
write('.gclient', '')
write('.gclient_entries', 'entries = {"buildtools": "..."}')
write(os.path.join('src', 'buildtools', 'reclient', 'version.txt'), '0.0')
argv = ["ninja_reclient.py", "-C", "out/a", "chrome"]
self.assertEqual(1, ninja_reclient.main(argv))
mock_ninja.assert_not_called()
mock_call.assert_not_called()
@unittest.mock.patch('subprocess.call', return_value=0)
@unittest.mock.patch('ninja.main', return_value=0)
def test_ninja_reclient_bins_not_found(self, mock_ninja, mock_call):
write('.gclient', '')
write('.gclient_entries', 'entries = {"buildtools": "..."}')
write(os.path.join('src', 'buildtools', 'reclient_cfgs', 'reproxy.cfg'),
'0.0')
argv = ["ninja_reclient.py", "-C", "out/a", "chrome"]
self.assertEqual(1, ninja_reclient.main(argv))
mock_ninja.assert_not_called()
mock_call.assert_not_called()
if __name__ == '__main__':
unittest.main()
Loading…
Cancel
Save