Bug 1429815 - Fix InstallManifest::add_entries_from with non-trivial base. r=gps
authorNick Alexander <nalexander@mozilla.com>
Thu, 18 Jan 2018 16:58:07 -0800
changeset 749078 31a0af4e29bbf601613a99d85ac4426d0a3251e8
parent 749077 ec90dfc1563680727d1b377e823db48a7ff5aa1f
child 749079 6639a46610c1f408fc5abebb91f490222dcdca1f
push id97316
push userbmo:continuation@gmail.com
push dateTue, 30 Jan 2018 22:53:01 +0000
reviewersgps
bugs1429815
milestone60.0a1
Bug 1429815 - Fix InstallManifest::add_entries_from with non-trivial base. r=gps There's a bug in InstallManifest::add_entries_from: some of the manifest entries bake their destination into both the manifest key and the manifest value, and add_entries_from with a non-empty "base" parameter to prepend to the destination only updates the manifest key and not the value. This bug causes |mach watch| to fail to _read_ the unified manifest that aggregates all of the build manifests relevant to |mach watch| that |mach build-backend -b FasterMake| successfully _wrote_, because the manifest keys are validated against the manifest values written to disk. I see no way to address this other than to manually reach into the manifest values and patch the internal destination parameter, which this patch does. MozReview-Commit-ID: FVyRB41NnHa
python/mozbuild/mozpack/manifests.py
python/mozbuild/mozpack/test/test_manifests.py
--- a/python/mozbuild/mozpack/manifests.py
+++ b/python/mozbuild/mozpack/manifests.py
@@ -346,17 +346,25 @@ class InstallManifest(object):
         # which source file each entry came from. However, this is more
         # complicated and not yet implemented. The current implementation
         # will result in over invalidation, possibly leading to performance
         # loss.
         self._source_files |= other._source_files
 
         for dest in sorted(other._dests):
             new_dest = mozpath.join(base, dest) if base else dest
-            self._add_entry(new_dest, other._dests[dest])
+            entry = other._dests[dest]
+            if entry[0] in (self.PATTERN_LINK, self.PATTERN_COPY):
+                entry_type, entry_base, entry_pattern, entry_dest = entry
+                new_entry_dest = mozpath.join(base, entry_dest) if base else entry_dest
+                new_entry = (entry_type, entry_base, entry_pattern, new_entry_dest)
+            else:
+                new_entry = tuple(entry)
+
+            self._add_entry(new_dest, new_entry)
 
     def populate_registry(self, registry, defines_override={},
                           link_policy='symlink'):
         """Populate a mozpack.copier.FileRegistry instance with data from us.
 
         The caller supplied a FileRegistry instance (or at least something that
         conforms to its interface) and that instance is populated with data
         from this manifest.
--- a/python/mozbuild/mozpack/test/test_manifests.py
+++ b/python/mozbuild/mozpack/test/test_manifests.py
@@ -388,10 +388,61 @@ class TestInstallManifest(TestWithTmpDir
         # ORing an InstallManifest should copy file dependencies
         m = InstallManifest()
         m |= InstallManifest(path=manifest)
         c = FileCopier()
         m.populate_registry(c)
         e = c._files['p_dest']
         self.assertEqual(e.extra_depends, [manifest])
 
+    def test_add_entries_from(self):
+        source = self.tmppath('source')
+        os.mkdir(source)
+        os.mkdir('%s/base' % source)
+        os.mkdir('%s/base/foo' % source)
+
+        with open('%s/base/foo/file1' % source, 'a'):
+            pass
+
+        with open('%s/base/foo/file2' % source, 'a'):
+            pass
+
+        m = InstallManifest()
+        m.add_pattern_link('%s/base' % source, '**', 'dest')
+
+        p = InstallManifest()
+        p.add_entries_from(m)
+        self.assertEqual(len(p), 1)
+
+        c = FileCopier()
+        p.populate_registry(c)
+        self.assertEqual(c.paths(), ['dest/foo/file1', 'dest/foo/file2'])
+
+        q = InstallManifest()
+        q.add_entries_from(m, base='target')
+        self.assertEqual(len(q), 1)
+
+        d = FileCopier()
+        q.populate_registry(d)
+        self.assertEqual(d.paths(), ['target/dest/foo/file1', 'target/dest/foo/file2'])
+
+        # Some of the values in an InstallManifest include destination
+        # information that is present in the keys.  Verify that we can
+        # round-trip serialization.
+        r = InstallManifest()
+        r.add_entries_from(m)
+        r.add_entries_from(m, base='target')
+        self.assertEqual(len(r), 2)
+
+        temp_path = self.tmppath('temp_path')
+        r.write(path=temp_path)
+
+        s = InstallManifest(path=temp_path)
+        e = FileCopier()
+        s.populate_registry(e)
+
+        self.assertEqual(e.paths(),
+                         ['dest/foo/file1', 'dest/foo/file2',
+                          'target/dest/foo/file1', 'target/dest/foo/file2'])
+
+
 if __name__ == '__main__':
     mozunit.main()