Bug 863069 - Part 3: Allow limited type coercion in moz.build sandbox; r=ted
authorGregory Szorc <gps@mozilla.com>
Thu, 16 May 2013 09:53:14 -0700
changeset 139253 f0661e6dd5b4e51db9002cf270d531b592a32a06
parent 139252 662d99ac18fb46effecde04685c9c5b2ccff3943
child 139254 94978dab7186a4abcc3d6a51dbe3098901b6d135
push idunknown
push userunknown
push dateunknown
reviewersted
bugs863069
milestone24.0a1
Bug 863069 - Part 3: Allow limited type coercion in moz.build sandbox; r=ted We now differentiate between the stored and incoming types on global variables. If an incoming type is not the stored type but is an allowed type, we construct the stored type from the incoming value.
python/mozbuild/mozbuild/frontend/sandbox.py
python/mozbuild/mozbuild/frontend/sandbox_symbols.py
python/mozbuild/mozbuild/test/frontend/test_sandbox_symbols.py
--- a/python/mozbuild/mozbuild/frontend/sandbox.py
+++ b/python/mozbuild/mozbuild/frontend/sandbox.py
@@ -76,18 +76,18 @@ class GlobalNamespace(dict):
         'True': True,
     })
 
     def __init__(self, allowed_variables=None, builtins=None):
         """Create a new global namespace having specific variables.
 
         allowed_variables is a dict of the variables that can be queried and
         mutated. Keys in this dict are the strings representing keys in this
-        namespace which are valid. Values are tuples of type, default value,
-        and a docstring describing the purpose of the variable.
+        namespace which are valid. Values are tuples of stored type, assigned
+        type, default value, and a docstring describing the purpose of the variable.
 
         builtins is the value to use for the special __builtins__ key. If not
         defined, the BUILTINS constant attached to this class is used. The
         __builtins__ object is read-only.
         """
         builtins = builtins or self.BUILTINS
 
         assert isinstance(builtins, ReadOnlyDict)
@@ -109,37 +109,47 @@ class GlobalNamespace(dict):
             pass
 
         # The variable isn't present yet. Fall back to VARIABLES.
         default = self._allowed_variables.get(name, None)
         if default is None:
             self.last_name_error = KeyError('global_ns', 'get_unknown', name)
             raise self.last_name_error
 
-        dict.__setitem__(self, name, copy.deepcopy(default[1]))
+        dict.__setitem__(self, name, copy.deepcopy(default[2]))
         return dict.__getitem__(self, name)
 
     def __setitem__(self, name, value):
         if self._allow_all_writes:
             dict.__setitem__(self, name, value)
             return
 
         # We don't need to check for name.isupper() here because LocalNamespace
         # only sends variables our way if isupper() is True.
-        default = self._allowed_variables.get(name, None)
+        stored_type, input_type, default, docs = \
+            self._allowed_variables.get(name, (None, None, None, None))
 
-        if default is None:
+        # Variable is unknown.
+        if stored_type is None:
             self.last_name_error = KeyError('global_ns', 'set_unknown', name,
                 value)
             raise self.last_name_error
 
