Bug 1416465 - Expand pattern when track file is created rather than read. r?Build draft
authorMasatoshi Kimura <VYV03354@nifty.ne.jp>
Sat, 02 Dec 2017 16:51:19 +0900
changeset 708620 ef510a32098f1e7b0e7f105df7ebd1da8d1ecde3
parent 708619 67c9e9708303b3da9e7880f70cc04871366c1414
child 743208 98a5f84177c6d535cc4b9977210514e04382370f
push id92401
push userVYV03354@nifty.ne.jp
push dateWed, 06 Dec 2017 22:08:43 +0000
reviewersBuild
bugs1416465
milestone59.0a1
Bug 1416465 - Expand pattern when track file is created rather than read. r?Build MozReview-Commit-ID: WISu4wThdw
CLOBBER
python/mozbuild/mozbuild/action/process_install_manifest.py
python/mozbuild/mozbuild/test/action/test_process_install_manifest.py
python/mozbuild/mozpack/manifests.py
python/mozbuild/mozpack/test/test_manifests.py
--- a/CLOBBER
+++ b/CLOBBER
@@ -17,9 +17,9 @@
 #
 # Modifying this file will now automatically clobber the buildbot machines \o/
 #
 
 # Are you updating CLOBBER because you think it's needed for your WebIDL
 # changes to stick? As of bug 928195, this shouldn't be necessary! Please
 # don't change CLOBBER for WebIDL changes any more.
 
-Bug 1419362 - a11y IDL changes not correctly handled by build system
+Bug 1416465 - Old track files may cause problems if they have wildcards.
--- a/python/mozbuild/mozbuild/action/process_install_manifest.py
+++ b/python/mozbuild/mozbuild/action/process_install_manifest.py
@@ -64,17 +64,19 @@ def process_manifest(destdir, paths, tra
         copier, defines_override=defines, link_policy=link_policy
     )
     result = copier.copy(destdir,
         remove_unaccounted=remove_unaccounted,
         remove_all_directory_symlinks=remove_all_directory_symlinks,
         remove_empty_directories=remove_empty_directories)
 
     if track:
-        manifest.write(path=track)
+        # We should record files that we actually copied.
+        # It is too late to expand wildcards when the track file is read.
+        manifest.write(path=track, expand_pattern=True)
 
     return result
 
 
 def main(argv):
     parser = argparse.ArgumentParser(
         description='Process install manifest files.')
 
