Bug 1063437 - Remove MozbuildSandbox.normalize_path. r=gps

python/mozbuild/mozbuild/frontend/reader.py

@@ -152,45 +152,7 @@ class MozbuildSandbox(Sandbox):
             return
         Sandbox.__setitem__(self, key, value)
 
-    def normalize_path(self, path, filesystem_absolute=False, srcdir=None):
+    def exec_file(self, path):
-        """Normalizes paths.
-
-        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.
-
-        If the path is not absolute, it will be treated as relative to the
-        currently executing file. If there is no currently executing file, it
-        will be treated as relative to topsrcdir.
-        """
-        if os.path.isabs(path):
-            if filesystem_absolute:
-                return path
-            roots = [self._context.config.topsrcdir]
-            if self._context.config.external_source_dir:
-                roots.append(self._context.config.external_source_dir)
-            for root in roots:
-                # mozpath.join would ignore the self._context.config.topsrcdir
-                # argument if we passed in the absolute path, so omit the
-                # leading /
-                p = mozpath.normpath(mozpath.join(root, path[1:]))
-                if os.path.exists(p):
-                    return p
-            # mozpath.join would ignore the self.condig.topsrcdir argument if
-            # we passed in the absolute path, so omit the leading /
-            return mozpath.normpath(
-                mozpath.join(self._context.config.topsrcdir, path[1:]))
-        elif srcdir:
-            return mozpath.normpath(mozpath.join(srcdir, path))
-        elif self._context.current_path:
-            return mozpath.normpath(mozpath.join(
-                mozpath.dirname(self._context.current_path), path))
-        else:
-            return mozpath.normpath(
-                mozpath.join(self._context.config.topsrcdir, path))
-
-    def exec_file(self, path, filesystem_absolute=False):
         """Override exec_file to normalize paths and restrict file loading.
 
         Paths will be rejected if they do not fall under topsrcdir or one of
@@ -199,13 +161,11 @@ class MozbuildSandbox(Sandbox):
 
         # realpath() is needed for true security. But, this isn't for security
         # protection, so it is omitted.
-        normalized_path = self.normalize_path(path,
+        if not is_read_allowed(path, self._context.config):
-            filesystem_absolute=filesystem_absolute)
-        if not is_read_allowed(normalized_path, self._context.config):
             raise SandboxLoadError(self._context.source_stack,
                 sys.exc_info()[2], illegal_path=path)
 
-        Sandbox.exec_file(self, normalized_path)
+        Sandbox.exec_file(self, path)
 
     def _add_java_jar(self, name):
         """Add a Java JAR build target."""
@@ -279,7 +239,7 @@ class MozbuildSandbox(Sandbox):
         """Include and exec another file within the context of this one."""
 
         # path is a SourcePath, and needs to be coerced to unicode.
-        self.exec_file(unicode(path), filesystem_absolute=True)
+        self.exec_file(unicode(path))
 
     def _warning(self, message):
         # FUTURE consider capturing warnings in a variable instead of printing.
@@ -774,8 +734,7 @@ class BuildReader(object):
         read, a new Context is created and emitted.
         """
         path = mozpath.join(self.config.topsrcdir, 'moz.build')
-        return self.read_mozbuild(path, self.config, read_tiers=True,
+        return self.read_mozbuild(path, self.config, read_tiers=True)
-            filesystem_absolute=True)
 
     def walk_topsrcdir(self):
         """Read all moz.build files in the source tree.
@@ -803,11 +762,11 @@ class BuildReader(object):
         for path, f in finder.find('**/moz.build'):
             path = os.path.join(self.config.topsrcdir, path)
             for s in self.read_mozbuild(path, self.config, descend=False,
-                filesystem_absolute=True, read_tiers=True):
+                read_tiers=True):
                 yield s
 
-    def read_mozbuild(self, path, config, read_tiers=False,
+    def read_mozbuild(self, path, config, read_tiers=False, descend=True,
-            filesystem_absolute=False, descend=True, metadata={}):
+            metadata={}):
         """Read and process a mozbuild file, descending into children.
 
         This starts with a single mozbuild file, executes it, and descends into
