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 708687 0d059559f650937ef69538ef3581667d167686f9
parent 708686 2ffd2727c6324296e8d8e99be71a00dbeb264e3a
child 708688 4210313d54e18eadad52d98f78343b322a222d47
push id92403
push userVYV03354@nifty.ne.jp
push dateWed, 06 Dec 2017 22:18:41 +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