Bug 1063437 - Remove MozbuildSandbox.normalize_path. r=gps
authorMike Hommey <mh+mozilla@glandium.org>
Thu, 02 Oct 2014 09:14:08 +0900
changeset 231506 3f9e0be6d08f51e87aa4b0aacf9022bf844f6128
parent 231505 8e70d430c86a6d9653756e8c1090f610bf2dc17f
child 231507 320de51add949cc1fe5a28d047e0326637da1670
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps
bugs1063437
milestone35.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1063437 - Remove MozbuildSandbox.normalize_path. r=gps
python/mozbuild/mozbuild/frontend/reader.py
python/mozbuild/mozbuild/test/frontend/test_reader.py
python/mozbuild/mozbuild/test/frontend/test_sandbox.py
--- a/python/mozbuild/mozbuild/frontend/reader.py
+++ b/python/mozbuild/mozbuild/frontend/reader.py
@@ -147,70 +147,30 @@ class MozbuildSandbox(Sandbox):
         if key in SPECIAL_VARIABLES or key in FUNCTIONS:
             raise KeyError()
         if key in self.exports:
             self._context[key] = value
             self.exports.remove(key)
             return
         Sandbox.__setitem__(self, key, value)
 
-    def normalize_path(self, path, filesystem_absolute=False, srcdir=None):
-        """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):
+    def exec_file(self, path):
         """Override exec_file to normalize paths and restrict file loading.
 
         Paths will be rejected if they do not fall under topsrcdir or one of
         the external roots.
         """
 
         # realpath() is needed for true security. But, this isn't for security
         # protection, so it is omitted.
-        normalized_path = self.normalize_path(path,
-            filesystem_absolute=filesystem_absolute)
-        if not is_read_allowed(normalized_path, self._context.config):
+        if not is_read_allowed(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."""
         if not name:
             raise Exception('Java JAR cannot be registered without a name')
 
         if '/' in name or '\\' in name or '.jar' in name:
             raise Exception('Java JAR names must not include slashes or'
@@ -274,17 +234,17 @@ class MozbuildSandbox(Sandbox):
         if 'exports' in self.metadata:
             for key in self.metadata['exports']:
                 self.metadata['exports'][key] = self[key]
 
     def _include(self, path):
         """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.
         print('WARNING: %s' % message, file=sys.stderr)
 
     def _error(self, message):
         raise SandboxCalledError(self._context.source_stack, message)
 
@@ -769,18 +729,17 @@ class BuildReader(object):
 
         This starts with the tree's top-most moz.build file and descends into
         all linked moz.build files until all relevant files have been evaluated.
 
         This is a generator of Context instances. As each moz.build file is
         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,
-            filesystem_absolute=True)
+        return self.read_mozbuild(path, self.config, read_tiers=True)
 
     def walk_topsrcdir(self):
         """Read all moz.build files in the source tree.
 
         This is different from read_topsrcdir() in that this version performs a
         filesystem walk to discover every moz.build file rather than relying on
         data from executed moz.build files to drive traversal.
 
@@ -798,21 +757,21 @@ class BuildReader(object):
         }
 
         finder = FileFinder(self.config.topsrcdir, find_executables=False,
             ignore=ignore)
 
         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,
-            filesystem_absolute=False, descend=True, metadata={}):
+    def read_mozbuild(self, path, config, read_tiers=False, descend=True,
+            metadata={}):
         """Read and process a mozbuild file, descending into children.
 
         This starts with a single mozbuild file, executes it, and descends into
         other referenced files per our traversal logic.
 
         The traversal logic is to iterate over the *DIRS variables, treating
         each element as a relative directory path. For each encountered
         directory, we will open the moz.build file located in that
@@ -830,17 +789,16 @@ class BuildReader(object):
         injecting state and annotations into moz.build files that is
         independent of the sandbox's execution context.
 
         Traversal is performed depth first (for no particular reason).
         """
         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
 
         except BuildReaderError as bre:
             raise bre
 
         except SandboxCalledError as sce:
             raise BuildReaderError(list(self._execution_stack),
@@ -857,18 +815,17 @@ class BuildReader(object):
         except SandboxValidationError as ve:
             raise BuildReaderError(list(self._execution_stack),
                 sys.exc_info()[2], validation_error=ve)
 
         except Exception as e:
             raise BuildReaderError(list(self._execution_stack),
                 sys.exc_info()[2], other_error=e)
 
-    def _read_mozbuild(self, path, config, read_tiers, filesystem_absolute,
-            descend, metadata):
+    def _read_mozbuild(self, path, config, read_tiers, descend, metadata):
         path = mozpath.normpath(path)
         log(self._log, logging.DEBUG, 'read_mozbuild', {'path': path},
             'Reading file: {path}')
 
         if path in self._read_files:
             log(self._log, logging.WARNING, 'read_already', {'path': path},
                 'File already read. Skipping: {path}')
             return
@@ -894,17 +851,17 @@ class BuildReader(object):
                 not config.substs.get('JS_STANDALONE'):
             config = ConfigEnvironment.from_config_status(
                 mozpath.join(topobjdir, reldir, 'config.status'))
             config.topobjdir = topobjdir
             config.external_source_dir = None
 
         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:
             self._sandbox_post_eval_cb(context)
 
         # We first collect directories populated in variables.
         dir_vars = ['DIRS']
 
@@ -988,13 +945,12 @@ class BuildReader(object):
                 raise SandboxValidationError(
                     'Attempting to process file outside of allowed paths: %s' %
                         child_path, context)
 
             if not descend:
                 continue
 
             for res in self.read_mozbuild(child_path, context.config,
-                read_tiers=False, filesystem_absolute=True,
-                metadata=child_metadata):
+                read_tiers=False, metadata=child_metadata):
                 yield res
 
         self._execution_stack.pop()
