Bug 1053070 - Output a nicer error when reassigning a variable in moz.build. r=gps
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 14 Aug 2014 07:15:40 +0900
changeset 199434 4b87bdfaf50e405b8fd386e37193e344146fcf86
parent 199433 9ab7126c7cba3a8803b71a75b0600032a8f0a170
child 199435 0ae7262449d8d7e4dcf0520c2b2ea539afc71a8d
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersgps
bugs1053070
milestone34.0a1
Bug 1053070 - Output a nicer error when reassigning a variable in moz.build. r=gps
python/mozbuild/mozbuild/frontend/reader.py
python/mozbuild/mozbuild/frontend/sandbox.py
python/mozbuild/mozbuild/test/frontend/test_namespaces.py
python/mozbuild/mozbuild/test/frontend/test_sandbox.py
--- a/python/mozbuild/mozbuild/frontend/reader.py
+++ b/python/mozbuild/mozbuild/frontend/reader.py
@@ -573,16 +573,26 @@ class BuildReaderError(Exception):
             return
 
         if inner.args[0] == 'global_ns':
             verb = None
             if inner.args[1] == 'get_unknown':
                 verb = 'read'
             elif inner.args[1] == 'set_unknown':
                 verb = 'write'
+            elif inner.args[1] == 'reassign':
+                s.write('The underlying problem is an attempt to reassign ')
+                s.write('a reserved UPPERCASE variable.\n')
+                s.write('\n')
+                s.write('The reassigned variable causing the error is:\n')
+                s.write('\n')
+                s.write('    %s\n' % inner.args[2])
+                s.write('\n')
+                s.write('Maybe you meant "+=" instead of "="?\n')
+                return
             else:
                 raise AssertionError('Unhandled global_ns: %s' % inner.args[1])
 
             s.write('The underlying problem is an attempt to %s ' % verb)
             s.write('a reserved UPPERCASE variable that does not exist.\n')
             s.write('\n')
             s.write('The variable %s causing the error is:\n' % verb)
             s.write('\n')
--- a/python/mozbuild/mozbuild/frontend/sandbox.py
+++ b/python/mozbuild/mozbuild/frontend/sandbox.py
@@ -159,17 +159,17 @@ class GlobalNamespace(dict):
         #   foo = namespace.__getitem__('FOO')
         #   foo.__iadd__(['bar'])
         #   namespace.__setitem__('FOO', foo)
         # This means __setitem__ is called with the value that is already
         # in the dict, when doing +=, which is permitted.
         if name in self._allow_one_mutation:
             self._allow_one_mutation.remove(name)
         elif name in self and dict.__getitem__(self, name) is not value:
-            raise Exception('Reassigning %s is forbidden' % name)
+            raise KeyError('global_ns', 'reassign', name)
 
         # We don't need to check for name.isupper() here because LocalNamespace
         # only sends variables our way if isupper() is True.
         stored_type, input_type, docs, tier = \
             self._allowed_variables.get(name, (None, None, None, None))
 
         # Variable is unknown.
         if stored_type is None:
--- a/python/mozbuild/mozbuild/test/frontend/test_namespaces.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_namespaces.py
@@ -76,28 +76,38 @@ class TestGlobalNamespace(unittest.TestC
             self.assertTrue(d['foo'])
 
         with self.assertRaises(KeyError) as ke:
             ns['bar'] = False
 
         self.assertEqual(ke.exception.args[1], 'set_unknown')
 
         ns['DIRS'] = []
-        with self.assertRaisesRegexp(Exception, 'Reassigning .* is forbidden') as ke:
+        with self.assertRaises(KeyError) as ke:
             ns['DIRS'] = []
 
+        e = ke.exception.args
+        self.assertEqual(e[0], 'global_ns')
+        self.assertEqual(e[1], 'reassign')
+        self.assertEqual(e[2], 'DIRS')
+
         with ns.allow_all_writes() as d:
             d['DIST_SUBDIR'] = 'foo'
 
         self.assertEqual(ns['DIST_SUBDIR'], 'foo')
         ns['DIST_SUBDIR'] = 'bar'
         self.assertEqual(ns['DIST_SUBDIR'], 'bar')
-        with self.assertRaisesRegexp(Exception, 'Reassigning .* is forbidden') as ke:
+        with self.assertRaises(KeyError) as ke:
             ns['DIST_SUBDIR'] = 'baz'
 
+        e = ke.exception.args
+        self.assertEqual(e[0], 'global_ns')
+        self.assertEqual(e[1], 'reassign')
+        self.assertEqual(e[2], 'DIST_SUBDIR')
+
         self.assertTrue(d['foo'])
 
     def test_key_checking(self):
         # Checking for existence of a key should not populate the key if it
         # doesn't exist.
         g = GlobalNamespace(allowed_variables=VARIABLES)
 
         self.assertFalse('DIRS' in g)
--- a/python/mozbuild/mozbuild/test/frontend/test_sandbox.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_sandbox.py
@@ -159,16 +159,32 @@ class TestSandbox(unittest.TestCase):
 
         e = se.exception
         self.assertIsInstance(e.exc_value, KeyError)
 
         e = se.exception.exc_value
         self.assertEqual(e.args[0], 'global_ns')
         self.assertEqual(e.args[1], 'set_unknown')
 
+    def test_exec_source_reassign(self):
+        sandbox = self.sandbox()
+
+        sandbox.exec_source('DIRS = ["foo"]', 'foo.py')
+        with self.assertRaises(SandboxExecutionError) as se:
+          sandbox.exec_source('DIRS = ["bar"]', 'foo.py')
+
+        self.assertEqual(sandbox['DIRS'], ['foo'])
+        e = se.exception
+        self.assertIsInstance(e.exc_value, KeyError)
+
+        e = se.exception.exc_value
+        self.assertEqual(e.args[0], 'global_ns')
+        self.assertEqual(e.args[1], 'reassign')
+        self.assertEqual(e.args[2], 'DIRS')
+
     def test_add_tier_dir_regular_str(self):
         sandbox = self.sandbox()
 
         sandbox.exec_source('add_tier_dir("t1", "foo")', 'foo.py')
 
         self.assertEqual(sandbox['TIERS']['t1'],
             {'regular': ['foo'], 'external': []})