Bug 1071568 - Only pass down templates to the sandbox context when calling moz.build templates. r=mshal
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 09 Oct 2014 10:14:26 +0900
changeset 209506 964eb640d4a8f38096239de314edac18be3385f5
parent 209505 b8cd2e3b170465af2cfc6d2dd41844a2e52ab2f3
child 209507 b37bddc0adfa78fbaadda9925f643602cd6debb4
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersmshal
bugs1071568
milestone35.0a1
Bug 1071568 - Only pass down templates to the sandbox context when calling moz.build templates. r=mshal It's not entirely clear passing down all the metadata makes sense. On the other hand, when creating the template execution sandbox, passing down exports does assign the value for the exported variable in that execution context. When that context is merged with the caller sandbox context, the exported variable is reassigned, even if the value is not modified. Then, if the caller sandbox itself reassigns the exported variable, it fails because calling a template already did it once, unexpectedly. Not passing down exported variables makes the template execution sandbox never set those exported variables, so that they are not merged back. The caller sandbox can then properly reassign the exported variable.
python/mozbuild/mozbuild/frontend/reader.py
python/mozbuild/mozbuild/test/frontend/test_sandbox.py
--- a/python/mozbuild/mozbuild/frontend/reader.py
+++ b/python/mozbuild/mozbuild/frontend/reader.py
@@ -345,17 +345,19 @@ class MozbuildSandbox(Sandbox):
         func, code, path = template
 
         def template_function(*args, **kwargs):
             context = TemplateContext(VARIABLES, self._context.config)
             context.add_source(self._context.current_path)
             for p in self._context.all_paths:
                 context.add_source(p)
 
-            sandbox = MozbuildSandbox(context, self.metadata)
+            sandbox = MozbuildSandbox(context, {
+                'templates': self.metadata.get('templates', {})
+            })
             for k, v in inspect.getcallargs(func, *args, **kwargs).items():
                 sandbox[k] = v
 
             sandbox.exec_source(code, path)
 
             # This is gross, but allows the merge to happen. Eventually, the
             # merging will go away and template contexts emitted independently.
             klass = self._context.__class__
--- a/python/mozbuild/mozbuild/test/frontend/test_sandbox.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_sandbox.py
@@ -206,25 +206,36 @@ class TestMozbuildSandbox(unittest.TestC
     def test_special_variables(self):
         sandbox = self.sandbox()
 
         for k in SPECIAL_VARIABLES:
             with self.assertRaises(KeyError):
                 sandbox[k] = 0
 
     def test_exec_source_reassign_exported(self):
+        template_sandbox = self.sandbox(data_path='templates')
+
+        # Templates need to be defined in actual files because of
+        # inspect.getsourcelines.
+        template_sandbox.exec_file('templates.mozbuild')
+
         config = MockConfig()
 
         exports = {'DIST_SUBDIR': 'browser'}
 
-        sandbox = MozbuildSandbox(Context(VARIABLES, config),
-            metadata={'exports': exports})
+        sandbox = TestedSandbox(Context(VARIABLES, config), metadata={
+            'exports': exports,
+            'templates': template_sandbox.templates,
+        })
 
         self.assertEqual(sandbox['DIST_SUBDIR'], 'browser')
 
+        # Templates should not interfere
+        sandbox.exec_source('Template([])', 'foo.mozbuild')
+
         sandbox.exec_source('DIST_SUBDIR = "foo"')
         with self.assertRaises(SandboxExecutionError) as se:
           sandbox.exec_source('DIST_SUBDIR = "bar"')
 
         self.assertEqual(sandbox['DIST_SUBDIR'], 'foo')
         e = se.exception
         self.assertIsInstance(e.exc_value, KeyError)