@@ -835,7 +794,6 @@ class BuildReader(object):
         self._execution_stack.append(path)
         try:
             for s in self._read_mozbuild(path, config, read_tiers=read_tiers,
-                filesystem_absolute=filesystem_absolute,
                 descend=descend, metadata=metadata):
                 yield s
 
@@ -862,8 +820,7 @@ class BuildReader(object):
             raise BuildReaderError(list(self._execution_stack),
                 sys.exc_info()[2], other_error=e)
 
-    def _read_mozbuild(self, path, config, read_tiers, filesystem_absolute,
+    def _read_mozbuild(self, path, config, read_tiers, descend, metadata):
-            descend, metadata):
         path = mozpath.normpath(path)
         log(self._log, logging.DEBUG, 'read_mozbuild', {'path': path},
             'Reading file: {path}')
@@ -899,7 +856,7 @@ class BuildReader(object):
 
         context = Context(VARIABLES, config)
         sandbox = MozbuildSandbox(context, metadata=metadata)
-        sandbox.exec_file(path, filesystem_absolute=filesystem_absolute)
+        sandbox.exec_file(path)
         context.execution_time = time.time() - time_start
 
         if self._sandbox_post_eval_cb:
@@ -993,8 +950,7 @@ class BuildReader(object):
                 continue
 
             for res in self.read_mozbuild(child_path, context.config,
-                read_tiers=False, filesystem_absolute=True,
+                read_tiers=False, metadata=child_metadata):
-                metadata=child_metadata):
                 yield res
 
         self._execution_stack.pop()

python/mozbuild/mozbuild/test/frontend/test_reader.py

@@ -58,7 +58,7 @@ class TestBuildReader(unittest.TestCase):
         self.assertTrue(os.path.exists(path))
 
         contexts = list(reader.read_mozbuild(path, reader.config,
-            filesystem_absolute=True, descend=False))
+            descend=False))
 
         self.assertEqual(len(contexts), 1)
 

python/mozbuild/mozbuild/test/frontend/test_sandbox.py

@@ -29,6 +29,7 @@ from mozbuild.frontend.context import (
 )
 
 from mozbuild.test.common import MockConfig
+from types import StringTypes
 
 import mozpack.path as mozpath
 
@@ -121,6 +122,24 @@ class TestSandbox(unittest.TestCase):
         self.assertEqual(e.args[0], 'Cannot reassign builtins')
 
 
+class TestedSandbox(MozbuildSandbox):
+    '''Version of MozbuildSandbox with a little more convenience for testing.
+
+    It automatically normalizes paths given to exec_file and exec_source. This
+    helps simplify the test code.
+    '''
+    def normalize_path(self, path):
+        return mozpath.normpath(
+            mozpath.join(self._context.config.topsrcdir, path))
+
+    def exec_file(self, path):
+        super(TestedSandbox, self).exec_file(self.normalize_path(path))
+
+    def exec_source(self, source, path=''):
+        super(TestedSandbox, self).exec_source(source,
+            self.normalize_path(path) if path else '')
+
+
 class TestMozbuildSandbox(unittest.TestCase):
     def sandbox(self, data_path=None, metadata={}):
         config = None
@@ -130,7 +149,7 @@ class TestMozbuildSandbox(unittest.TestCase):
         else:
             config = MockConfig()
 
-        return MozbuildSandbox(Context(VARIABLES, config), metadata)
+        return TestedSandbox(Context(VARIABLES, config), metadata)
 
     def test_default_state(self):
         sandbox = self.sandbox()
@@ -330,7 +349,7 @@ Template([
     'foo.cpp',
 ])
 '''
-        sandbox2.exec_source(source, sandbox.normalize_path('foo.mozbuild'))
+        sandbox2.exec_source(source, 'foo.mozbuild')
 
         self.assertEqual(sandbox2._context, {
             'SOURCES': ['foo.cpp'],
@@ -348,7 +367,7 @@ Template([
 ])
 SOURCES += ['hoge.cpp']
 '''
