Bug 946175 - Forbid assigning over a value previously set in moz.build. r=gps
authorMike Hommey <mh+mozilla@glandium.org>
Mon, 09 Dec 2013 13:34:00 +0900
changeset 168636 c9d4612aef4c5e14a194d8cd815fee3a302ccfaf
parent 168635 e926070b0d18361ecabc60324690f142d2ea47a2
child 168637 473d459b4bba7cbd915bcc9693875d3f770a341f
push idunknown
push userunknown
push dateunknown
reviewersgps
bugs946175
milestone28.0a1
Bug 946175 - Forbid assigning over a value previously set in moz.build. r=gps
netwerk/wifi/moz.build
python/mozbuild/mozbuild/frontend/reader.py
python/mozbuild/mozbuild/frontend/sandbox.py
python/mozbuild/mozbuild/test/frontend/data/inheriting-variables/bar/moz.build
python/mozbuild/mozbuild/test/frontend/test_namespaces.py
python/mozbuild/mozbuild/test/frontend/test_reader.py
python/mozbuild/mozbuild/test/frontend/test_sandbox.py
toolkit/components/diskspacewatcher/moz.build
--- a/netwerk/wifi/moz.build
+++ b/netwerk/wifi/moz.build
@@ -1,16 +1,14 @@
 # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # 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/.
 
-FAIL_ON_WARNINGS = True
-
 XPIDL_SOURCES += [
     'nsIWifiAccessPoint.idl',
     'nsIWifiListener.idl',
     'nsIWifiMonitor.idl',
 ]
 
 XPIDL_MODULE = 'necko_wifi'
 
@@ -22,19 +20,20 @@ if CONFIG['MOZ_WIDGET_TOOLKIT'] == 'gonk
     UNIFIED_SOURCES += [
         'nsWifiMonitorGonk.cpp',
     ]
 else:
     UNIFIED_SOURCES += [
         'nsWifiMonitor.cpp',
     ]
 
+# osx_corewlan.mm has warnings I don't understand.
+FAIL_ON_WARNINGS = CONFIG['OS_ARCH'] != 'Darwin'
+
 if CONFIG['OS_ARCH'] == 'Darwin':