--- a/python/mozbuild/mozbuild/test/frontend/test_reader.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_reader.py
@@ -53,17 +53,17 @@ class TestBuildReader(unittest.TestCase)
 
     def test_dirs_traversal_no_descend(self):
         reader = self.reader('traversal-simple')
 
         path = mozpath.join(reader.config.topsrcdir, 'moz.build')
         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)
 
     def test_dirs_traversal_all_variables(self):
         reader = self.reader('traversal-all-vars', enable_tests=True)
 
         contexts = list(reader.read_topsrcdir())
         self.assertEqual(len(contexts), 3)
--- a/python/mozbuild/mozbuild/test/frontend/test_sandbox.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_sandbox.py
@@ -24,16 +24,17 @@ from mozbuild.frontend.sandbox import (
 from mozbuild.frontend.context import (
     Context,
     FUNCTIONS,
     SPECIAL_VARIABLES,
     VARIABLES,
 )
 
 from mozbuild.test.common import MockConfig
+from types import StringTypes
 
 import mozpack.path as mozpath
 
 test_data_path = mozpath.abspath(mozpath.dirname(__file__))
 test_data_path = mozpath.join(test_data_path, 'data')
 
 
 class TestSandbox(unittest.TestCase):
@@ -116,26 +117,44 @@ class TestSandbox(unittest.TestCase):
 
         e = se.exception
         self.assertIsInstance(e.exc_value, KeyError)
 
         e = se.exception.exc_value
         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
 
         if data_path is not None:
             config = MockConfig(mozpath.join(test_data_path, data_path))
         else:
             config = MockConfig()
 
-        return MozbuildSandbox(Context(VARIABLES, config), metadata)
+        return TestedSandbox(Context(VARIABLES, config), metadata)
 
     def test_default_state(self):
         sandbox = self.sandbox()
         sandbox._context.add_source(sandbox.normalize_path('moz.build'))
         config = sandbox._context.config
 
         self.assertEqual(sandbox['TOPSRCDIR'], config.topsrcdir)
         self.assertEqual(sandbox['TOPOBJDIR'], config.topobjdir)
@@ -325,17 +344,17 @@ class TestMozbuildSandbox(unittest.TestC
         sandbox.exec_file('templates.mozbuild')
 
         sandbox2 = self.sandbox(metadata={'templates': sandbox.templates})
         source = '''
 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'],
             'DIRS': [],
         })
 
         sandbox2 = self.sandbox(metadata={'templates': sandbox.templates})
         source = '''
@@ -343,78 +362,78 @@ SOURCES += ['qux.cpp']
 Template([
     'bar.cpp',
     'foo.cpp',
 ],[
     'foo',
 ])
 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'],
             'DIRS': [sandbox.normalize_path('foo')],
         })
 
         sandbox2 = self.sandbox(metadata={'templates': sandbox.templates})
         source = '''
 TemplateError([
     'foo.cpp',
 ])
 '''
         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)
 
         e = se.exception.exc_value
         self.assertEqual(e.args[0], 'global_ns')
         self.assertEqual(e.args[1], 'set_unknown')
 
         # TemplateGlobalVariable tries to access 'illegal' but that is expected
         # to throw.
         sandbox2 = self.sandbox(metadata={'templates': sandbox.templates})
         source = '''
 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)
 
         # TemplateGlobalUPPERVariable sets SOURCES with DIRS, but the context
         # used when running the template is not expected to access variables
         # from the global context.
         sandbox2 = self.sandbox(metadata={'templates': sandbox.templates})
         source = '''
 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
         # context.
         sandbox2 = self.sandbox(metadata={'templates': sandbox.templates})
         source = '''
 SOURCES += ['qux.cpp']
 TemplateInherit([
     'bar.cpp',
     'foo.cpp',
 ])
 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'],
             'USE_LIBS': ['foo'],
             'DIRS': [],
         })
 
         # Template names must be CamelCase. Here, we can define the template
@@ -422,34 +441,34 @@ SOURCES += ['hoge.cpp']
         sandbox2 = self.sandbox(metadata={'templates': sandbox.templates})
         source = '''
 @template
 def foo():
     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, NameError)
 
         e = se.exception.exc_value
         self.assertEqual(e.message,
             'Template function names must be CamelCase.')
 
         # Template names must not already be registered.
         sandbox2 = self.sandbox(metadata={'templates': sandbox.templates})
         source = '''
 @template
 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)
 
         e = se.exception.exc_value
         self.assertEqual(e.message,
             'A template named "Template" was already declared in %s.' %
             sandbox.normalize_path('templates.mozbuild'))
@@ -464,35 +483,33 @@ def Template():
             'foo': (lambda self: foo, (Foo, int), ''),
         })
 
         try:
             sandbox = self.sandbox()
             source = 'foo("a", "b")'
 
             with self.assertRaises(SandboxExecutionError) as se:
-                sandbox.exec_source(source,
-                    sandbox.normalize_path('foo.mozbuild'))
+                sandbox.exec_source(source, 'foo.mozbuild')
 
             e = se.exception
             self.assertIsInstance(e.exc_value, ValueError)
 
             sandbox = self.sandbox()
             source = 'foo(1, "b")'
 
             with self.assertRaises(SandboxExecutionError) as se:
-                sandbox.exec_source(source,
-                    sandbox.normalize_path('foo.mozbuild'))
+                sandbox.exec_source(source, '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:
             del FUNCTIONS['foo']
 
 
 if __name__ == '__main__':
     main()