Bug 863069 - Part 5: Require sorted lists in moz.build files; r=ted
authorGregory Szorc <gps@mozilla.com>
Tue, 14 May 2013 15:13:37 -0700
changeset 139298 8fbd99874bad6860aa6aa0618554c79b54ca5d23
parent 139297 da470133a9ba07b9e5b4a6d1d93bfa8b84544ef5
child 139299 efbb1fb5e563a527ef13c2f2a457f3b1fb22b739
push id3911
push userakeybl@mozilla.com
push dateMon, 24 Jun 2013 20:17:26 +0000
treeherdermozilla-aurora@7e26ca8db92b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersted
bugs863069
milestone24.0a1
Bug 863069 - Part 5: Require sorted lists in moz.build files; r=ted
python/mozbuild/mozbuild/frontend/sandbox_symbols.py
python/mozbuild/mozbuild/test/backend/data/variable_passthru/moz.build
python/mozbuild/mozbuild/test/backend/test_recursivemake.py
python/mozbuild/mozbuild/test/frontend/data/variable-passthru/moz.build
python/mozbuild/mozbuild/test/frontend/data/xpcshell_manifests/moz.build
python/mozbuild/mozbuild/test/frontend/test_emitter.py
python/mozbuild/mozbuild/test/frontend/test_sandbox.py
python/mozbuild/mozbuild/test/test_util.py
python/mozbuild/mozbuild/util.py
--- a/python/mozbuild/mozbuild/frontend/sandbox_symbols.py
+++ b/python/mozbuild/mozbuild/frontend/sandbox_symbols.py
@@ -13,17 +13,20 @@ special meaning in the frontend files fo
 
 If you are looking for the absolute authority on what the global namespace in
 the Sandbox consists of, you've come to the right place.
 """
 
 from __future__ import unicode_literals
 
 from collections import OrderedDict
-from mozbuild.util import HierarchicalStringList
+from mozbuild.util import (
+    HierarchicalStringList,
+    StrictOrderingOnAppendList,
+)
 
 
 def doc_to_paragraphs(doc):
     """Take a documentation string and converts it to paragraphs.
 
     This normalizes the inline strings in VARIABLES and elsewhere in this file.
 
     It returns a list of paragraphs. It is up to the caller to insert newlines