-    # osx_corewlan.mm has warnings I don't understand.
-    FAIL_ON_WARNINGS = False
     UNIFIED_SOURCES += [
         'nsWifiScannerMac.cpp',
     ]
     UNIFIED_SOURCES += [
         'osx_corewlan.mm',
     ]
 elif CONFIG['OS_ARCH'] == 'WINNT':
     UNIFIED_SOURCES += [
--- a/python/mozbuild/mozbuild/frontend/reader.py
+++ b/python/mozbuild/mozbuild/frontend/reader.py
@@ -180,19 +180,19 @@ class MozbuildSandbox(Sandbox):
             var = metadata.get('var', None)
             if var and var in ['TOOL_DIRS', 'TEST_TOOL_DIRS']:
                 d['IS_TOOL_DIR'] = True
 
             # Register functions.
             for name, func in FUNCTIONS.items():
                 d[name] = getattr(self, func[0])
 
-        # Initialize the exports that we need in the global.
-        extra_vars = self.metadata.get('exports', dict())
-        self._globals.update(extra_vars)
+            # Initialize the exports that we need in the global.
+            extra_vars = self.metadata.get('exports', dict())
+            self._globals.update(extra_vars)
 
     def exec_file(self, path, filesystem_absolute=False):
         """Override exec_file to normalize paths and restrict file loading.
 
         If the path is absolute, behavior is governed by filesystem_absolute.
         If filesystem_absolute is True, the path is interpreted as absolute on
         the actual filesystem. If it is false, the path is treated as absolute
         within the current topsrcdir.
--- a/python/mozbuild/mozbuild/frontend/sandbox.py
+++ b/python/mozbuild/mozbuild/frontend/sandbox.py
@@ -119,16 +119,18 @@ class GlobalNamespace(dict):
         self._allowed_variables = allowed_variables or {}
 
         # We need to record this because it gets swallowed as part of
         # evaluation.
         self.last_name_error = None
 
         self._allow_all_writes = False
 
+        self._allow_one_mutation = set()
+
     def __getitem__(self, name):
         try:
             return dict.__getitem__(self, name)
         except KeyError:
             pass
 
         # The variable isn't present yet. Fall back to VARIABLES.
         default = self._allowed_variables.get(name, None)
@@ -146,18 +148,31 @@ class GlobalNamespace(dict):
             value = default()
 
         dict.__setitem__(self, name, value)
         return dict.__getitem__(self, name)
 
     def __setitem__(self, name, value):
         if self._allow_all_writes:
             dict.__setitem__(self, name, value)
+            self._allow_one_mutation.add(name)
             return
 
+        # Forbid assigning over a previously set value. Interestingly, when
+        # doing FOO += ['bar'], python actually does something like:
+        #   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)
+
         # 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:
             self.last_name_error = KeyError('global_ns', 'set_unknown', name,
@@ -190,16 +205,21 @@ class GlobalNamespace(dict):
         (__setitem__ calls) are allowed. When the context manager is exited,
         the instance goes back to its default behavior of only allowing
         whitelisted mutations.
         """
         self._allow_all_writes = True
         yield self
         self._allow_all_writes = False
 
+    # dict.update doesn't call our __setitem__, so we have to override it.
+    def update(self, other):
+        for name, value in other.items():
+            self.__setitem__(name, value)
+
 
 class LocalNamespace(dict):
     """Represents the locals namespace in a Sandbox.
 
     This behaves like a dict except with some additional behavior tailored
     to our sandbox execution model.
 
     Under normal rules of exec(), doing things like += could have interesting
--- a/python/mozbuild/mozbuild/test/frontend/data/inheriting-variables/bar/moz.build
+++ b/python/mozbuild/mozbuild/test/frontend/data/inheriting-variables/bar/moz.build
@@ -1,7 +1,5 @@
 # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # 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/.
-
-XPIDL_MODULE = 'bazbar'
--- a/python/mozbuild/mozbuild/test/frontend/test_namespaces.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_namespaces.py
@@ -71,20 +71,33 @@ class TestGlobalNamespace(unittest.TestC
     def test_allow_all_writes(self):
         ns = GlobalNamespace(allowed_variables=VARIABLES)
 
         with ns.allow_all_writes() as d:
             d['foo'] = True
             self.assertTrue(d['foo'])
 
         with self.assertRaises(KeyError) as ke:
-            ns['foo'] = False
+            ns['bar'] = False
 
         self.assertEqual(ke.exception.args[1], 'set_unknown')
 
+        ns['DIRS'] = []
+        with self.assertRaisesRegexp(Exception, 'Reassigning .* is forbidden') as ke:
+            ns['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:
+            ns['DIST_SUBDIR'] = 'baz'
+
         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_reader.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_reader.py
@@ -257,12 +257,12 @@ class TestBuildReader(unittest.TestCase)
         reader = self.reader('inheriting-variables')
 
         sandboxes = list(reader.read_topsrcdir())
 
         self.assertEqual(len(sandboxes), 4)
         self.assertEqual([sandbox['RELATIVEDIR'] for sandbox in sandboxes],
             ['', 'foo', 'foo/baz', 'bar'])
         self.assertEqual([sandbox['XPIDL_MODULE'] for sandbox in sandboxes],
-            ['foobar', 'foobar', 'foobar', 'bazbar'])
+            ['foobar', 'foobar', 'foobar', 'foobar'])
 
 if __name__ == '__main__':
     main()
--- a/python/mozbuild/mozbuild/test/frontend/test_sandbox.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_sandbox.py
@@ -141,19 +141,19 @@ class TestSandbox(unittest.TestCase):
 
         self.assertIsInstance(se.exception, SandboxExecutionError)
         self.assertEqual(se.exception.exc_type, ImportError)
 
     def test_exec_source_multiple(self):
         sandbox = self.sandbox()
 
         sandbox.exec_source('DIRS = ["foo"]', 'foo.py')
-        sandbox.exec_source('DIRS = ["bar"]', 'foo.py')
+        sandbox.exec_source('DIRS += ["bar"]', 'foo.py')
 
-        self.assertEqual(sandbox['DIRS'], ['bar'])
+        self.assertEqual(sandbox['DIRS'], ['foo', 'bar'])
 
     def test_exec_source_illegal_key_set(self):
         sandbox = self.sandbox()
 
         with self.assertRaises(SandboxExecutionError) as se:
             sandbox.exec_source('ILLEGAL = True', 'foo.py')
 
         e = se.exception
--- a/toolkit/components/diskspacewatcher/moz.build
+++ b/toolkit/components/diskspacewatcher/moz.build
@@ -8,17 +8,16 @@ XPIDL_SOURCES += [
     'nsIDiskSpaceWatcher.idl',
 ]
 
 EXPORTS += [
     'DiskSpaceWatcher.h'
 ]
 
 XPIDL_MODULE = 'diskspacewatcher'
-XPIDL_MODULE = 'toolkitcomps'
 
 SOURCES = [
     'DiskSpaceWatcher.cpp',
 ]
 
 include('/ipc/chromium/chromium-config.mozbuild')
 
 FINAL_LIBRARY = 'xul'