Bug 934739 - Part 2: Add pattern matches to install manifests; r=glandium
authorGregory Szorc <gps@mozilla.com>
Mon, 09 Dec 2013 19:02:35 +0900
changeset 159619 7300b41e4b1f44b18ced541cb078870047ceab18
parent 159618 982d8e522f1887a017bdee0c711bdd77aff805f6
child 159620 dd46a766c14430e1009ec33f809d02a43e30ff01
push id25808
push usercbook@mozilla.com
push dateTue, 10 Dec 2013 12:03:31 +0000
treeherdermozilla-central@7fb91a422c5e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersglandium
bugs934739
milestone29.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 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.
python/mozbuild/mozbuild/backend/recursivemake.py
python/mozbuild/mozbuild/frontend/data.py
python/mozbuild/mozbuild/frontend/emitter.py
python/mozbuild/mozbuild/test/backend/data/test-manifests-written/xpcshell.ini
python/mozbuild/mozbuild/test/backend/test_recursivemake.py
python/mozbuild/mozbuild/test/frontend/test_emitter.py
python/mozbuild/mozpack/files.py
python/mozbuild/mozpack/manifests.py
python/mozbuild/mozpack/test/test_manifests.py
--- a/python/mozbuild/mozbuild/backend/recursivemake.py
+++ b/python/mozbuild/mozbuild/backend/recursivemake.py
@@ -1046,16 +1046,24 @@ class RecursiveMakeBackend(CommonBackend
         # Duplicate manifests may define the same file. That's OK.
         for source, dest in obj.installs.items():
             try:
                 self._install_manifests['tests'].add_symlink(source, dest)
             except ValueError:
                 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)
             except ValueError:
                 if not obj.dupe_manifest:
                     raise
 
         m = self._test_manifests.setdefault(obj.flavor,
--- a/python/mozbuild/mozbuild/frontend/data.py
+++ b/python/mozbuild/mozbuild/frontend/data.py
@@ -356,16 +356,20 @@ class TestManifest(SandboxDerived):
     __slots__ = (
         # The type of test manifest this is.
         'flavor',
 
         # Maps source filename to destination filename. The destination
         # 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',
 
         # Set of files provided by an external mechanism.
         'external_installs',
 
         # The full path of this manifest file.
@@ -395,16 +399,17 @@ class TestManifest(SandboxDerived):
         self.path = path
         self.directory = os.path.dirname(path)
         self.manifest = manifest
         self.flavor = flavor
         self.install_prefix = install_prefix
         self.manifest_relpath = relpath
         self.dupe_manifest = dupe_manifest
         self.installs = {}
+        self.pattern_installs = []
         self.tests = []
         self.external_installs = set()
 
 
 class LocalInclude(SandboxDerived):
     """Describes an individual local include path."""
 
     __slots__ = (
--- a/python/mozbuild/mozbuild/frontend/emitter.py
+++ b/python/mozbuild/mozbuild/frontend/emitter.py
@@ -10,18 +10,16 @@ import os
 import traceback
 import sys
 
 from mach.mixin.logging import LoggingMixin
 
 import mozpack.path as mozpath
 import manifestparser
 
-from mozpack.files import FileFinder
-
 from .data import (
     ConfigFileSubstitution,
     Defines,
     DirectoryTraversal,
     Exports,
     GeneratedEventWebIDLFile,
     GeneratedInclude,
     GeneratedWebIDLFile,
@@ -400,18 +398,16 @@ class TreeMetadataEmitter(LoggingMixin):
 
             filtered = m.tests
 
             if filter_inactive:
                 filtered = m.active_tests(disabled=False, **self.mozinfo)
 
             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.
             #
             # Keep a set of already seen support file patterns, because
             # repeatedly processing the patterns from the default section
             # for every test is quite costly (see bug 922517).
             extras = (('head', set()),
                       ('tail', set()),
@@ -426,32 +422,19 @@ class TreeMetadataEmitter(LoggingMixin):
                 for thing, seen in extras:
                     value = test.get(thing, '')
                     if value in seen:
                         continue
                     seen.add(value)
                     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))
                             # Only install paths in our directory. This
                             # rule is somewhat arbitrary and could be lifted.
                             if not full.startswith(manifest_dir):
                                 continue
 
--- a/python/mozbuild/mozbuild/test/backend/data/test-manifests-written/xpcshell.ini
+++ b/python/mozbuild/mozbuild/test/backend/data/test-manifests-written/xpcshell.ini
@@ -1,3 +1,4 @@
 [DEFAULT]
+support-files = support/**
 
 [xpcshell.js]
--- a/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
+++ b/python/mozbuild/mozbuild/test/backend/test_recursivemake.py
@@ -377,16 +377,28 @@ class TestRecursiveMakeBackend(BackendTe
         with open(all_tests_path, 'rt') as fh:
             o = json.load(fh)
 
             self.assertIn('xpcshell.js', o)
             self.assertIn('dir1/test_bar.js', o)
 
             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)
 
         # Install manifests should contain entries.
         install_dir = os.path.join(env.topobjdir, '_build_manifests',
             'install')
         self.assertTrue(os.path.isfile(os.path.join(install_dir, 'dist_idl')))
--- a/python/mozbuild/mozbuild/test/frontend/test_emitter.py
+++ b/python/mozbuild/mozbuild/test/frontend/test_emitter.py
@@ -243,20 +243,18 @@ class TestEmitterBasic(unittest.TestCase
         self.assertEqual(len(objs), 6)
 
         metadata = {
             'a11y.ini': {
                 'flavor': 'a11y',
                 'installs': {
                     'a11y.ini',
                     'test_a11y.js',
-                    # From ** wildcard.
-                    'a11y-support/foo',
-                    'a11y-support/dir1/bar',
                 },
+                'pattern-installs': 1,
             },
             'browser.ini': {
                 'flavor': 'browser-chrome',
                 'installs': {
                     'browser.ini',
                     'test_browser.js',
                     'support1',
                     'support2',
@@ -314,16 +312,19 @@ class TestEmitterBasic(unittest.TestCase
 
             self.assertEqual(len(o.installs), len(m['installs']))
             for path in o.installs.keys():
                 self.assertTrue(path.startswith(o.directory))
                 path = path[len(o.directory)+1:]
 
                 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')
 
         with self.assertRaisesRegexp(SandboxValidationError,
             'entry in generated-files not present elsewhere'):
             self.read_topsrcdir(reader),
 
     # This test is only needed until all harnesses support filtering from
--- a/python/mozbuild/mozpack/files.py
+++ b/python/mozbuild/mozpack/files.py
@@ -551,17 +551,20 @@ class FileFinder(BaseFinder):
 
     def _find_dir(self, path):
         '''
         Actual implementation of FileFinder.find() when the given pattern
         corresponds to an existing directory under the base directory.
         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)):
                 yield p_, f
 
     def _find_file(self, path):
         '''
         Actual implementation of FileFinder.find() when the given pattern
@@ -591,17 +594,18 @@ class FileFinder(BaseFinder):
                 yield p, f
         elif pattern[0] == '**':
             for p, f in self._find(base):
                 if mozpack.path.match(p, mozpack.path.join(*pattern)):
                     yield p, f
         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]):
                     for p_, f in self._find_glob(mozpack.path.join(base, p),
                                                  pattern[1:]):
                         yield p_, f
         else:
             for p, f in self._find_glob(mozpack.path.join(base, pattern[0]),
--- a/python/mozbuild/mozpack/manifests.py
+++ b/python/mozbuild/mozpack/manifests.py
@@ -1,21 +1,21 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 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
 
 
 # This probably belongs in a more generic module. Where?
 @contextmanager
 def _auto_fileobj(path, fileobj, mode='r'):
     if path and fileobj:
@@ -58,25 +58,34 @@ class InstallManifest(object):
       exists -- The destination path is accounted for and won't be deleted by
           the FileCopier. If the destination path doesn't exist, an error is
           raised.
 
       optional -- The destination path is accounted for and won't be deleted by
           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'
 
     SYMLINK = 1
     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.
 
         If path is defined, the manifest will be populated with data from the
         file path.
 
         If fh is defined, the manifest will be populated with data read
@@ -89,29 +98,29 @@ class InstallManifest(object):
         if not path and not fileobj:
             return
 
         with _auto_fileobj(path, fileobj, 'rb') as fh:
             self._load_from_fileobj(fh)
 
     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)
 
         for line in fileobj:
             line = line.rstrip()
 
             fields = line.split(self.FIELD_SEPARATOR)
 
             record_type = int(fields[0])
 
             if record_type == self.SYMLINK:
-                dest, source= fields[1:]
+                dest, source = fields[1:]
                 self.add_symlink(source, dest)
                 continue
 
             if record_type == self.COPY:
                 dest, source = fields[1:]
                 self.add_copy(source, dest)
                 continue
 
@@ -120,16 +129,26 @@ class InstallManifest(object):
                 self.add_required_exists(path)
                 continue
 
             if record_type == self.OPTIONAL_EXISTS:
                 _, path = fields
                 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)
 
     def __len__(self):
         return len(self._dests)
 
     def __contains__(self, item):
         return item in self._dests
@@ -153,17 +172,17 @@ class InstallManifest(object):
         """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('2\n')
+            fh.write('3\n')
 
             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))
@@ -193,16 +212,39 @@ class InstallManifest(object):
         """Record that a destination file may exist.
 
         This effectively prevents the listed file from being deleted. Unlike a
         "required exists" file, files of this type do not raise errors if the
         destination file does not exist.
         """
         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)
 
         self._dests[dest] = entry
 
     def populate_registry(self, registry):
         """Populate a mozpack.copier.FileRegistry instance with data from us.
@@ -226,10 +268,26 @@ class InstallManifest(object):
             if install_type == self.REQUIRED_EXISTS:
                 registry.add(dest, ExistingFile(required=True))
                 continue
 
             if install_type == self.OPTIONAL_EXISTS:
                 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)
--- a/python/mozbuild/mozpack/test/test_manifests.py
+++ b/python/mozbuild/mozpack/test/test_manifests.py
@@ -24,18 +24,20 @@ class TestInstallManifest(TestWithTmpDir
         self.assertEqual(len(m), 0)
 
     def test_adds(self):
         m = InstallManifest()
         m.add_symlink('s_source', 's_dest')
         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)
         self.assertIn('o_dest', m)
 
         with self.assertRaises(ValueError):
             m.add_symlink('s_other', 's_dest')
 
@@ -43,47 +45,49 @@ class TestInstallManifest(TestWithTmpDir
             m.add_copy('c_other', 'c_dest')
 
         with self.assertRaises(ValueError):
             m.add_required_exists('e_dest')
 
         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
 
     def test_serialization(self):
         m = self._get_test_manifest()
 
         p = self.tmppath('m')
         m.write(path=p)
         self.assertTrue(os.path.isfile(p))
 
         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)
         p2 = self.tmppath('m2')
         m2.write(path=p2)
 
         with open(p2, 'rb') as fh:
             c2 = fh.read()
@@ -93,26 +97,46 @@ class TestInstallManifest(TestWithTmpDir
     def test_populate_registry(self):
         m = self._get_test_manifest()
         r = FileRegistry()
         m.populate_registry(r)
 
         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')
 
         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)
 
     def test_copier_application(self):
         dest = self.tmppath('dest')
         os.mkdir(dest)