Bug 1316250 - Don't pretend imported modules in templates are inherited by functions they contain. r?chmanchester draft
authorMike Hommey <mh+mozilla@glandium.org>
Wed, 09 Nov 2016 15:22:17 +0900
changeset 435815 3cb4e1cb6073045baeb1a6e79e5b4de6956fae18
parent 435814 d76e0d2be865f40a4bf86fddd1d5430f74b7b5bf
child 435816 2f13db72e87a6d83c5f7302344ef6e05fada873a
push id35126
push userbmo:mh+mozilla@glandium.org
push dateWed, 09 Nov 2016 06:43:04 +0000
reviewerschmanchester
bugs1316250
milestone52.0a1
Bug 1316250 - Don't pretend imported modules in templates are inherited by functions they contain. r?chmanchester The current state of python configure sandbox execution is that if a template imports a module, and a function defined in the template tries to use the module, it doesn't work. Ideally, it would, but rather than try to fix this corner case, we remove the unit tests that assume it works (and consequently pass for half bad reasons), and add a unit test so that the behavior doesn't change unwillingly.
python/mozbuild/mozbuild/test/configure/test_configure.py
python/mozbuild/mozbuild/test/configure/test_lint.py
--- a/python/mozbuild/mozbuild/test/configure/test_configure.py
+++ b/python/mozbuild/mozbuild/test/configure/test_configure.py
@@ -334,16 +334,34 @@ class TestConfigure(unittest.TestCase):
         )
 
         self.assertIs(sandbox['foo'](), sandbox)
 
         # Nothing leaked from the function being executed
         self.assertEquals(sandbox.keys(), ['__builtins__', 'foo'])
         self.assertEquals(sandbox['__builtins__'], ConfigureSandbox.BUILTINS)
 
+        exec_(textwrap.dedent('''
+            @template
+            @imports('sys')
+            def foo():
+                @depends(when=True)
+                def bar():
+                    return sys
+                return bar
+            bar = foo()'''),
+            sandbox
+        )
+
+        with self.assertRaises(NameError) as e:
+            sandbox._depends[sandbox['bar']].result
+
+        self.assertEquals(e.exception.message,
+                          "global name 'sys' is not defined")
+
     def test_apply_imports(self):
         imports = []
 
         class CountApplyImportsSandbox(ConfigureSandbox):
             def _apply_imports(self, *args, **kwargs):
                 imports.append((args, kwargs))
                 super(CountApplyImportsSandbox, self)._apply_imports(
                     *args, **kwargs)
--- a/python/mozbuild/mozbuild/test/configure/test_lint.py
+++ b/python/mozbuild/mozbuild/test/configure/test_lint.py
@@ -84,38 +84,16 @@ class TestLint(unittest.TestCase):
                 tmpl()
             '''):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
                           "`bar` depends on '--help' and `foo`. "
                           "`foo` must depend on '--help'")
 
-        with self.assertRaises(ConfigureError) as e:
-            with self.moz_configure('''
-                @template
-                @imports('os')
-                def tmpl():
-                    option('--foo', help='foo')
-                    @depends('--foo')
-                    def foo(value):
-                        os
-                        return value
-
-                    @depends('--help', foo)
-                    def bar(help, foo):
-                        return
-                tmpl()
-            '''):
-                self.lint_test()
-
-        self.assertEquals(e.exception.message,
-                          "`bar` depends on '--help' and `foo`. "
-                          "`foo` must depend on '--help'")
-
         with self.moz_configure('''
             option('--foo', help='foo')
             @depends('--foo')
             def foo(value):
                 return value
 
             include(foo)
         '''):
@@ -131,32 +109,11 @@ class TestLint(unittest.TestCase):
 
                 include(foo)
             '''):
                 self.lint_test()
 
         self.assertEquals(e.exception.message,
                           "Missing @depends for `foo`: '--help'")
 
-        with self.assertRaises(ConfigureError) as e:
-            with self.moz_configure('''
-                @template
-                @imports('os')
-                def tmpl():
-                    option('--foo', help='foo')
-                    @depends('--foo')
-                    def foo(value):
-                        os
-                        return value
-                    return foo
-
-                foo = tmpl()
-
-                include(foo)
-            '''):
-                self.lint_test()
-
-        self.assertEquals(e.exception.message,
-                          "Missing @depends for `foo`: '--help'")
-
 
 if __name__ == '__main__':
     main()