Bug 934739 - Part 2: Add pattern matches to install manifests; r=glandium

This patch adds pattern matching entries to install manifests. We store
metadata necessary to construct a pattern match at a later point in
time. When we convert the install manifest to a file registry, we
resolve the patterns using FileFinder.

The build config logic has been updated to store support-files values as
pattern entries. This should resolve the clobber needed issue and make
the local development experience more pleasant as well.

--HG--
extra : amend_source : 3fe659f7ad6930ef54316b5babac6b83bee240af

python/mozbuild/mozbuild/backend/recursivemake.py

@@ -1051,6 +1051,14 @@ class RecursiveMakeBackend(CommonBackend):
                 if not obj.dupe_manifest:
                     raise
 
+        for base, pattern, dest in obj.pattern_installs:
+            try:
+                self._install_manifests['tests'].add_pattern_symlink(base,
+                    pattern, dest)
+            except ValueError:
+                if not obj.dupe_manifest:
+                    raise
+
         for dest in obj.external_installs:
             try:
                 self._install_manifests['tests'].add_optional_exists(dest)

python/mozbuild/mozbuild/frontend/data.py

@@ -361,6 +361,10 @@ class TestManifest(SandboxDerived):
         # path is relative from the tests root directory.
         'installs',
 
+        # A list of pattern matching installs to perform. Entries are
+        # (base, pattern, dest).
+        'pattern_installs',
+
         # Where all files for this manifest flavor are installed in the unified
         # test package directory.
         'install_prefix',
@@ -400,6 +404,7 @@ class TestManifest(SandboxDerived):
         self.manifest_relpath = relpath
         self.dupe_manifest = dupe_manifest
         self.installs = {}
+        self.pattern_installs = []
         self.tests = []
         self.external_installs = set()
 

python/mozbuild/mozbuild/frontend/emitter.py

@@ -15,8 +15,6 @@ from mach.mixin.logging import LoggingMixin
 import mozpack.path as mozpath
 import manifestparser
 
