diff --git a/autoninja b/autoninja index 06d45d1bf3..4ee2421f82 100755 --- a/autoninja +++ b/autoninja @@ -20,23 +20,16 @@ fi # Execute whatever is printed by autoninja.py. # Also print it to reassure that the right settings are being used. -command=$(python3 "$(dirname -- "$0")/autoninja.py" "$@") -if [ "$NINJA_SUMMARIZE_BUILD" == "1" ]; then - echo "$command" -fi -if eval "$command"; then - if [ "$NINJA_SUMMARIZE_BUILD" == "1" ]; then - python3 "$(dirname -- "$0")/post_build_ninja_summary.py" "$@" - fi - - # Collect ninjalog from googler. - python3 "$(dirname -- "$0")/ninjalog_uploader_wrapper.py" --cmd $command - exit +python3 "$(dirname -- "$0")/autoninja.py" "$@" +retval=$? + +if [ "$retval" == "0" ] && [ "$NINJA_SUMMARIZE_BUILD" == "1" ]; then + python3 "$(dirname -- "$0")/post_build_ninja_summary.py" "$@" fi # Collect ninjalog from googler. -python3 "$(dirname -- "$0")/ninjalog_uploader_wrapper.py" --cmd $command +python3 "$(dirname -- "$0")/ninjalog_uploader_wrapper.py" --cmdline "$@" -# Return an error code of 1 so that if a developer types: +# Pass-through autoninja's error code so that if a developer types: # "autoninja chrome && chrome" then chrome won't run if the build fails. -exit 1 +exit $retval diff --git a/autoninja.bat b/autoninja.bat index cfe6625485..03fba6476b 100755 --- a/autoninja.bat +++ b/autoninja.bat @@ -24,28 +24,13 @@ if not defined AUTONINJA_BUILD_ID ( :: Windows. The trailing space is intentional. if "%NINJA_SUMMARIZE_BUILD%" == "1" set "NINJA_STATUS=[%%r processes, %%f/%%t @ %%o/s : %%es ] " -:loop -IF NOT "%1"=="" ( - :: Tell goma or reclient to not do network compiles. - IF "%1"=="--offline" ( - SET GOMA_DISABLED=1 - SET RBE_remote_disabled=1 - ) - IF "%1"=="-o" ( - SET GOMA_DISABLED=1 - SET RBE_remote_disabled=1 - ) - SHIFT - GOTO :loop -) - -:: Execute whatever is printed by autoninja.py. -:: Also print it to reassure that the right settings are being used. +:: Execute autoninja.py and pass all arguments to it. :: Don't use vpython - it is too slow to start. :: Don't use python3 because it doesn't work in git bash on Windows and we :: should be consistent between autoninja.bat and the autoninja script used by :: git bash. -FOR /f "usebackq tokens=*" %%a in (`%scriptdir%python-bin\python3.bat %scriptdir%autoninja.py "%*"`) do echo %%a & %%a + +@call %scriptdir%python-bin\python3.bat %scriptdir%autoninja.py "%%*" @if errorlevel 1 goto buildfailure :: Use call to invoke python script here, because we use python via python3.bat. diff --git a/autoninja.py b/autoninja.py index 08d9b0f3e4..7f100d1e53 100755 --- a/autoninja.py +++ b/autoninja.py @@ -18,14 +18,51 @@ import multiprocessing import os import platform import re +import shlex import subprocess import sys +import autosiso +import ninja +import ninja_reclient +import siso + if sys.platform in ['darwin', 'linux']: import resource SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) +# See [1] and [2] for the painful details of this next section, which handles +# escaping command lines so that they can be copied and pasted into a cmd +# window. +# +# pylint: disable=line-too-long +# [1] https://learn.microsoft.com/en-us/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way # noqa +# [2] https://web.archive.org/web/20150815000000*/https://www.microsoft.com/resources/documentation/windows/xp/all/proddocs/en-us/set.mspx # noqa +_UNSAFE_FOR_CMD = set('^<>&|()%') +_ALL_META_CHARS = _UNSAFE_FOR_CMD.union(set('"')) + + +def _quote_for_cmd(arg): + # First, escape the arg so that CommandLineToArgvW will parse it properly. + if arg == '' or ' ' in arg or '"' in arg: + quote_re = re.compile(r'(\\*)"') + arg = '"%s"' % (quote_re.sub(lambda mo: 2 * mo.group(1) + '\\"', arg)) + + # Then check to see if the arg contains any metacharacters other than + # double quotes; if it does, quote everything (including the double + # quotes) for safety. + if any(a in _UNSAFE_FOR_CMD for a in arg): + arg = ''.join('^' + a if a in _ALL_META_CHARS else a for a in arg) + return arg + + +def _print_cmd(cmd): + shell_quoter = shlex.quote + if sys.platform.startswith('win'): + shell_quoter = _quote_for_cmd + print(*[shell_quoter(arg) for arg in cmd], file=sys.stderr) + def _gn_lines(output_dir, path): """ @@ -58,6 +95,7 @@ def main(args): offline = False output_dir = '.' input_args = args + summarize_build = os.environ.get('NINJA_SUMMARIZE_BUILD') == '1' # On Windows the autoninja.bat script passes along the arguments enclosed in # double quotes. This prevents multiple levels of parsing of the special '^' # characters needed when compiling a single file but means that this script @@ -134,21 +172,19 @@ def main(args): # if there is a .ninja_log but no .siso_deps then that implies a # ninja build. if os.path.exists(ninja_marker) and not os.path.exists(siso_marker): - return ( - 'echo Run gn clean before switching from ninja to siso in ' - '%s' % output_dir) - siso = ['autosiso'] if use_remoteexec else ['siso', 'ninja'] - if sys.platform.startswith('win'): - # An explicit 'call' is needed to make sure the invocation of - # autosiso returns to autoninja.bat, and the command prompt - # title gets reset. - siso = ['call'] + siso - return ' '.join(siso + input_args[1:]) + print('Run gn clean before switching from ninja to siso in %s' % + output_dir, + file=sys.stderr) + return 1 + if use_remoteexec: + return autosiso.main(['autosiso'] + input_args[1:]) + return siso.main(['siso', 'ninja'] + input_args[1:]) if os.path.exists(siso_marker): - return ( - 'echo Run gn clean before switching from siso to ninja in %s' % - output_dir) + print('Run gn clean before switching from siso to ninja in %s' % + output_dir, + file=sys.stderr) + return 1 else: for relative_path in [ @@ -207,19 +243,20 @@ def main(args): sys.exit(1) # A large build (with or without goma) tends to hog all system resources. - # Launching the ninja process with 'nice' priorities improves this - # situation. - prefix_args = [] - if (sys.platform.startswith('linux') - and os.environ.get('NINJA_BUILD_IN_BACKGROUND', '0') == '1'): - # nice -10 is process priority 10 lower than default 0 - # ionice -c 3 is IO priority IDLE - prefix_args = ['nice'] + ['-10'] - - # Tell goma or reclient to do local compiles. On Windows these environment - # variables are set by the wrapper batch file. - offline_env = ['RBE_remote_disabled=1', 'GOMA_DISABLED=1' - ] if offline and not sys.platform.startswith('win') else [] + # Depending on the operating system, we might have mechanisms available + # to run at a lower priority, which improves this situation. + if os.environ.get('NINJA_BUILD_IN_BACKGROUND') == '1': + if sys.platform in ['darwin', 'linux']: + # nice-level 10 is usually considered a good default for background + # tasks. The niceness is inherited by child processes, so we can + # just set it here for us and it'll apply to the build tool we + # spawn later. + os.nice(10) + + # Tell goma or reclient to do local compiles. + if offline: + os.environ['RBE_remote_disabled'] = '1' + os.environ['GOMA_DISABLED'] = '1' # On macOS and most Linux distributions, the default limit of open file # descriptors is too low (256 and 1024, respectively). @@ -227,7 +264,6 @@ def main(args): # Check whether the limit can be raised to a large enough value. If yes, # use `ulimit -n .... &&` as a prefix to increase the limit when running # ninja. - prepend_command = [] if sys.platform in ['darwin', 'linux']: # Increase the number of allowed open file descriptors to the maximum. fileno_limit, hard_limit = resource.getrlimit(resource.RLIMIT_NOFILE) @@ -235,12 +271,10 @@ def main(args): try: resource.setrlimit(resource.RLIMIT_NOFILE, (hard_limit, hard_limit)) - except Exception as _: + except Exception: pass fileno_limit, hard_limit = resource.getrlimit( resource.RLIMIT_NOFILE) - if fileno_limit == hard_limit: - prepend_command = ['ulimit', '-n', f'{fileno_limit}', '&&'] # Call ninja.py so that it can find ninja binary installed by DEPS or one in # PATH. @@ -250,9 +284,7 @@ def main(args): if use_remoteexec: ninja_path = os.path.join(SCRIPT_DIR, 'ninja_reclient.py') - args = prepend_command + offline_env + prefix_args + [ - sys.executable, ninja_path - ] + input_args[1:] + args = [sys.executable, ninja_path] + input_args[1:] num_cores = multiprocessing.cpu_count() if not j_specified and not t_specified: @@ -291,20 +323,20 @@ def main(args): args.append('-j') args.append('%d' % j_value) - # On Windows, fully quote the path so that the command processor doesn't - # think the whole output is the command. On Linux and Mac, if people put - # depot_tools in directories with ' ', shell would misunderstand ' ' as a - # path separation. TODO(yyanagisawa): provide proper quoting for Windows. - # see https://cs.chromium.org/chromium/src/tools/mb/mb.py - for i in range(len(args)): - if (i == 0 and sys.platform.startswith('win')) or ' ' in args[i]: - args[i] = '"%s"' % args[i].replace('"', '\\"') - - if os.environ.get('NINJA_SUMMARIZE_BUILD', '0') == '1': + if summarize_build: + # Enable statistics collection in Ninja. args += ['-d', 'stats'] + # Print the command-line to reassure the user that the right settings + # are being used. + _print_cmd(args) - return ' '.join(args) + if use_remoteexec: + return ninja_reclient.main(args[1:]) + return ninja.main(args[1:]) if __name__ == '__main__': - print(main(sys.argv)) + try: + sys.exit(main(sys.argv)) + except KeyboardInterrupt: + sys.exit(1) diff --git a/tests/autoninja_test.py b/tests/autoninja_test.py index 5746f49289..8bf6eac7f4 100755 --- a/tests/autoninja_test.py +++ b/tests/autoninja_test.py @@ -6,9 +6,11 @@ import multiprocessing import os import os.path +import io import sys import unittest -import unittest.mock +import contextlib +from unittest import mock ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) sys.path.insert(0, ROOT_DIR) @@ -38,42 +40,117 @@ class AutoninjaTest(trial_dir.TestCase): super(AutoninjaTest, self).tearDown() def test_autoninja(self): - autoninja.main([]) + """Test that by default (= no GN args) autoninja delegates to ninja.""" + with mock.patch('ninja.main', return_value=0) as ninja_main: + out_dir = os.path.join('out', 'dir') + write(os.path.join(out_dir, 'args.gn'), '') + autoninja.main(['autoninja.py', '-C', out_dir]) + ninja_main.assert_called_once() + args = ninja_main.call_args.args[0] + self.assertIn('-C', args) + self.assertEqual(args[args.index('-C') + 1], out_dir) + + @mock.patch('sys.platform', 'win32') + def test_autoninja_splits_args_on_windows(self): + """ + Test that autoninja correctly handles the special case of being + passed its arguments as a quoted, whitespace-delimited string on + Windows. + """ + with mock.patch('ninja.main', return_value=0) as ninja_main: + out_dir = os.path.join('out', 'dir') + write(os.path.join(out_dir, 'args.gn'), '') + autoninja.main([ + 'autoninja.py', + '-C {} base'.format(out_dir), + ]) + ninja_main.assert_called_once() + args = ninja_main.call_args.args[0] + self.assertIn('-C', args) + self.assertEqual(args[args.index('-C') + 1], out_dir) + self.assertIn('base', args) def test_autoninja_goma(self): - with unittest.mock.patch( - 'subprocess.call', - return_value=0) as mock_call, unittest.mock.patch.dict( - os.environ, - {"GOMA_DIR": os.path.join(self.root_dir, 'goma_dir')}): + """ + Test that when specifying use_goma=true, autoninja verifies that Goma + is running and then delegates to ninja. + """ + goma_dir = os.path.join(self.root_dir, 'goma_dir') + with mock.patch('subprocess.call', return_value=0), \ + mock.patch('ninja.main', return_value=0) as ninja_main, \ + mock.patch.dict(os.environ, {"GOMA_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() - + autoninja.main(['autoninja.py', '-C', out_dir]) + ninja_main.assert_called_once() + args = ninja_main.call_args.args[0] + self.assertIn('-C', args) + self.assertEqual(args[args.index('-C') + 1], out_dir) + # Check that autoninja correctly calculated the number of jobs to use + # as required for remote execution, instead of using the value for + # local execution. self.assertIn('-j', args) parallel_j = int(args[args.index('-j') + 1]) self.assertGreater(parallel_j, multiprocessing.cpu_count() * 2) - 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(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() - + """ + Test that when specifying use_remoteexec=true, autoninja delegates to + ninja_reclient. + """ + with mock.patch('ninja_reclient.main', + return_value=0) as ninja_reclient_main: + out_dir = os.path.join('out', 'dir') + write(os.path.join(out_dir, 'args.gn'), 'use_remoteexec=true') + write(os.path.join('buildtools', 'reclient_cfgs', 'reproxy.cfg'), + 'RBE_v=2') + write(os.path.join('buildtools', 'reclient', 'version.txt'), '0.0') + autoninja.main(['autoninja.py', '-C', out_dir]) + ninja_reclient_main.assert_called_once() + args = ninja_reclient_main.call_args.args[0] + self.assertIn('-C', args) + self.assertEqual(args[args.index('-C') + 1], out_dir) + # Check that autoninja correctly calculated the number of jobs to use + # as required for remote execution, instead of using the value for + # local execution. self.assertIn('-j', args) parallel_j = int(args[args.index('-j') + 1]) self.assertGreater(parallel_j, multiprocessing.cpu_count() * 2) - self.assertIn(os.path.join(autoninja.SCRIPT_DIR, 'ninja_reclient.py'), - args) + + def test_autoninja_siso(self): + """ + Test that when specifying use_siso=true, autoninja delegates to siso. + """ + with mock.patch('siso.main', return_value=0) as siso_main: + out_dir = os.path.join('out', 'dir') + write(os.path.join(out_dir, 'args.gn'), 'use_siso=true') + autoninja.main(['autoninja.py', '-C', out_dir]) + siso_main.assert_called_once() + args = siso_main.call_args.args[0] + self.assertIn('-C', args) + self.assertEqual(args[args.index('-C') + 1], out_dir) + + def test_autoninja_siso_reclient(self): + """ + Test that when specifying use_siso=true and use_remoteexec=true, + autoninja delegates to autosiso. + """ + with mock.patch('autosiso.main', return_value=0) as autosiso_main: + out_dir = os.path.join('out', 'dir') + write(os.path.join(out_dir, 'args.gn'), + 'use_siso=true\nuse_remoteexec=true') + write(os.path.join('buildtools', 'reclient_cfgs', 'reproxy.cfg'), + 'RBE_v=2') + write(os.path.join('buildtools', 'reclient', 'version.txt'), '0.0') + autoninja.main(['autoninja.py', '-C', out_dir]) + autosiso_main.assert_called_once() + args = autosiso_main.call_args.args[0] + self.assertIn('-C', args) + self.assertEqual(args[args.index('-C') + 1], out_dir) def test_gn_lines(self): out_dir = os.path.join('out', 'dir') @@ -92,6 +169,34 @@ class AutoninjaTest(trial_dir.TestCase): 'use_remoteexec=true', ]) + @mock.patch('sys.platform', 'win32') + def test_print_cmd_windows(self): + args = [ + 'C:\\Program Files\\Python 3\\bin\\python3.exe', 'ninja.py', '-C', + 'out\\direc tory\\', + '../../base/types/expected_macros_unittest.cc^', '-j', '140' + ] + with contextlib.redirect_stderr(io.StringIO()) as f: + autoninja._print_cmd(args) + self.assertEqual( + f.getvalue(), + '"C:\\Program Files\\Python 3\\bin\\python3.exe" ninja.py -C ' + + '"out\\direc tory\\" ' + + '../../base/types/expected_macros_unittest.cc^^ -j 140\n') + + @mock.patch('sys.platform', 'linux') + def test_print_cmd_linux(self): + args = [ + '/home/user name/bin/python3', 'ninja.py', '-C', 'out/direc tory/', + '../../base/types/expected_macros_unittest.cc^', '-j', '140' + ] + with contextlib.redirect_stderr(io.StringIO()) as f: + autoninja._print_cmd(args) + self.assertEqual( + f.getvalue(), + "'/home/user name/bin/python3' ninja.py -C 'out/direc tory/' " + + "'../../base/types/expected_macros_unittest.cc^' -j 140\n") + if __name__ == '__main__': unittest.main()