diff --git a/python/mach/mach/main.py b/python/mach/mach/main.py index 75aed4650e9..9589b649305 100644 --- a/python/mach/mach/main.py +++ b/python/mach/mach/main.py @@ -314,6 +314,18 @@ To see more help for a specific command, run: # The first frame is us and is never used. stack = traceback.extract_tb(exc_tb)[1:] + # If we have nothing on the stack, the exception was raised as part + # of calling the @Command method itself. This likely means a + # mismatch between @CommandArgument and arguments to the method. + # e.g. there exists a @CommandArgument without the corresponding + # argument on the method. We handle that here until the module + # loader grows the ability to validate better. + if not len(stack): + print(COMMAND_ERROR) + self._print_exception(sys.stdout, exc_type, exc_value, + traceback.extract_tb(exc_tb)) + return 1 + # Split the frames into those from the module containing the # command and everything else. command_frames = [] diff --git a/python/mach/mach/mixin/process.py b/python/mach/mach/mixin/process.py index d812592caf5..375aa2863a8 100644 --- a/python/mach/mach/mixin/process.py +++ b/python/mach/mach/mixin/process.py @@ -43,7 +43,7 @@ class ProcessExecutionMixin(LoggingMixin): def run_process(self, args=None, cwd=None, append_env=None, explicit_env=None, log_name=None, log_level=logging.INFO, line_handler=None, require_unix_environment=False, - ensure_exit_code=0, ignore_children=False): + ensure_exit_code=0, ignore_children=False, pass_thru=False): """Runs a single process to completion. Takes a list of arguments to run where the first item is the @@ -65,6 +65,13 @@ class ProcessExecutionMixin(LoggingMixin): what is expected. If it is an integer, we raise an Exception if the exit code does not match this value. If it is True, we ensure the exit code is 0. If it is False, we don't perform any exit code validation. + + pass_thru is a special execution mode where the child process inherits + this process's standard file handles (stdin, stdout, stderr) as well as + additional file descriptors. It should be used for interactive processes + where buffering from mozprocess could be an issue. pass_thru does not + use mozprocess. Therefore, arguments like log_name, line_handler, + and ignore_children have no effect. """ args = self._normalize_command(args, require_unix_environment) @@ -94,12 +101,15 @@ class ProcessExecutionMixin(LoggingMixin): self.log(logging.DEBUG, 'process', {'env': use_env}, 'Environment: {env}') - p = ProcessHandlerMixin(args, cwd=cwd, env=use_env, - processOutputLine=[handleLine], universal_newlines=True, - ignore_children=ignore_children) - p.run() - p.processOutput() - status = p.wait() + if pass_thru: + status = subprocess.call(args, cwd=cwd, env=use_env) + else: + p = ProcessHandlerMixin(args, cwd=cwd, env=use_env, + processOutputLine=[handleLine], universal_newlines=True, + ignore_children=ignore_children) + p.run() + p.processOutput() + status = p.wait() if ensure_exit_code is False: return status diff --git a/python/mozbuild/mozbuild/base.py b/python/mozbuild/mozbuild/base.py index 31b3739d6b0..4641bea6bf5 100644 --- a/python/mozbuild/mozbuild/base.py +++ b/python/mozbuild/mozbuild/base.py @@ -2,7 +2,7 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. -from __future__ import unicode_literals +from __future__ import print_function, unicode_literals import logging import os @@ -14,7 +14,11 @@ from mach.mixin.logging import LoggingMixin from mach.mixin.process import ProcessExecutionMixin from .config import BuildConfig -from .mozconfig import MozconfigLoader +from .mozconfig import ( + MozconfigFindException, + MozconfigLoadException, + MozconfigLoader, +) class MozbuildObject(ProcessExecutionMixin): @@ -122,7 +126,8 @@ class MozbuildObject(ProcessExecutionMixin): def _run_make(self, directory=None, filename=None, target=None, log=True, srcdir=False, allow_parallel=True, line_handler=None, append_env=None, explicit_env=None, ignore_errors=False, - ensure_exit_code=0, silent=True, print_directory=True): + ensure_exit_code=0, silent=True, print_directory=True, + pass_thru=False): """Invoke make. directory -- Relative directory to look for Makefile in. @@ -179,6 +184,7 @@ class MozbuildObject(ProcessExecutionMixin): 'log_level': logging.INFO, 'require_unix_environment': True, 'ensure_exit_code': ensure_exit_code, + 'pass_thru': pass_thru, # Make manages its children, so mozprocess doesn't need to bother. # Having mozprocess manage children can also have side-effects when @@ -242,3 +248,27 @@ class MachCommandBase(MozbuildObject): def __init__(self, context): MozbuildObject.__init__(self, context.topdir, context.settings, context.log_manager) + + # Incur mozconfig processing so we have unified error handling for + # errors. Otherwise, the exceptions could bubble back to mach's error + # handler. + try: + self.mozconfig + + except MozconfigFindException as e: + print(e.message) + sys.exit(1) + + except MozconfigLoadException as e: + print('Error loading mozconfig: ' + e.path) + print('') + print(e.message) + if e.output: + print('') + print('mozconfig output:') + print('') + for line in e.output: + print(line) + + sys.exit(1) + diff --git a/python/mozbuild/mozbuild/mozconfig.py b/python/mozbuild/mozbuild/mozconfig.py index aa52910f15e..28e3e1a02f7 100644 --- a/python/mozbuild/mozbuild/mozconfig.py +++ b/python/mozbuild/mozbuild/mozconfig.py @@ -26,6 +26,12 @@ supported. Please move it to %s/.mozconfig or set an explicit path via the $MOZCONFIG environment variable. '''.strip() +MOZCONFIG_BAD_EXIT_CODE = ''' +Evaluation of your mozconfig exited with an error. This could be triggered +by a command inside your mozconfig failing. Please change your mozconfig +to not error and/or to catch errors in executed commands. +'''.strip() + class MozconfigFindException(Exception): """Raised when a mozconfig location is not defined properly.""" @@ -37,8 +43,9 @@ class MozconfigLoadException(Exception): This typically indicates a malformed or misbehaving mozconfig file. """ - def __init__(self, path, message): + def __init__(self, path, message, output=None): self.path = path + self.output = output Exception.__init__(self, message) @@ -173,8 +180,22 @@ class MozconfigLoader(ProcessExecutionMixin): args = self._normalize_command([self._loader_script, self.topsrcdir, path], True) - output = subprocess.check_output(args, stderr=subprocess.PIPE, - cwd=self.topsrcdir, env=env) + try: + # We need to capture stderr because that's where the shell sends + # errors if execution fails. + output = subprocess.check_output(args, stderr=subprocess.STDOUT, + cwd=self.topsrcdir, env=env) + except subprocess.CalledProcessError as e: + lines = e.output.splitlines() + + # Output before actual execution shouldn't be relevant. + try: + index = lines.index('------END_BEFORE_SOURCE') + lines = lines[index + 1:] + except ValueError: + pass + + raise MozconfigLoadException(path, MOZCONFIG_BAD_EXIT_CODE, lines) parsed = self._parse_loader_output(output) @@ -245,7 +266,7 @@ class MozconfigLoader(ProcessExecutionMixin): current_type = None in_variable = None - for line in output.split('\n'): + for line in output.splitlines(): if not len(line): continue diff --git a/python/mozbuild/mozbuild/test/test_mozconfig.py b/python/mozbuild/mozbuild/test/test_mozconfig.py index c72a587cb3d..cf972f26bb4 100644 --- a/python/mozbuild/mozbuild/test/test_mozconfig.py +++ b/python/mozbuild/mozbuild/test/test_mozconfig.py @@ -17,6 +17,7 @@ from tempfile import ( from mozbuild.mozconfig import ( MozconfigFindException, + MozconfigLoadException, MozconfigLoader, ) @@ -296,3 +297,18 @@ class TestMozconfigLoader(unittest.TestCase): self.assertIn('EMPTY', result['env']['added']) self.assertEqual(result['env']['added']['EMPTY'], '') + def test_read_load_exception(self): + """Ensure non-0 exit codes in mozconfigs are handled properly.""" + with NamedTemporaryFile(mode='w') as mozconfig: + mozconfig.write('echo "hello world"\n') + mozconfig.write('exit 1\n') + mozconfig.flush() + + with self.assertRaises(MozconfigLoadException) as e: + self.get_loader().read_mozconfig(mozconfig.name) + + self.assertTrue(e.exception.message.startswith( + 'Evaluation of your mozconfig exited with an error')) + self.assertEquals(e.exception.path, mozconfig.name) + self.assertEquals(e.exception.output, ['hello world']) + diff --git a/testing/mochitest/mach_commands.py b/testing/mochitest/mach_commands.py index 292a0861627..9df5b930006 100644 --- a/testing/mochitest/mach_commands.py +++ b/testing/mochitest/mach_commands.py @@ -23,6 +23,8 @@ from mach.decorators import ( generic_help = 'Test to run. Can be specified as a single file, a ' +\ 'directory, or omitted. If omitted, the entire test suite is executed.' +debugger_help = 'Debugger binary to run test in. Program name or path.' + class MochitestRunner(MozbuildObject): """Easily run mochitests. @@ -50,7 +52,7 @@ class MochitestRunner(MozbuildObject): self.run_chrome_suite() self.run_browser_chrome_suite() - def run_mochitest_test(self, test_file=None, suite=None): + def run_mochitest_test(self, suite=None, test_file=None, debugger=None): """Runs a mochitest. test_file is a path to a test file. It can be a relative path from the @@ -59,6 +61,9 @@ class MochitestRunner(MozbuildObject): suite is the type of mochitest to run. It can be one of ('plain', 'chrome', 'browser'). + + debugger is a program name or path to a binary (presumably a debugger) + to run the test in. e.g. 'gdb' """ # TODO hook up harness via native Python @@ -82,38 +87,53 @@ class MochitestRunner(MozbuildObject): else: env = {} + pass_thru = False + + if debugger: + env[b'EXTRA_TEST_ARGS'] = '--debugger=%s' % debugger + pass_thru = True + return self._run_make(directory='.', target=target, append_env=env, - ensure_exit_code=False) + ensure_exit_code=False, pass_thru=pass_thru) @CommandProvider class MachCommands(MachCommandBase): @Command('mochitest-plain', help='Run a plain mochitest.') + @CommandArgument('--debugger', '-d', metavar='DEBUGGER', + help=debugger_help) @CommandArgument('test_file', default=None, nargs='?', metavar='TEST', help=generic_help) - def run_mochitest_plain(self, test_file): - return self.run_mochitest(test_file, 'plain') + def run_mochitest_plain(self, test_file, debugger=None): + return self.run_mochitest(test_file, 'plain', debugger=debugger) @Command('mochitest-chrome', help='Run a chrome mochitest.') + @CommandArgument('--debugger', '-d', metavar='DEBUGGER', + help=debugger_help) @CommandArgument('test_file', default=None, nargs='?', metavar='TEST', help=generic_help) - def run_mochitest_chrome(self, test_file): - return self.run_mochitest(test_file, 'chrome') + def run_mochitest_chrome(self, test_file, debugger=None): + return self.run_mochitest(test_file, 'chrome', debugger=debugger) @Command('mochitest-browser', help='Run a mochitest with browser chrome.') + @CommandArgument('--debugger', '-d', metavar='DEBUGGER', + help=debugger_help) @CommandArgument('test_file', default=None, nargs='?', metavar='TEST', help=generic_help) - def run_mochitest_browser(self, test_file): - return self.run_mochitest(test_file, 'browser') + def run_mochitest_browser(self, test_file, debugger=None): + return self.run_mochitest(test_file, 'browser', debugger=debugger) @Command('mochitest-a11y', help='Run an a11y mochitest.') + @CommandArgument('--debugger', '-d', metavar='DEBUGGER', + help=debugger_help) @CommandArgument('test_file', default=None, nargs='?', metavar='TEST', help=generic_help) - def run_mochitest_a11y(self, test_file): - return self.run_mochitest(test_file, 'a11y') + def run_mochitest_a11y(self, test_file, debugger=None): + return self.run_mochitest(test_file, 'a11y', debugger=debugger) - def run_mochitest(self, test_file, flavor): + def run_mochitest(self, test_file, flavor, debugger=None): self._ensure_state_subdir_exists('.') mochitest = self._spawn(MochitestRunner) - return mochitest.run_mochitest_test(test_file, flavor) + return mochitest.run_mochitest_test(test_file=test_file, suite=flavor, + debugger=debugger) diff --git a/testing/xpcshell/mach_commands.py b/testing/xpcshell/mach_commands.py index cc8f190024d..747802f67e1 100644 --- a/testing/xpcshell/mach_commands.py +++ b/testing/xpcshell/mach_commands.py @@ -41,7 +41,7 @@ class XPCShellRunner(MozbuildObject): manifest = os.path.join(self.topobjdir, '_tests', 'xpcshell', 'xpcshell.ini') - self._run_xpcshell_harness(manifest=manifest, **kwargs) + return self._run_xpcshell_harness(manifest=manifest, **kwargs) def run_test(self, test_file, debug=False, interactive=False, keep_going=False, shuffle=False): @@ -83,7 +83,7 @@ class XPCShellRunner(MozbuildObject): if os.path.isfile(test_file): args['test_path'] = os.path.basename(test_file) - self._run_xpcshell_harness(**args) + return self._run_xpcshell_harness(**args) def _run_xpcshell_harness(self, test_dirs=None, manifest=None, test_path=None, debug=False, shuffle=False, interactive=False, @@ -140,11 +140,12 @@ class XPCShellRunner(MozbuildObject): filtered_args[k] = v - # TODO do something with result. - xpcshell.runTests(**filtered_args) + result = xpcshell.runTests(**filtered_args) self.log_manager.disable_unstructured() + return int(not result) + @CommandProvider class MachCommands(MachCommandBase):