-from mozpack.files import FileFinder
-
 from .data import (
     ConfigFileSubstitution,
     Defines,
@@ -405,8 +403,6 @@ class TreeMetadataEmitter(LoggingMixin):
 
             out_dir = mozpath.join(install_prefix, manifest_reldir)
 
-            finder = FileFinder(base=manifest_dir, find_executables=False)
-
             # "head" and "tail" lists.
             # All manifests support support-files.
             #
@@ -431,22 +427,9 @@ class TreeMetadataEmitter(LoggingMixin):
                     for pattern in value.split():
                         # We only support globbing on support-files because
                         # the harness doesn't support * for head and tail.
-                        #
-                        # While we could feed everything through the finder, we
-                        # don't because we want explicitly listed files that
-                        # no longer exist to raise an error. The finder is also
-                        # slower than simple lookup.
                         if '*' in pattern and thing == 'support-files':
-                            paths = [f[0] for f in finder.find(pattern)]
-                            if not paths:
-                                raise SandboxValidationError('%s support-files '
-                                    'wildcard in %s returns no results.' % (
-                                    pattern, path))
-
-                            for f in paths:
-                                full = mozpath.normpath(mozpath.join(manifest_dir, f))
-                                obj.installs[full] = mozpath.join(out_dir, f)
-
+                            obj.pattern_installs.append(
+                                (manifest_dir, pattern, out_dir))
                         else:
                             full = mozpath.normpath(mozpath.join(manifest_dir,
                                 pattern))

python/mozbuild/mozbuild/test/backend/data/test-manifests-written/xpcshell.ini

@@ -1,3 +1,4 @@
 [DEFAULT]
+support-files = support/**
 
 [xpcshell.js]

python/mozbuild/mozbuild/test/backend/test_recursivemake.py

@@ -382,6 +382,18 @@ class TestRecursiveMakeBackend(BackendTester):
 
             self.assertEqual(len(o['xpcshell.js']), 1)
 
+    def test_test_manifest_pattern_matches_recorded(self):
+        """Pattern matches in test manifests' support-files should be recorded."""
+        env = self._consume('test-manifests-written', RecursiveMakeBackend)
+        m = InstallManifest(path=os.path.join(env.topobjdir,
+            '_build_manifests', 'install', 'tests'))
+
+        # This is not the most robust test in the world, but it gets the job
+        # done.
+        entries = [e for e in m._dests.keys() if '**' in e]
+        self.assertEqual(len(entries), 1)
+        self.assertIn('support/**', entries[0])
+
     def test_xpidl_generation(self):
         """Ensure xpidl files and directories are written out."""
         env = self._consume('xpidl', RecursiveMakeBackend)

python/mozbuild/mozbuild/test/frontend/test_emitter.py

@@ -248,10 +248,8 @@ class TestEmitterBasic(unittest.TestCase):
                 'installs': {
                     'a11y.ini',
                     'test_a11y.js',
-                    # From ** wildcard.
-                    'a11y-support/foo',
-                    'a11y-support/dir1/bar',
                 },
+                'pattern-installs': 1,
             },
             'browser.ini': {
                 'flavor': 'browser-chrome',
@@ -319,6 +317,9 @@ class TestEmitterBasic(unittest.TestCase):
 
                 self.assertIn(path, m['installs'])
 
+            if 'pattern-installs' in m:
+                self.assertEqual(len(o.pattern_installs), m['pattern-installs'])
+
     def test_test_manifest_unmatched_generated(self):
         reader = self.reader('test-manifest-unmatched-generated')
 

python/mozbuild/mozpack/files.py

@@ -556,7 +556,10 @@ class FileFinder(BaseFinder):
         Ignores file names starting with a '.' under the given path. If the
         path itself has leafs starting with a '.', they are not ignored.
         '''
-        for p in os.listdir(os.path.join(self.base, path)):
+        # The sorted makes the output idempotent. Otherwise, we are
+        # likely dependent on filesystem implementation details, such as
+        # inode ordering.
+        for p in sorted(os.listdir(os.path.join(self.base, path))):
             if p.startswith('.'):
                 continue
             for p_, f in self._find(mozpack.path.join(path, p)):
@@ -596,7 +599,8 @@ class FileFinder(BaseFinder):
         elif '*' in pattern[0]:
             if not os.path.exists(os.path.join(self.base, base)):
                 return
-            for p in os.listdir(os.path.join(self.base, base)):
+            # See above comment w.r.t. sorted() and idempotent behavior.
+            for p in sorted(os.listdir(os.path.join(self.base, base))):
                 if p.startswith('.') and not pattern[0].startswith('.'):
                     continue
                 if mozpack.path.match(p, pattern[0]):

python/mozbuild/mozpack/manifests.py

@@ -6,11 +6,11 @@ from __future__ import unicode_literals
 
 from contextlib import contextmanager
 
-from .copier import FilePurger
 from .files import (
     AbsoluteSymlinkFile,
     ExistingFile,
     File,
+    FileFinder,
 )
 import mozpack.path as mozpath
 
@@ -63,8 +63,15 @@ class InstallManifest(object):
           the FileCopier. No error is raised if the destination path does not
           exist.
 
-    Versions 1 and 2 of the manifest format are similar. Version 2 added
-    optional path support.
+      patternsymlink -- Paths matched by the expression in the source path
+          will be symlinked to the destination directory.
+
+      patterncopy -- Similar to patternsymlink except files are copied, not
+          symlinked.
+
+    Version 1 of the manifest was the initial version.
+    Version 2 added optional path support
+    Version 3 added support for pattern entries.
     """
     FIELD_SEPARATOR = '\x1f'
 
@@ -72,6 +79,8 @@ class InstallManifest(object):
     COPY = 2
     REQUIRED_EXISTS = 3
     OPTIONAL_EXISTS = 4
+    PATTERN_SYMLINK = 5
+    PATTERN_COPY = 6
 
     def __init__(self, path=None, fileobj=None):
         """Create a new InstallManifest entry.
@@ -94,7 +103,7 @@ class InstallManifest(object):
 
     def _load_from_fileobj(self, fileobj):
         version = fileobj.readline().rstrip()
-        if version not in ('1', '2'):
+        if version not in ('1', '2', '3'):
             raise UnreadableInstallManifest('Unknown manifest version: ' %
                 version)
 
@@ -106,7 +115,7 @@ class InstallManifest(object):
             record_type = int(fields[0])
 
             if record_type == self.SYMLINK:
-                dest, source= fields[1:]
+                dest, source = fields[1:]
                 self.add_symlink(source, dest)
                 continue
 
@@ -125,6 +134,16 @@ class InstallManifest(object):
                 self.add_optional_exists(path)
                 continue
 
+            if record_type == self.PATTERN_SYMLINK:
+                _, base, pattern, dest = fields[1:]
+                self.add_pattern_symlink(base, pattern, dest)
+                continue
+
+            if record_type == self.PATTERN_COPY:
+                _, base, pattern, dest = fields[1:]
+                self.add_pattern_copy(base, pattern, dest)
+                continue
+
             raise UnreadableInstallManifest('Unknown record type: %d' %
                 record_type)
 
@@ -158,7 +177,7 @@ class InstallManifest(object):
         It is an error if both are specified.
         """
         with _auto_fileobj(path, fileobj, 'wb') as fh:
-            fh.write('2\n')
+            fh.write('3\n')
 
             for dest in sorted(self._dests):
                 entry = self._dests[dest]
@@ -198,6 +217,29 @@ class InstallManifest(object):
         """
         self._add_entry(dest, (self.OPTIONAL_EXISTS,))
 
+    def add_pattern_symlink(self, base, pattern, dest):
+        """Add a pattern match that results in symlinks being created.
+
+        A ``FileFinder`` will be created with its base set to ``base``
+        and ``FileFinder.find()`` will be called with ``pattern`` to discover
+        source files. Each source file will be symlinked 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.PATTERN_SYMLINK, base, pattern, dest))
+
+    def add_pattern_copy(self, base, pattern, dest):
+        """Add a pattern match that results in copies.
+
+        See ``add_pattern_symlink()`` for usage.
+        """
+        self._add_entry(mozpath.join(base, pattern, dest),
+            (self.PATTERN_COPY, base, pattern, dest))
+
     def _add_entry(self, dest, entry):
         if dest in self._dests:
             raise ValueError('Item already in manifest: %s' % dest)
@@ -231,5 +273,21 @@ class InstallManifest(object):
                 registry.add(dest, ExistingFile(required=False))
                 continue
 
+            if install_type in (self.PATTERN_SYMLINK, self.PATTERN_COPY):
+                _, base, pattern, dest = entry
+                finder = FileFinder(base, find_executables=False)
+                paths = [f[0] for f in finder.find(pattern)]
+
+                if install_type == self.PATTERN_SYMLINK:
+                    cls = AbsoluteSymlinkFile
+                else:
+                    cls = File
+
+                for path in paths:
+                    source = mozpath.join(base, path)
+                    registry.add(mozpath.join(dest, path), cls(source))
+
+                continue
+
             raise Exception('Unknown install type defined in manifest: %d' %
                 install_type)

python/mozbuild/mozpack/test/test_manifests.py

@@ -29,8 +29,10 @@ class TestInstallManifest(TestWithTmpDir):
         m.add_copy('c_source', 'c_dest')
         m.add_required_exists('e_dest')
         m.add_optional_exists('o_dest')
+        m.add_pattern_symlink('ps_base', 'ps/*', 'ps_dest')
+        m.add_pattern_copy('pc_base', 'pc/**', 'pc_dest')
 
-        self.assertEqual(len(m), 4)
+        self.assertEqual(len(m), 6)
         self.assertIn('s_dest', m)
         self.assertIn('c_dest', m)
         self.assertIn('e_dest', m)
@@ -48,12 +50,20 @@ class TestInstallManifest(TestWithTmpDir):
         with self.assertRaises(ValueError):
             m.add_optional_exists('o_dest')
 
+        with self.assertRaises(ValueError):
+            m.add_pattern_symlink('ps_base', 'ps/*', 'ps_dest')
+
+        with self.assertRaises(ValueError):
+            m.add_pattern_copy('pc_base', 'pc/**', 'pc_dest')
+
     def _get_test_manifest(self):
         m = InstallManifest()
         m.add_symlink(self.tmppath('s_source'), 's_dest')
         m.add_copy(self.tmppath('c_source'), 'c_dest')
         m.add_required_exists('e_dest')
         m.add_optional_exists('o_dest')
+        m.add_pattern_symlink('ps_base', '*', 'ps_dest')
+        m.add_pattern_copy('pc_base', '**', 'pc_dest')
 
         return m
 
@@ -67,18 +77,12 @@ class TestInstallManifest(TestWithTmpDir):
         with open(p, 'rb') as fh:
             c = fh.read()
 
-        self.assertEqual(c.count('\n'), 5)
+        self.assertEqual(c.count('\n'), 7)
 
         lines = c.splitlines()
-        self.assertEqual(len(lines), 5)
+        self.assertEqual(len(lines), 7)
 
-        self.assertEqual(lines[0], '2')
-        self.assertEqual(lines[1], '2\x1fc_dest\x1f%s' %
-            self.tmppath('c_source'))
-        self.assertEqual(lines[2], '3\x1fe_dest')
-        self.assertEqual(lines[3], '4\x1fo_dest')
-        self.assertEqual(lines[4], '1\x1fs_dest\x1f%s' %
-            self.tmppath('s_source'))
+        self.assertEqual(lines[0], '3')
 
         m2 = InstallManifest(path=p)
         self.assertEqual(m, m2)
@@ -98,8 +102,28 @@ class TestInstallManifest(TestWithTmpDir):
         self.assertEqual(len(r), 4)
         self.assertEqual(r.paths(), ['c_dest', 'e_dest', 'o_dest', 's_dest'])
 
+    def test_pattern_expansion(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_symlink('%s/base' % source, '**', 'dest')
+
+        c = FileCopier()
+        m.populate_registry(c)
+        self.assertEqual(c.paths(), ['dest/foo/file1', 'dest/foo/file2'])
+
     def test_or(self):
         m1 = self._get_test_manifest()
+        orig_length = len(m1)
         m2 = InstallManifest()
         m2.add_symlink('s_source2', 's_dest2')
         m2.add_copy('c_source2', 'c_dest2')
@@ -107,7 +131,7 @@ class TestInstallManifest(TestWithTmpDir):
         m1 |= m2
 
         self.assertEqual(len(m2), 2)
-        self.assertEqual(len(m1), 6)
+        self.assertEqual(len(m1), orig_length + 2)
 
         self.assertIn('s_dest2', m1)
         self.assertIn('c_dest2', m1)