Bug 1366729 - Properly handle "multi-content" manifest entries after bug 1366169. r?gps draft
authorMike Hommey <mh+mozilla@glandium.org>
Tue, 23 May 2017 07:51:22 +0900
changeset 582694 b9a36219ebfe255be65c569982b226f8fd9af219
parent 580976 aff050364eef05fde04c2efb484fdc568f981554
child 582695 795b9715ed9d2d28702b60922dc641fcd452678b
push id60143
push userbmo:mh+mozilla@glandium.org
push dateMon, 22 May 2017 23:01:28 +0000
reviewersgps
bugs1366729, 1366169
milestone55.0a1
Bug 1366729 - Properly handle "multi-content" manifest entries after bug 1366169. r?gps Some manifest entries (e.g. skin or locale) have an attached identifier, and there can be different entries with different identifiers for the same chrome name. The change from bug 1366169 would consider those as errors, while they are the expected configuration.
python/mozbuild/mozpack/packager/formats.py
python/mozbuild/mozpack/test/test_packager_formats.py
--- a/python/mozbuild/mozpack/packager/formats.py
+++ b/python/mozbuild/mozpack/packager/formats.py
@@ -6,16 +6,17 @@ from __future__ import absolute_import
 
 from mozpack.chrome.manifest import (
     Manifest,
     ManifestEntryWithRelPath,
     ManifestInterfaces,
     ManifestChrome,
     ManifestBinaryComponent,
     ManifestResource,
+    ManifestMultiContent,
 )
 from mozpack.errors import errors
 from urlparse import urlparse
 import mozpack.path as mozpath
 from mozpack.files import (
     ManifestFile,
     XPTFile,
 )
@@ -157,17 +158,21 @@ class FlatSubFormatter(object):
                 relbase = mozpath.basename(entry.base)
                 relpath = mozpath.join(relbase,
                                             mozpath.basename(path))
                 self.add_manifest(Manifest(parent, relpath))
             self.copier.add(path, ManifestFile(entry.base))
 
         if isinstance(entry, ManifestChrome):
             data = self._chrome_db.setdefault(entry.name, {})
-            entries = data.setdefault(entry.type, [])
+            if isinstance(entry, ManifestMultiContent):
+                entries = data.setdefault(entry.type, {}) \
+                              .setdefault(entry.id, [])
+            else:
+                entries = data.setdefault(entry.type, [])
             for e in entries:
                 # Ideally, we'd actually check whether entry.flags are more
                 # specific than e.flags, but in practice the following test
                 # is enough for now.
                 if not entry.flags or e.flags and entry.flags == e.flags:
                     errors.fatal('"%s" overrides "%s"' % (entry, e))
             entries.append(entry)
 
--- a/python/mozbuild/mozpack/test/test_packager_formats.py
+++ b/python/mozbuild/mozpack/test/test_packager_formats.py
@@ -14,16 +14,18 @@ from mozpack.files import (
     GeneratedFile,
     ManifestFile,
 )
 from mozpack.chrome.manifest import (
     ManifestContent,
     ManifestComponent,
     ManifestResource,
     ManifestBinaryComponent,
+    ManifestSkin,
+    ManifestLocale,
 )
 from mozpack.errors import (
     errors,
     ErrorMessage,
 )
 from mozpack.test.test_files import (
     MockDest,
     foo_xpt,
@@ -463,10 +465,39 @@ class TestFormatters(unittest.TestCase):
         self.assertEqual(e.exception.message,
             'Error: "content bar bar/unix" overrides '
             '"content bar bar/win os=WINNT"')
 
         # Adding something more specific still works.
         f.add_manifest(ManifestContent('chrome', 'bar', 'bar/win',
                                        'os=WINNT osversion>=7.0'))
 
+        # Variations of skin/locales are allowed.
+        f.add_manifest(ManifestSkin('chrome', 'foo', 'classic/1.0',
+                                    'foo/skin/classic/'))
+        f.add_manifest(ManifestSkin('chrome', 'foo', 'modern/1.0',
+                                    'foo/skin/modern/'))
+
+        f.add_manifest(ManifestLocale('chrome', 'foo', 'en-US',
+                                    'foo/locale/en-US/'))
+        f.add_manifest(ManifestLocale('chrome', 'foo', 'ja-JP',
+                                    'foo/locale/ja-JP/'))
+
+        # But same-skin/locale still error out.
+        with self.assertRaises(ErrorMessage) as e:
+            f.add_manifest(ManifestSkin('chrome', 'foo', 'classic/1.0',
+                                        'foo/skin/classic/foo'))
+
+        self.assertEqual(e.exception.message,
+            'Error: "skin foo classic/1.0 foo/skin/classic/foo" overrides '
+            '"skin foo classic/1.0 foo/skin/classic/"')
+
+        with self.assertRaises(ErrorMessage) as e:
+            f.add_manifest(ManifestLocale('chrome', 'foo', 'en-US',
+                                         'foo/locale/en-US/foo'))
+
+        self.assertEqual(e.exception.message,
+            'Error: "locale foo en-US foo/locale/en-US/foo" overrides '
+            '"locale foo en-US foo/locale/en-US/"')
+
+
 if __name__ == '__main__':
     mozunit.main()