--- a/python/mozbuild/mozbuild/test/action/test_process_install_manifest.py
+++ b/python/mozbuild/mozbuild/test/action/test_process_install_manifest.py
@@ -55,26 +55,19 @@ class TestGenerateManifest(TestWithTmpDi
 
         for i in range(2):
             process_install_manifest.process_manifest(dest, [p], track)
 
             self.assertTrue(os.path.exists(self.tmppath('dest/foo/file1')))
             self.assertTrue(os.path.exists(self.tmppath('dest/foo/file2')))
             self.assertTrue(os.path.exists(self.tmppath('dest/foo/file3')))
 
-    @expectedFailure
-    def test_process_manifest2(self):
-        self.test_process_manifest()
         m = InstallManifest()
-        p = self.tmppath('m')
         m.write(path=p)
 
-        dest = self.tmppath('dest')
-        track = self.tmppath('track')
-
         for i in range(2):
             process_install_manifest.process_manifest(dest, [p], track)
 
             self.assertFalse(os.path.exists(self.tmppath('dest/foo/file1')))
             self.assertFalse(os.path.exists(self.tmppath('dest/foo/file2')))
             self.assertFalse(os.path.exists(self.tmppath('dest/foo/file3')))
 
 if __name__ == '__main__':
--- a/python/mozbuild/mozpack/manifests.py
+++ b/python/mozbuild/mozpack/manifests.py
@@ -215,34 +215,45 @@ class InstallManifest(object):
     def _decode_field_entry(self, data):
         """Restores an object from a format that can be stored in the manifest file.
 
         Complex data types, such as ``dict``, need to be converted into a text
         representation before they can be written to a file.
         """
         return json.loads(data)
 
-    def write(self, path=None, fileobj=None):
+    def write(self, path=None, fileobj=None, expand_pattern=False):
         """Serialize this manifest to a file or file object.
 
         If path is specified, that file will be written to. If fileobj is specified,
         the serialized content will be written to that file object.
 
         It is an error if both are specified.
         """
         with _auto_fileobj(path, fileobj, 'wb') as fh:
             fh.write('%d\n' % self.CURRENT_VERSION)
 
             for dest in sorted(self._dests):
                 entry = self._dests[dest]
 
-                parts = ['%d' % entry[0], dest]
-                parts.extend(entry[1:])
-                fh.write('%s\n' % self.FIELD_SEPARATOR.join(
-                    p.encode('utf-8') for p in parts))
+                if expand_pattern and entry[0] in (self.PATTERN_LINK, self.PATTERN_COPY):
+                    type, base, pattern, dest = entry
+                    type = self.LINK if type == self.PATTERN_LINK else self.COPY
+                    finder = FileFinder(base)
+                    paths = [f[0] for f in finder.find(pattern)]
+                    for path in paths:
+                        source = mozpath.join(base, path)
+                        parts = ['%d' % type, mozpath.join(dest, path), source]
+                        fh.write('%s\n' % self.FIELD_SEPARATOR.join(
+                            p.encode('utf-8') for p in parts))
+                else:
+                    parts = ['%d' % entry[0], dest]
+                    parts.extend(entry[1:])
+                    fh.write('%s\n' % self.FIELD_SEPARATOR.join(
+                        p.encode('utf-8') for p in parts))
 
     def add_link(self, source, dest):
         """Add a link to this manifest.
 
         dest will be either a symlink or hardlink to source.
         """
         self._add_entry(dest, (self.LINK, source))
 
@@ -277,25 +288,25 @@ class InstallManifest(object):
         source files. Each source file will be either symlinked or hardlinked
         under ``dest``.
 
         Filenames under ``dest`` are constructed by taking the path fragment
         after ``base`` and concatenating it with ``dest``. e.g.
 
            <base>/foo/bar.h -> <dest>/foo/bar.h
         """
-        self._add_entry(mozpath.join(base, pattern, dest),
+        self._add_entry(mozpath.join(dest, pattern),
             (self.PATTERN_LINK, base, pattern, dest))
 
     def add_pattern_copy(self, base, pattern, dest):
         """Add a pattern match that results in copies.
 
         See ``add_pattern_link()`` for usage.
         """
-        self._add_entry(mozpath.join(base, pattern, dest),
+        self._add_entry(mozpath.join(dest, pattern),
             (self.PATTERN_COPY, base, pattern, dest))
 
     def add_preprocess(self, source, dest, deps, marker='#', defines={},
                        silence_missing_directive_warnings=False):
         """Add a preprocessed file to this manifest.
 
         ``source`` will be passed through preprocessor.py, and the output will be
         written to ``dest``.
--- a/python/mozbuild/mozpack/test/test_manifests.py
+++ b/python/mozbuild/mozpack/test/test_manifests.py
@@ -136,16 +136,38 @@ class TestInstallManifest(TestWithTmpDir
 
         m = InstallManifest()
         m.add_pattern_link('%s/base' % source, '**', 'dest')
 
         c = FileCopier()
         m.populate_registry(c)
         self.assertEqual(c.paths(), ['dest/foo/file1', 'dest/foo/file2'])
 
+    def test_write_expand_pattern(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')
+
+        track = self.tmppath('track')
+        m.write(path=track, expand_pattern=True)
+
+        m = InstallManifest(path=track)
+        self.assertEqual([dest for dest in m._dests],
+                         ['dest/foo/file1', 'dest/foo/file2'])
+
     def test_or(self):
         m1 = self._get_test_manifest()
         orig_length = len(m1)
         m2 = InstallManifest()
         m2.add_link('s_source2', 's_dest2')
         m2.add_copy('c_source2', 'c_dest2')
 
         m1 |= m2