-        if not isinstance(value, default[0]):
-            self.last_name_error = ValueError('global_ns', 'set_type', name,
-                value, default[0])
-            raise self.last_name_error
+        # If the incoming value is not the type we store, we try to convert
+        # it to that type. This relies on proper coercion rules existing. This
+        # is the responsibility of whoever defined the symbols: a type should
+        # not be in the allowed set if the constructor function for the stored
+        # type does not accept an instance of that type.
+        if not isinstance(value, stored_type):
+            if not isinstance(value, input_type):
+                self.last_name_error = ValueError('global_ns', 'set_type', name,
+                    value, input_type)
+                raise self.last_name_error
+
+            value = stored_type(value)
 
         dict.__setitem__(self, name, value)
 
     @contextmanager
     def allow_all_writes(self):
         """Allow any variable to be written to this instance.
 
         This is used as a context manager. When activated, all writes
--- a/python/mozbuild/mozbuild/frontend/sandbox_symbols.py
+++ b/python/mozbuild/mozbuild/frontend/sandbox_symbols.py
@@ -48,72 +48,72 @@ def doc_to_paragraphs(doc):
 
     return paragraphs
 
 
 # This defines the set of mutable global variables.
 #
 # Each variable is a tuple of:
 #
-#   (type, default_value, docs)
+#   (storage_type, input_types, default_value, docs)
 #
 VARIABLES = {
     # Variables controlling reading of other frontend files.
-    'ASFILES': (list, [],
+    'ASFILES': (list, list, [],
         """ Assembly file sources.
 
         This variable contains a list of files to invoke the assembler on.
         """),
 
-    'DIRS': (list, [],
+    'DIRS': (list, list, [],
         """Child directories to descend into looking for build frontend files.
 
         This works similarly to the DIRS variable in make files. Each str value
         in the list is the name of a child directory. When this file is done
         parsing, the build reader will descend into each listed directory and
         read the frontend file there. If there is no frontend file, an error
         is raised.
 
         Values are relative paths. They can be multiple directory levels
         above or below. Use ".." for parent directories and "/" for path
         delimiters.
         """),
 
-    'PARALLEL_DIRS': (list, [],
+    'PARALLEL_DIRS': (list, list, [],
         """A parallel version of DIRS.
 
         Ideally this variable does not exist. It is provided so a transition
         from recursive makefiles can be made. Once the build system has been
         converted to not use Makefile's for the build frontend, this will
         likely go away.
         """),
 
-    'TOOL_DIRS': (list, [],
+    'TOOL_DIRS': (list, list, [],
         """Like DIRS but for tools.
 
         Tools are for pieces of the build system that aren't required to
         produce a working binary (in theory). They provide things like test
         code and utilities.
         """),
 
-    'TEST_DIRS': (list, [],
+    'TEST_DIRS': (list, list, [],
         """Like DIRS but only for directories that contain test-only code.
 
         If tests are not enabled, this variable will be ignored.
 
         This variable may go away once the transition away from Makefiles is
         complete.
         """),
 
-    'TEST_TOOL_DIRS': (list, [],
+    'TEST_TOOL_DIRS': (list, list, [],
         """TOOL_DIRS that is only executed if tests are enabled.
         """),
 
 
-    'TIERS': (OrderedDict, OrderedDict(),
+    'TIERS': (OrderedDict, dict, OrderedDict(),
         """Defines directories constituting the tier traversal mechanism.
 
         The recursive make backend iteration is organized into tiers. There are
         major tiers (keys in this dict) that correspond roughly to applications
         or libraries being built. e.g. base, nspr, js, platform, app. Within
         each tier are phases like export, libs, and tools. The recursive make
         backend iterates over each phase in the first tier then proceeds to the
         next tier until all tiers are exhausted.
