Bug 1416465 - Expand pattern when track file is created rather than read. r=mshal
authorMasatoshi Kimura <VYV03354@nifty.ne.jp>
Sat, 02 Dec 2017 16:51:19 +0900
changeset 395372 f6b4d071186895efd54fec3c529c86ab4d451395
parent 395371 c60850f84692deb266c2b14d1d5de63b6f45656b
child 395373 e82fc38fe40de7de72e9172ee66d4646e30818e1
push id33040
push userbtara@mozilla.com
push dateThu, 07 Dec 2017 10:50:47 +0000
treeherdermozilla-central@3c2576d4ea82 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmshal
bugs1416465
milestone59.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 1416465 - Expand pattern when track file is created rather than read. r=mshal 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