-        sandbox2.exec_source(source, sandbox.normalize_path('foo.mozbuild'))
+        sandbox2.exec_source(source, 'foo.mozbuild')
 
         self.assertEqual(sandbox2._context, {
             'SOURCES': ['qux.cpp', 'bar.cpp', 'foo.cpp', 'hoge.cpp'],
@@ -362,7 +381,7 @@ TemplateError([
 ])
 '''
         with self.assertRaises(SandboxExecutionError) as se:
-            sandbox2.exec_source(source, sandbox.normalize_path('foo.mozbuild'))
+            sandbox2.exec_source(source, 'foo.mozbuild')
 
         e = se.exception
         self.assertIsInstance(e.exc_value, KeyError)
@@ -379,7 +398,7 @@ illegal = True
 TemplateGlobalVariable()
 '''
         with self.assertRaises(SandboxExecutionError) as se:
-            sandbox2.exec_source(source, sandbox.normalize_path('foo.mozbuild'))
+            sandbox2.exec_source(source, 'foo.mozbuild')
 
         e = se.exception
         self.assertIsInstance(e.exc_value, NameError)
@@ -392,10 +411,10 @@ TemplateGlobalVariable()
 DIRS += ['foo']
 TemplateGlobalUPPERVariable()
 '''
-        sandbox2.exec_source(source, sandbox.normalize_path('foo.mozbuild'))
+        sandbox2.exec_source(source, 'foo.mozbuild')
         self.assertEqual(sandbox2._context, {
             'SOURCES': [],
-            'DIRS': [sandbox.normalize_path('foo')],
+            'DIRS': [sandbox2.normalize_path('foo')],
         })
 
         # However, the result of the template is mixed with the global
@@ -409,7 +428,7 @@ TemplateInherit([
 ])
 SOURCES += ['hoge.cpp']
 '''
-        sandbox2.exec_source(source, sandbox.normalize_path('foo.mozbuild'))
+        sandbox2.exec_source(source, 'foo.mozbuild')
 
         self.assertEqual(sandbox2._context, {
             'SOURCES': ['qux.cpp', 'bar.cpp', 'foo.cpp', 'hoge.cpp'],
@@ -427,7 +446,7 @@ def foo():
 '''
 
         with self.assertRaises(SandboxExecutionError) as se:
-            sandbox2.exec_source(source, sandbox.normalize_path('foo.mozbuild'))
+            sandbox2.exec_source(source, 'foo.mozbuild')
 
         e = se.exception
         self.assertIsInstance(e.exc_value, NameError)
@@ -444,7 +463,7 @@ def Template():
     pass
 '''
         with self.assertRaises(SandboxExecutionError) as se:
-            sandbox2.exec_source(source, sandbox.normalize_path('foo.mozbuild'))
+            sandbox2.exec_source(source, 'foo.mozbuild')
 
         e = se.exception
         self.assertIsInstance(e.exc_value, KeyError)
@@ -469,8 +488,7 @@ def Template():
             source = 'foo("a", "b")'
 
             with self.assertRaises(SandboxExecutionError) as se:
-                sandbox.exec_source(source,
+                sandbox.exec_source(source, 'foo.mozbuild')
-                    sandbox.normalize_path('foo.mozbuild'))
 
             e = se.exception
             self.assertIsInstance(e.exc_value, ValueError)
@@ -479,15 +497,14 @@ def Template():
             source = 'foo(1, "b")'
 
             with self.assertRaises(SandboxExecutionError) as se:
-                sandbox.exec_source(source,
+                sandbox.exec_source(source, 'foo.mozbuild')
-                    sandbox.normalize_path('foo.mozbuild'))
 
             e = se.exception
             self.assertIsInstance(e.exc_value, ValueError)
 
             sandbox = self.sandbox()
             source = 'a = foo(1, 2)'
-            sandbox.exec_source(source, sandbox.normalize_path('foo.mozbuild'))
+            sandbox.exec_source(source, 'foo.mozbuild')
 
             self.assertEquals(sandbox['a'], (Foo, int))
         finally: