Bug 1366729 - Properly handle "multi-content" manifest entries after bug 1366169. r=gps
authorMike Hommey <mh+mozilla@glandium.org>
Tue, 23 May 2017 07:51:22 +0900
changeset 360181 aec720895e5e79d126283c118cb4d26744c3750a
parent 360180 d11aec13b38e0837c07d0906c1ab506e162b2a2f
child 360182 249495cc2ea52fb0fa0f8f4db6894070789953bd
push id31871
push userryanvm@gmail.com
push dateTue, 23 May 2017 22:02:07 +0000
treeherdermozilla-central@545ffce30eac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps
bugs1366729, 1366169
milestone55.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 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,
 )
@@ -152,17 +153,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,
@@ -461,10 +463,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()