Bug 818545 - Gracefully handle errors when loading mozconfigs; r=glandium
authorGregory Szorc <gps@mozilla.com>
Wed, 05 Dec 2012 14:34:14 -0800
changeset 124178 b0cb01ce8f12c30105522bce27f95feba23e7e22
parent 124177 ba0ff949ba6050718f6bd685d20c44364072dc1d
child 124179 112c885bd01f504678222b0810df95387e0ab990
push id2151
push userlsblakk@mozilla.com
push dateTue, 19 Feb 2013 18:06:57 +0000
treeherdermozilla-beta@4952e88741ec [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs818545
milestone20.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 818545 - Gracefully handle errors when loading mozconfigs; r=glandium DONTBUILD (NPOTB)
python/mozbuild/mozbuild/base.py
python/mozbuild/mozbuild/mozconfig.py
python/mozbuild/mozbuild/test/test_mozconfig.py
--- a/python/mozbuild/mozbuild/base.py
+++ b/python/mozbuild/mozbuild/base.py
@@ -1,25 +1,29 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # 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
 import subprocess
 import sys
 import which
 
 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):
     """Base class providing basic functionality useful to many modules.
 
     Modules in this package typically require common functionality such as
     accessing the current config, getting the location of the source directory,
     running processes, etc. This classes provides that functionality. Other
@@ -239,8 +243,32 @@ class MachCommandBase(MozbuildObject):
 
     This provides a level of indirection so MozbuildObject can be refactored
     without having to change everything that inherits from it.
     """
 
     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)
+
--- a/python/mozbuild/mozbuild/mozconfig.py
+++ b/python/mozbuild/mozbuild/mozconfig.py
@@ -21,29 +21,36 @@ variable, use MOZCONFIG instead.
 '''.strip()
 
 MOZCONFIG_LEGACY_PATH = '''
 You currently have a mozconfig at %s. This implicit location is no longer
 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."""
 
 
 class MozconfigLoadException(Exception):
     """Raised when a mozconfig could not be loaded properly.
 
     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)
 
 
 class MozconfigLoader(ProcessExecutionMixin):
     """Handles loading and parsing of mozconfig files."""
 
     RE_MAKE_VARIABLE = re.compile('''
         ^\s*                    # Leading whitespace
@@ -168,18 +175,32 @@ class MozconfigLoader(ProcessExecutionMi
         result['make_extra'] = []
         result['make_flags'] = []
 
         env = dict(os.environ)
 
         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)
 
         all_variables = set(parsed['vars_before'].keys())
         all_variables |= set(parsed['vars_after'].keys())
 
         changed = {
             'added': {},
@@ -240,17 +261,17 @@ class MozconfigLoader(ProcessExecutionMi
         ac_options = []
         before_source = {}
         after_source = {}
 
         current = None
         current_type = None
         in_variable = None
 
-        for line in output.split('\n'):
+        for line in output.splitlines():
             if not len(line):
                 continue
 
             if line.startswith('------BEGIN_'):
                 assert current_type is None
                 assert current is None
                 assert not in_variable
                 current_type = line[len('------BEGIN_'):]
--- a/python/mozbuild/mozbuild/test/test_mozconfig.py
+++ b/python/mozbuild/mozbuild/test/test_mozconfig.py
@@ -12,16 +12,17 @@ from shutil import rmtree
 from tempfile import (
     gettempdir,
     mkdtemp,
     NamedTemporaryFile,
 )
 
 from mozbuild.mozconfig import (
     MozconfigFindException,
+    MozconfigLoadException,
     MozconfigLoader,
 )
 
 
 class TestMozconfigLoader(unittest.TestCase):
     def setUp(self):
         self._old_env = dict(os.environ)
         self._temp_dirs = set()
@@ -291,8 +292,23 @@ class TestMozconfigLoader(unittest.TestC
             mozconfig.write('EMPTY=\n')
             mozconfig.flush()
 
             result = self.get_loader().read_mozconfig(mozconfig.name)
 
             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'])
+