@@ -52,18 +55,18 @@ def doc_to_paragraphs(doc):
 # This defines the set of mutable global variables.
 #
 # Each variable is a tuple of:
 #
 #   (storage_type, input_types, default_value, docs)
 #
 VARIABLES = {
     # Variables controlling reading of other frontend files.
-    'ASFILES': (list, list, [],
-        """ Assembly file sources.
+    'ASFILES': (StrictOrderingOnAppendList, list, [],
+        """Assembly file sources.
 
         This variable contains a list of files to invoke the assembler on.
         """),
 
     '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
@@ -133,17 +136,17 @@ VARIABLES = {
         directory and that the directory does not define itself with moz.build
         files.
         """),
 
     'PARALLEL_EXTERNAL_MAKE_DIRS': (list, list, [],
         """Parallel version of EXTERNAL_MAKE_DIRS.
         """),
 
-    'CONFIGURE_SUBST_FILES': (list, list, [],
+    'CONFIGURE_SUBST_FILES': (StrictOrderingOnAppendList, 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.
         """),
@@ -177,17 +180,17 @@ VARIABLES = {
         """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, list, [],
+    'XPIDL_SOURCES': (StrictOrderingOnAppendList, 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, unicode, "",
@@ -201,17 +204,17 @@ VARIABLES = {
     '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, list, [],
+    'XPCSHELL_TESTS_MANIFESTS': (StrictOrderingOnAppendList, 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/backend/data/variable_passthru/moz.build
+++ b/python/mozbuild/mozbuild/test/backend/data/variable_passthru/moz.build
@@ -1,9 +1,9 @@
 # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
 # Any copyright is dedicated to the Public Domain.
 # http://creativecommons.org/publicdomain/zero/1.0/
 
-XPIDL_SOURCES = ['foo.idl', 'bar.idl', 'biz.idl']
+XPIDL_SOURCES = ['bar.idl', 'biz.idl', 'foo.idl']
 XPIDL_MODULE = 'module_name'
 XPIDL_FLAGS = ['-Idir1', '-Idir2', '-Idir3']
 
-ASFILES = ['foo.asm', 'bar.s']
+ASFILES = ['bar.s', 'foo.asm']
--- a/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
+++ b/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
@@ -133,26 +133,34 @@ class TestRecursiveMakeBackend(BackendTe
     def test_variable_passthru(self):
         """Ensure variable passthru is written out correctly."""
         env = self._consume('variable_passthru', RecursiveMakeBackend)
 
         backend_path = os.path.join(env.topobjdir, 'backend.mk')
         lines = [l.strip() for l in open(backend_path, 'rt').readlines()[2:-1]]
 
         expected = {
-            'ASFILES': ['ASFILES += foo.asm', 'ASFILES += bar.s'],
-            'XPIDL_FLAGS': ['XPIDL_FLAGS += -Idir1',
-                            'XPIDL_FLAGS += -Idir2',
-                            'XPIDL_FLAGS += -Idir3',
-                            ],
-            'XPIDL_MODULE': ['XPIDL_MODULE := module_name'],
-            'XPIDLSRCS': ['XPIDLSRCS += foo.idl',
-                          'XPIDLSRCS += bar.idl',
-                          'XPIDLSRCS += biz.idl']
-            }
+            'ASFILES': [
+                'ASFILES += bar.s',
+                'ASFILES += foo.asm',
+            ],
+            'XPIDL_FLAGS': [
+                'XPIDL_FLAGS += -Idir1',
+                'XPIDL_FLAGS += -Idir2',
+                'XPIDL_FLAGS += -Idir3',
+            ],
+            'XPIDL_MODULE': [
+                'XPIDL_MODULE := module_name'
+            ],
+            'XPIDLSRCS': [
+                'XPIDLSRCS += bar.idl',
+                'XPIDLSRCS += biz.idl',
+                'XPIDLSRCS += foo.idl',
+            ]
+        }
 
         for var, val in expected.items():
             # print("test_variable_passthru[%s]" % (var))
             found = [str for str in lines if str.startswith(var)]
             self.assertEqual(found, val)
 
     def test_exports(self):
         """Ensure EXPORTS is written out correctly."""
--- a/python/mozbuild/mozbuild/test/frontend/data/variable-passthru/moz.build
+++ b/python/mozbuild/mozbuild/test/frontend/data/variable-passthru/moz.build
@@ -1,9 +1,9 @@
 # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
 # Any copyright is dedicated to the Public Domain.
 # http://creativecommons.org/publicdomain/zero/1.0/
 
-XPIDL_SOURCES += ['foo.idl', 'bar.idl', 'biz.idl']
+XPIDL_SOURCES += ['bar.idl', 'biz.idl', 'foo.idl']
 XPIDL_MODULE = 'module_name'
 XPIDL_FLAGS += ['-Idir1', '-Idir2', '-Idir3']
 
-ASFILES += ['tans.s', 'fans.asm']
+ASFILES += ['fans.asm', 'tans.s']
--- a/python/mozbuild/mozbuild/test/frontend/data/xpcshell_manifests/moz.build
+++ b/python/mozbuild/mozbuild/test/frontend/data/xpcshell_manifests/moz.build
@@ -1,14 +1,14 @@
 # -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
 # vim: set filetype=python:
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
-XPCSHELL_TESTS_MANIFESTS += ['foo/xpcshell.ini', 'bar/xpcshell.ini']
+XPCSHELL_TESTS_MANIFESTS += ['bar/xpcshell.ini', 'foo/xpcshell.ini']
 XPCSHELL_TESTS_MANIFESTS += ['tans/xpcshell.ini']
 
 if CONFIG['DISABLED_VAR']:
     XPCSHELL_TESTS_MANIFESTS += ['disabled_var/xpcshell.ini']
 
 if not CONFIG['DISABLED_VAR']:
     XPCSHELL_TESTS_MANIFESTS += ['enabled_var/xpcshell.ini']
--- a/python/mozbuild/mozbuild/test/frontend/test_emitter.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_emitter.py
@@ -120,18 +120,18 @@ class TestEmitterBasic(unittest.TestCase
         reader = self.reader('variable-passthru')
         objs = self.read_topsrcdir(reader)
 
         self.assertEqual(len(objs), 2)
         self.assertIsInstance(objs[0], DirectoryTraversal)
         self.assertIsInstance(objs[1], VariablePassthru)
 
         wanted = dict(
-            ASFILES=['tans.s', 'fans.asm'],
-            XPIDLSRCS=['foo.idl', 'bar.idl', 'biz.idl'],
+            ASFILES=['fans.asm', 'tans.s'],
+            XPIDLSRCS=['bar.idl', 'biz.idl', 'foo.idl'],
             XPIDL_MODULE='module_name',
             XPIDL_FLAGS=['-Idir1', '-Idir2', '-Idir3'],
             )
 
         variables = objs[1].variables
         self.assertEqual(len(variables), len(wanted))
 
         for var, val in wanted.items():
--- a/python/mozbuild/mozbuild/test/frontend/test_sandbox.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_sandbox.py
@@ -291,19 +291,19 @@ add_tier_dir('t1', 'bat', static=True)
             sandbox.exec_source('error("This is an error.")', 'test.py')
 
         e = sce.exception
         self.assertEqual(e.message, 'This is an error.')
 
     def test_substitute_config_files(self):
         sandbox = self.sandbox()
 
-        sandbox.exec_source('CONFIGURE_SUBST_FILES += ["foo", "bar"]',
+        sandbox.exec_source('CONFIGURE_SUBST_FILES += ["bar", "foo"]',
             'test.py')
-        self.assertEqual(sandbox['CONFIGURE_SUBST_FILES'], ['foo', 'bar'])
+        self.assertEqual(sandbox['CONFIGURE_SUBST_FILES'], ['bar', 'foo'])
 
     def test_invalid_utf8_substs(self):
         """Ensure invalid UTF-8 in substs is converted with an error."""
 
         config = MockConfig()
         # This is really mbcs. It's a bunch of invalid UTF-8.
         config.substs['BAD_UTF8'] = b'\x83\x81\x83\x82\x3A'
 
--- a/python/mozbuild/mozbuild/test/test_util.py
+++ b/python/mozbuild/mozbuild/test/test_util.py
@@ -16,16 +16,18 @@ from mozunit import (
 )
 
 from mozbuild.util import (
     FileAvoidWrite,
     hash_file,
     resolve_target_to_make,
     MozbuildDeletionError,
     HierarchicalStringList,
+    StrictOrderingOnAppendList,
+    UnsortedError,
 )
 
 if sys.version_info[0] == 3:
     str_type = 'str'
 else:
     str_type = 'unicode'
 
 data_path = os.path.abspath(os.path.dirname(__file__))
@@ -212,10 +214,74 @@ class TestHierarchicalStringList(unittes
                          "Expected a list of strings, not an element of "
                          "<type 'bool'>")
 
     def test_del_exports(self):
         with self.assertRaises(MozbuildDeletionError) as mde:
             self.EXPORTS.foo += ['bar.h']
             del self.EXPORTS.foo
 
+    def test_unsorted_appends(self):
+        with self.assertRaises(UnsortedError) as ee:
+            self.EXPORTS += ['foo.h', 'bar.h']
+
+
+class TestStrictOrderingOnAppendList(unittest.TestCase):
+    def test_init(self):
+        l = StrictOrderingOnAppendList()
+        self.assertEqual(len(l), 0)
+
+        l = StrictOrderingOnAppendList(['a', 'b', 'c'])
+        self.assertEqual(len(l), 3)
+
+        with self.assertRaises(UnsortedError):
+            StrictOrderingOnAppendList(['c', 'b', 'a'])
+
+        self.assertEqual(len(l), 3)
+
+    def test_extend(self):
+        l = StrictOrderingOnAppendList()
+        l.extend(['a', 'b'])
+        self.assertEqual(len(l), 2)
+        self.assertIsInstance(l, StrictOrderingOnAppendList)
+
+        with self.assertRaises(UnsortedError):
+            l.extend(['d', 'c'])
+
+        self.assertEqual(len(l), 2)
+
+    def test_slicing(self):
+        l = StrictOrderingOnAppendList()
+        l[:] = ['a', 'b']
+        self.assertEqual(len(l), 2)
+        self.assertIsInstance(l, StrictOrderingOnAppendList)
+
+        with self.assertRaises(UnsortedError):
+            l[:] = ['b', 'a']
+
+        self.assertEqual(len(l), 2)
+
+    def test_add(self):
+        l = StrictOrderingOnAppendList()
+        l2 = l + ['a', 'b']
+        self.assertEqual(len(l), 0)
+        self.assertEqual(len(l2), 2)
+        self.assertIsInstance(l2, StrictOrderingOnAppendList)
+
+        with self.assertRaises(UnsortedError):
+            l2 = l + ['b', 'a']
+
+        self.assertEqual(len(l), 0)
+
+    def test_iadd(self):
+        l = StrictOrderingOnAppendList()
+        l += ['a', 'b']
+        self.assertEqual(len(l), 2)
+        self.assertIsInstance(l, StrictOrderingOnAppendList)
+
+        with self.assertRaises(UnsortedError):
+            l += ['b', 'a']
+
+        self.assertEqual(len(l), 2)
+
+
 if __name__ == '__main__':
     main()
--- a/python/mozbuild/mozbuild/util.py
+++ b/python/mozbuild/mozbuild/util.py
@@ -213,16 +213,97 @@ def resolve_target_to_make(topobjdir, ta
         # We append to target every iteration, so the check below
         # happens exactly once.
         if target != 'Makefile' and os.path.exists(make_path):
             return (reldir, target)
 
         target = os.path.join(os.path.basename(reldir), target)
         reldir = os.path.dirname(reldir)
 
+
+class UnsortedError(Exception):
+    def __init__(self, srtd, original):
+        assert len(srtd) == len(original)
+
+        self.sorted = srtd
+        self.original = original
+
+        for i, orig in enumerate(original):
+            s = srtd[i]
+
+            if orig != s:
+                self.i = i
+                break
+
+    def __str__(self):
+        s = StringIO()
+
+        s.write('An attempt was made to add an unsorted sequence to a list. ')
+        s.write('The incoming list is unsorted starting at element %d. ' %
+            self.i)
+        s.write('We expected "%s" but got "%s"' % (
+            self.sorted[self.i], self.original[self.i]))
+
+        return s.getvalue()
+
+
+class StrictOrderingOnAppendList(list):
+    """A list specialized for moz.build environments.
+
+    We overload the assignment and append operations to require that incoming
+    elements be ordered. This enforces cleaner style in moz.build files.
+    """
+    @staticmethod
+    def ensure_sorted(l):
+        srtd = sorted(l)
+
+        if srtd != l:
+            raise UnsortedError(srtd, l)
+
+    def __init__(self, iterable=[]):
+        StrictOrderingOnAppendList.ensure_sorted(iterable)
+
+        list.__init__(self, iterable)
+
+    def extend(self, l):
+        if not isinstance(l, list):
+            raise ValueError('List can only be extended with other list instances.')
+
+        StrictOrderingOnAppendList.ensure_sorted(l)
+
+        return list.extend(self, l)
+
+    def __setslice__(self, i, j, sequence):
+        if not isinstance(sequence, list):
+            raise ValueError('List can only be sliced with other list instances.')
+
+        StrictOrderingOnAppendList.ensure_sorted(sequence)
+
+        return list.__setslice__(self, i, j, sequence)
+
+    def __add__(self, other):
+        if not isinstance(other, list):
+            raise ValueError('Only lists can be appended to lists.')
+
+        StrictOrderingOnAppendList.ensure_sorted(other)
+
+        # list.__add__ will return a new list. We "cast" it to our type.
+        return StrictOrderingOnAppendList(list.__add__(self, other))
+
+    def __iadd__(self, other):
+        if not isinstance(other, list):
+            raise ValueError('Only lists can be appended to lists.')
+
+        StrictOrderingOnAppendList.ensure_sorted(other)
+
+        list.__iadd__(self, other)
+
+        return self
+
+
 class MozbuildDeletionError(Exception):
     pass
 
 class HierarchicalStringList(object):
     """A hierarchy of lists of strings.
 
     Each instance of this object contains a list of strings, which can be set or
     appended to. A sub-level of the hierarchy is also an instance of this class,
@@ -236,17 +317,17 @@ class HierarchicalStringList(object):
 
     In this case, we have 3 instances (EXPORTS, EXPORTS.mozilla, and
     EXPORTS.mozilla.dom), and the first and last each have one element in their
     list.
     """
     __slots__ = ('_strings', '_children')
 
     def __init__(self):
-        self._strings = []
+        self._strings = StrictOrderingOnAppendList()
         self._children = {}
 
     def get_children(self):
         return self._children
 
     def get_strings(self):
         return self._strings