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

--HG--
extra : rebase_source : 1c5ea3b0f1fc1a7717843d0fd2d8191e924d724b

python/mozbuild/mozbuild/frontend/context.py

@@ -804,7 +804,7 @@ class Schedules(object):
         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
@@ -815,9 +815,9 @@ class Schedules(object):
             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))
+            raise Exception("unexpected inclusive component(s) " + ', '.join(unexpected))
 
-    # exclusive is immuntable but can be set (= only)
+    # exclusive is immutable but can be set (= only)
     @property
     def exclusive(self):
         return self._exclusive
@@ -836,6 +836,25 @@ class Schedules(object):
     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
@@ -1086,6 +1105,10 @@ class Files(SubContext):
                 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

python/mozbuild/mozbuild/test/frontend/data/schedules/moz.build

@@ -7,5 +7,13 @@ with Files('*.win'):
 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']

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']

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

@@ -483,7 +483,14 @@ class TestBuildReader(unittest.TestCase):
 
     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)
@@ -496,12 +503,24 @@ class TestBuildReader(unittest.TestCase):
         # 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']))
+        self.fail()
+
+    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()