Bug 1426255: combine Files:SCHEDULES correctly; r=gps
authorDustin J. Mitchell <dustin@mozilla.com>
Tue, 19 Dec 2017 23:58:17 +0000
changeset 452571 67ab725f9d508ce8a27f2d15520a41b2704dc58e
parent 452570 3416e5bed288f3754a921b06e281959670d52466
child 452572 bd11f296e5ed66413b24d091dd78bbf49a352b29
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps
bugs1426255
milestone59.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 1426255: combine Files:SCHEDULES correctly; r=gps When multiple SCHEDULES are set for the same file (for example in different files), combine them in a sensible way: the union of inclusive components, and whichever has set its exclusive components. Two conflicting assignments to SCHEDULES.exclusive is considered an error. We might relax this situation later if a sensible answer can be determined. Note that this error will only be detected when a reader consults the relevant file. MozReview-Commit-ID: A49L9ISZXOE
python/mozbuild/mozbuild/frontend/context.py
python/mozbuild/mozbuild/test/frontend/data/schedules/moz.build
python/mozbuild/mozbuild/test/frontend/data/schedules/subd/moz.build
python/mozbuild/mozbuild/test/frontend/test_reader.py
--- a/python/mozbuild/mozbuild/frontend/context.py
+++ b/python/mozbuild/mozbuild/frontend/context.py
@@ -799,30 +799,30 @@ class Schedules(object):
        values from mozbuild.schedules.ALL_COMPONENTS
     """
     __slots__ = ('_exclusive', '_inclusive')
 
     def __init__(self):
         self._inclusive = TypedList(Enum(*schedules.INCLUSIVE_COMPONENTS))()
         self._exclusive = ImmutableStrictOrderingOnAppendList(schedules.EXCLUSIVE_COMPONENTS)
 
-    # inclusive is mutable cannot be assigned to (+= only)
+    # inclusive is mutable but cannot be assigned to (+= only)
     @property
     def inclusive(self):
         return self._inclusive
 
     @inclusive.setter
     def inclusive(self, value):
         if value is not self._inclusive:
             raise AttributeError("Cannot assign to this value - use += instead")
         unexpected = [v for v in value if v not in schedules.INCLUSIVE_COMPONENTS]
         if unexpected:
-            raise Exception("unexpected exclusive component(s) " + ', '.join(unexpected))
-
-    # exclusive is immuntable but can be set (= only)
+            raise Exception("unexpected inclusive component(s) " + ', '.join(unexpected))
+
+    # exclusive is immutable but can be set (= only)
     @property
     def exclusive(self):
         return self._exclusive
 
     @exclusive.setter
     def exclusive(self, value):
         if not isinstance(value, (tuple, list)):
             raise Exception("expected a tuple or list")
@@ -831,16 +831,35 @@ class Schedules(object):
             raise Exception("unexpected exclusive component(s) " + ', '.join(unexpected))
         self._exclusive = ImmutableStrictOrderingOnAppendList(sorted(value))
 
     # components provides a synthetic summary of all components
     @property
     def components(self):
         return list(sorted(set(self._inclusive) | set(self._exclusive)))
 
+    # The `Files` context uses | to combine SCHEDULES from multiple levels; at this
+    # point the immutability is no longer needed so we use plain lists
+    def __or__(self, other):
+        rv = Schedules()
+        rv._inclusive = self._inclusive + other._inclusive
+        if other._exclusive == self._exclusive:
+            rv._exclusive = self._exclusive
+        elif self._exclusive == schedules.EXCLUSIVE_COMPONENTS:
+            rv._exclusive = other._exclusive
+        elif other._exclusive == schedules.EXCLUSIVE_COMPONENTS:
+            rv._exclusive = self._exclusive
+        else:
+            msg = 'Two Files sections have set SCHEDULES.exclusive to different' \
+                'values; these cannot be combined: {} and {}'
+            msg = msg.format(self._exclusive, other._exclusive)
+            raise ValueError(msg)
+        return rv
+
+
 @memoize
 def ContextDerivedTypedHierarchicalStringList(type):
     """Specialized HierarchicalStringList for use with ContextDerivedValue
     types."""
     class _TypedListWithItems(ContextDerivedValue, HierarchicalStringList):
         __slots__ = ('_strings', '_children', '_context')
 
         def __init__(self, context):
@@ -1081,16 +1100,20 @@ class Files(SubContext):
         for k, v in other.items():
             if k == 'IMPACTED_TESTS':
                 self.test_files |= set(mozpath.relpath(e.full_path, e.context.config.topsrcdir)
                                        for e in v.files)
                 self.test_tags |= set(v.tags)
                 self.test_flavors |= set(v.flavors)
                 continue
 
+            if k == 'SCHEDULES' and 'SCHEDULES' in self:
+                self['SCHEDULES'] = self['SCHEDULES'] | v
+                continue
+
             # Ignore updates to finalized flags.
             if k in self.finalized:
                 continue
 
             # Only finalize variables defined in this instance.
             if k == 'FINAL':
                 self.finalized |= set(other) - {'FINAL'}
                 continue
--- a/python/mozbuild/mozbuild/test/frontend/data/schedules/moz.build
+++ b/python/mozbuild/mozbuild/test/frontend/data/schedules/moz.build
@@ -2,10 +2,18 @@
 # http://creativecommons.org/publicdomain/zero/1.0/
 
 with Files('*.win'):
     SCHEDULES.exclusive = ['windows']
 
 with Files('*.osx'):
     SCHEDULES.exclusive = ['macosx']
 
+with Files('bad.osx'):
+    # this conflicts with the previous clause and will cause an error
+    # when read
+    SCHEDULES.exclusive = ['macosx', 'windows']
+
 with Files('subd/**.py'):
     SCHEDULES.inclusive += ['py-lint']
+
+with Files('**/*.js'):
+    SCHEDULES.inclusive += ['js-lint']
--- a/python/mozbuild/mozbuild/test/frontend/data/schedules/subd/moz.build
+++ b/python/mozbuild/mozbuild/test/frontend/data/schedules/subd/moz.build
@@ -1,2 +1,5 @@
 with Files('yaml.py'):
     SCHEDULES.inclusive += ['yaml-lint']
+
+with Files('win.js'):
+    SCHEDULES.exclusive = ['windows']
--- a/python/mozbuild/mozbuild/test/frontend/test_reader.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_reader.py
@@ -478,30 +478,48 @@ class TestBuildReader(unittest.TestCase)
     def test_invalid_flavor(self):
         reader = self.reader('invalid-files-flavor')
 
         with self.assertRaises(BuildReaderError):
             reader.files_info(['foo.js'])
 
     def test_schedules(self):
         reader = self.reader('schedules')
-        info = reader.files_info(['somefile', 'foo.win', 'foo.osx', 'subd/aa.py', 'subd/yaml.py'])
+        info = reader.files_info([
+            'somefile',
+            'foo.win',
+            'foo.osx',
+            'subd/aa.py',
+            'subd/yaml.py',
+            'subd/win.js',
+        ])
         # default: all exclusive, no inclusive
         self.assertEqual(info['somefile']['SCHEDULES'].inclusive, [])
         self.assertEqual(info['somefile']['SCHEDULES'].exclusive, schedules.EXCLUSIVE_COMPONENTS)
         # windows-only
         self.assertEqual(info['foo.win']['SCHEDULES'].inclusive, [])
         self.assertEqual(info['foo.win']['SCHEDULES'].exclusive, ['windows'])
         # osx-only
         self.assertEqual(info['foo.osx']['SCHEDULES'].inclusive, [])
         self.assertEqual(info['foo.osx']['SCHEDULES'].exclusive, ['macosx'])
         # top-level moz.build specifies subd/**.py with an inclusive option
         self.assertEqual(info['subd/aa.py']['SCHEDULES'].inclusive, ['py-lint'])
         self.assertEqual(info['subd/aa.py']['SCHEDULES'].exclusive, schedules.EXCLUSIVE_COMPONENTS)
-        # Files('yaml.py') in subd/moz.build *overrides* Files('subdir/**.py')
-        self.assertEqual(info['subd/yaml.py']['SCHEDULES'].inclusive, ['yaml-lint'])
+        # Files('yaml.py') in subd/moz.build combines with Files('subdir/**.py')
+        self.assertEqual(info['subd/yaml.py']['SCHEDULES'].inclusive, ['py-lint', 'yaml-lint'])
         self.assertEqual(info['subd/yaml.py']['SCHEDULES'].exclusive, schedules.EXCLUSIVE_COMPONENTS)
+        # .. but exlusive does not override inclusive
+        self.assertEqual(info['subd/win.js']['SCHEDULES'].inclusive, ['js-lint'])
+        self.assertEqual(info['subd/win.js']['SCHEDULES'].exclusive, ['windows'])
 
         self.assertEqual(set(info['subd/yaml.py']['SCHEDULES'].components),
-                         set(schedules.EXCLUSIVE_COMPONENTS + ['yaml-lint']))
+                         set(schedules.EXCLUSIVE_COMPONENTS + ['py-lint', 'yaml-lint']))
+
+    def test_schedules_conflicting_excludes(self):
+        reader = self.reader('schedules')
+
+        # bad.osx is defined explicitly, and matches *.osx, and the two have
+        # conflicting SCHEDULES.exclusive settings
+        with self.assertRaisesRegexp(ValueError, r"Two Files sections"):
+            reader.files_info(['bad.osx'])
 
 if __name__ == '__main__':
     main()