@@ -121,97 +121,97 @@ VARIABLES = {
         Tiers are a way of working around deficiencies in recursive make. These
         will probably disappear once we no longer rely on recursive make for
         the build backend. They will likely be replaced by DIRS.
 
         This variable is typically not populated directly. Instead, it is
         populated by calling add_tier_dir().
         """),
 
-    'EXTERNAL_MAKE_DIRS': (list, [],
+    'EXTERNAL_MAKE_DIRS': (list, list, [],
         """Directories that build with make but don't use moz.build files.
 
         This is like DIRS except it implies that |make| is used to build the
         directory and that the directory does not define itself with moz.build
         files.
         """),
 
-    'PARALLEL_EXTERNAL_MAKE_DIRS': (list, [],
+    'PARALLEL_EXTERNAL_MAKE_DIRS': (list, list, [],
         """Parallel version of EXTERNAL_MAKE_DIRS.
         """),
 
-    'CONFIGURE_SUBST_FILES': (list, [],
+    'CONFIGURE_SUBST_FILES': (list, list, [],
         """Output files that will be generated using configure-like substitution.
 
         This is a substitute for AC_OUTPUT in autoconf. For each path in this
         list, we will search for a file in the srcdir having the name
         {path}.in. The contents of this file will be read and variable patterns
         like @foo@ will be substituted with the values of the AC_SUBST
         variables declared during configure.
         """),
 
-    'MODULE': (unicode, "",
+    'MODULE': (unicode, unicode, "",
         """Module name.
 
         Historically, this variable was used to describe where to install header
         files, but that feature is now handled by EXPORTS_NAMESPACES. Currently
         it is used as the XPIDL module name if XPIDL_MODULE is not defined, but
         using XPIDL_MODULE directly is preferred. MODULE will likely be removed
         in the future.
         """),
 
-    'EXPORTS': (HierarchicalStringList, HierarchicalStringList(),
+    'EXPORTS': (HierarchicalStringList, list, HierarchicalStringList(),
         """List of files to be exported, and in which subdirectories.
 
         EXPORTS is generally used to list the include files to be exported to
         dist/include, but it can be used for other files as well. This variable
         behaves as a list when appending filenames for export in the top-level
         directory. Files can also be appended to a field to indicate which
         subdirectory they should be exported to. For example, to export 'foo.h'
         to the top-level directory, and 'bar.h' to mozilla/dom/, append to
         EXPORTS like so:
 
         EXPORTS += ['foo.h']
         EXPORTS.mozilla.dom += ['bar.h']
         """),
 
-    'PROGRAM' : (unicode, "",
+    'PROGRAM' : (unicode, unicode, "",
         """Compiled executable name.
 
         If the configuration token 'BIN_SUFFIX' is set, its value will be
         automatically appended to PROGRAM. If PROGRAM already ends with
         BIN_SUFFIX, PROGRAM will remain unchanged.
         """),
 
     # IDL Generation.
-    'XPIDL_SOURCES': (list, [],
+    'XPIDL_SOURCES': (list, list, [],
         """XPCOM Interface Definition Files (xpidl).
 
         This is a list of files that define XPCOM interface definitions.
         Entries must be files that exist. Entries are almost certainly .idl
         files.
         """),
 
-    'XPIDL_MODULE': (unicode, "",
+    'XPIDL_MODULE': (unicode, unicode, "",
         """XPCOM Interface Definition Module Name.
 
         This is the name of the .xpt file that is created by linking
         XPIDL_SOURCES together. If unspecified, it defaults to be the same as
         MODULE.
         """),
 
-    'XPIDL_FLAGS': (list, [],
+    'XPIDL_FLAGS': (list, list, [],
         """XPCOM Interface Definition Module Flags.
 
         This is a list of extra flags that are passed to the IDL compiler.
         Typically this is a set of -I flags that denote extra include
         directories to search for included .idl files.
         """),
 
-    'XPCSHELL_TESTS_MANIFESTS': (list, [],
+    'XPCSHELL_TESTS_MANIFESTS': (list, list, [],
         """XPCSHELL Test Manifest list
 
         This is a list of xpcshell.ini manifest files.
         Formerly XPCSHELL_TESTS=
         """),
 }
 
 # The set of functions exposed to the sandbox.
--- a/python/mozbuild/mozbuild/test/frontend/test_sandbox_symbols.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_sandbox_symbols.py
@@ -31,17 +31,17 @@ class TestSymbols(unittest.TestCase):
 
         self.assertGreater(len(lines), 0)
         self.assertGreater(len(lines[0].strip()), 0)
 
         # Last line should be empty.
         self.assertEqual(lines[-1].strip(), '')
 
     def test_documentation_formatting(self):
-        for typ, default, doc in VARIABLES.values():
+        for typ, inp, default, doc in VARIABLES.values():
             self._verify_doc(doc)
 
         for attr, args, doc in FUNCTIONS.values():
             self._verify_doc(doc)
 
         for typ, doc in SPECIAL_VARIABLES.values():
             self._verify_doc(doc)