bug 462159 - Use install manifests to track header files from dist/include back to srcdir in symbolstore.py. r=gps

Makefile.in

@@ -200,6 +200,7 @@ MAKE_SYM_STORE_ARGS := -c --vcs-info
 DUMP_SYMS_BIN ?= $(DIST)/host/bin/dump_syms
 MAKE_SYM_STORE_PATH := $(DIST)/bin
 endif
+MAKE_SYM_STORE_ARGS += --install-manifest=$(DEPTH)/_build_manifests/install/dist_include,$(DIST)/include
 
 SYM_STORE_SOURCE_DIRS := $(topsrcdir)
 

python/mozbuild/mozpack/manifests.py

@@ -115,7 +115,7 @@ class InstallManifest(object):
     def _load_from_fileobj(self, fileobj):
         version = fileobj.readline().rstrip()
         if version not in ('1', '2', '3', '4'):
-            raise UnreadableInstallManifest('Unknown manifest version: ' %
+            raise UnreadableInstallManifest('Unknown manifest version: %s' %
                 version)
 
         for line in fileobj:

python/mozbuild/mozpack/test/test_manifests.py

@@ -14,6 +14,7 @@ from mozpack.copier import (
 )
 from mozpack.manifests import (
     InstallManifest,
+    UnreadableInstallManifest,
 )
 from mozpack.test.test_files import TestWithTmpDir
 
@@ -23,6 +24,12 @@ class TestInstallManifest(TestWithTmpDir):
         m = InstallManifest()
         self.assertEqual(len(m), 0)
 
+    def test_malformed(self):
+        f = self.tmppath('manifest')
+        open(f, 'wb').write('junk\n')
+        with self.assertRaises(UnreadableInstallManifest):
+            m = InstallManifest(f)
+
     def test_adds(self):
         m = InstallManifest()
         m.add_symlink('s_source', 's_dest')

toolkit/crashreporter/tools/symbolstore.py

@@ -20,6 +20,7 @@
 #     -s <srcdir>  : Use <srcdir> as the top source directory to
 #                    generate relative filenames.
 
+import errno
 import sys
 import platform
 import os
@@ -34,6 +35,12 @@ import collections
 from optparse import OptionParser
 from xml.dom.minidom import parse
 
+from mozpack.copier import FileRegistry
+from mozpack.manifests import (
+    InstallManifest,
+    UnreadableInstallManifest,
+)
+
 # Utility classes
 
 class VCSFileInfo:
@@ -278,6 +285,39 @@ def GetVCSFilename(file, srcdirs):
     # we want forward slashes on win32 paths
     return (file.replace("\\", "/"), root)
 
+def validate_install_manifests(install_manifest_args):
+    args = []
+    for arg in install_manifest_args:
+        bits = arg.split(',')
+        if len(bits) != 2:
+            raise ValueError('Invalid format for --install-manifest: '
+                             'specify manifest,target_dir')
+        manifest_file, destination = map(os.path.abspath, bits)
+        if not os.path.isfile(manifest_file):
+            raise IOError(errno.ENOENT, 'Manifest file not found',
+                          manifest_file)
+        if not os.path.isdir(destination):
+            raise IOError(errno.ENOENT, 'Install directory not found',
+                          destination)
+        try:
+            manifest = InstallManifest(manifest_file)
+        except UnreadableInstallManifest:
+            raise IOError(errno.EINVAL, 'Error parsing manifest file',
+                          manifest_file)
+        args.append((manifest, destination))
+    return args
+
+def make_file_mapping(install_manifests):
+    file_mapping = {}
+    for manifest, destination in install_manifests:
+        destination = os.path.abspath(destination)
+        reg = FileRegistry()
+        manifest.populate_registry(reg)
+        for dst, src in reg:
+            if hasattr(src, 'path'):
+                file_mapping[os.path.join(destination, dst)] = src.path
+    return file_mapping
+
 def GetPlatformSpecificDumper(**kwargs):
     """This function simply returns a instance of a subclass of Dumper
     that is appropriate for the current platform."""
@@ -343,7 +383,8 @@ class Dumper:
                  vcsinfo=False,
                  srcsrv=False,
                  exclude=[],
-                 repo_manifest=None):
+                 repo_manifest=None,
+                 file_mapping=None):
         # popen likes absolute paths, at least on windows
         self.dump_syms = os.path.abspath(dump_syms)
         self.symbol_path = symbol_path
@@ -359,6 +400,7 @@ class Dumper:
         self.exclude = exclude[:]
         if repo_manifest:
             self.parse_repo_manifest(repo_manifest)
+        self.file_mapping = file_mapping or {}
 
         # book-keeping to keep track of our jobs and the cleanup work per file tuple
         self.files_record = {}
@@ -593,13 +635,9 @@ class Dumper:
                         if line.startswith("FILE"):
                             # FILE index filename
                             (x, index, filename) = line.rstrip().split(None, 2)
-                            if sys.platform == "sunos5":
-                                for srcdir in self.srcdirs:
-                                    start = filename.find(self.srcdir)
-                                    if start != -1:
-                                        filename = filename[start:]
-                                        break
-                            filename = self.FixFilenameCase(filename)
+                            filename = os.path.normpath(self.FixFilenameCase(filename))
+                            if filename in self.file_mapping:
+                                filename = self.file_mapping[filename]
                             sourcepath = filename
                             if self.vcsinfo:
                                 (filename, rootname) = GetVCSFilename(filename, self.srcdirs)
@@ -607,7 +645,7 @@ class Dumper:
                                 if vcs_root is None:
                                   if rootname:
                                      vcs_root = rootname
-                            # gather up files with hg for indexing   
+                            # gather up files with hg for indexing
                             if filename.startswith("hg"):
                                 (ver, checkout, source_file, revision) = filename.split(":", 3)
                                 sourceFileStream += sourcepath + "*" + source_file + '*' + revision + "\r\n"
@@ -881,6 +919,13 @@ def main():
                       action="store", dest="repo_manifest",
                       help="""Get source information from this XML manifest
 produced by the `repo manifest -r` command.
+""")
+    parser.add_option("--install-manifest",
+                      action="append", dest="install_manifests",
+                      default=[],
+                      help="""Use this install manifest to map filenames back
+to canonical locations in the source repository. Specify
+<install manifest filename>,<install destination> as a comma-separated pair.
 """)
     (options, args) = parser.parse_args()
 
@@ -895,6 +940,12 @@ produced by the `repo manifest -r` command.
         parser.error("not enough arguments")
         exit(1)
 
+    try:
+        manifests = validate_install_manifests(options.install_manifests)
+    except (IOError, ValueError) as e:
+        parser.error(str(e))
+        exit(1)
+    file_mapping = make_file_mapping(manifests)
     dumper = GetPlatformSpecificDumper(dump_syms=args[0],
                                        symbol_path=args[1],
                                        copy_debug=options.copy_debug,
@@ -903,7 +954,8 @@ produced by the `repo manifest -r` command.
                                        vcsinfo=options.vcsinfo,
                                        srcsrv=options.srcsrv,
                                        exclude=options.exclude,
-                                       repo_manifest=options.repo_manifest)
+                                       repo_manifest=options.repo_manifest,
+                                       file_mapping=file_mapping)
     for arg in args[2:]:
         dumper.Process(arg)
     dumper.Finish()

toolkit/crashreporter/tools/unit-symbolstore.py

@@ -8,6 +8,8 @@ import mock
 from mock import patch
 import symbolstore
 
+from mozpack.manifests import InstallManifest
+
 # Some simple functions to mock out files that the platform-specific dumpers will accept.
 # dump_syms itself will not be run (we mock that call out), but we can't override
 # the ShouldProcessFile method since we actually want to test that.
@@ -42,8 +44,8 @@ class HelperMixin(object):
     """
     def setUp(self):
         self.test_dir = tempfile.mkdtemp()
-        if not self.test_dir.endswith("/"):
-            self.test_dir += "/"
+        if not self.test_dir.endswith(os.sep):
+            self.test_dir += os.sep
         symbolstore.srcdirRepoInfo = {}
         symbolstore.vcsFileInfoCache = {}
 
@@ -294,6 +296,126 @@ if platform.system() in ("Windows", "Microsoft"):
             self.assertEqual(len(hgserver), 1)
             self.assertEqual(hgserver[0].split("=")[1], "http://example.com/repo")
 
+class TestInstallManifest(HelperMixin, unittest.TestCase):
+    def setUp(self):
+        HelperMixin.setUp(self)
+        self.srcdir = os.path.join(self.test_dir, 'src')
+        os.mkdir(self.srcdir)
+        self.objdir = os.path.join(self.test_dir, 'obj')
+        os.mkdir(self.objdir)
+        self.manifest = InstallManifest()
+        self.canonical_mapping = {}
+        for s in ['src1', 'src2']:
+            srcfile = os.path.join(self.srcdir, s)
+            objfile = os.path.join(self.objdir, s)
+            self.canonical_mapping[objfile] = srcfile
+            self.manifest.add_copy(srcfile, s)
+        self.manifest_file = os.path.join(self.test_dir, 'install-manifest')
+        self.manifest.write(self.manifest_file)
+
+    def testMakeFileMapping(self):
+        '''
+        Test that valid arguments are validated.
+        '''
+        arg = '%s,%s' % (self.manifest_file, self.objdir)
+        ret = symbolstore.validate_install_manifests([arg])
+        self.assertEqual(len(ret), 1)
+        manifest, dest = ret[0]
+        self.assertTrue(isinstance(manifest, InstallManifest))
+        self.assertEqual(dest, self.objdir)
+
+        file_mapping = symbolstore.make_file_mapping(ret)
+        for obj, src in self.canonical_mapping.iteritems():
+            self.assertTrue(obj in file_mapping)
+            self.assertEqual(file_mapping[obj], src)
+
+    def testMissingFiles(self):
+        '''
+        Test that missing manifest files or install directories give errors.
+        '''
+        missing_manifest = os.path.join(self.test_dir, 'missing-manifest')
+        arg = '%s,%s' % (missing_manifest, self.objdir)
+        with self.assertRaises(IOError) as e:
+            symbolstore.validate_install_manifests([arg])
+            self.assertEqual(e.filename, missing_manifest)
+
+        missing_install_dir = os.path.join(self.test_dir, 'missing-dir')
+        arg = '%s,%s' % (self.manifest_file, missing_install_dir)
+        with self.assertRaises(IOError) as e:
+            symbolstore.validate_install_manifests([arg])
+            self.assertEqual(e.filename, missing_install_dir)
+
+    def testBadManifest(self):
+        '''
+        Test that a bad manifest file give errors.
+        '''
+        bad_manifest = os.path.join(self.test_dir, 'bad-manifest')
+        with open(bad_manifest, 'wb') as f:
+            f.write('junk\n')
+        arg = '%s,%s' % (bad_manifest, self.objdir)
+        with self.assertRaises(IOError) as e:
+            symbolstore.validate_install_manifests([arg])
+            self.assertEqual(e.filename, bad_manifest)
+
+    def testBadArgument(self):
+        '''
+        Test that a bad manifest argument gives an error.
+        '''
+        with self.assertRaises(ValueError) as e:
+            symbolstore.validate_install_manifests(['foo'])
+
+class TestFileMapping(HelperMixin, unittest.TestCase):
+    def setUp(self):
+        HelperMixin.setUp(self)
+        self.srcdir = os.path.join(self.test_dir, 'src')
+        os.mkdir(self.srcdir)
+        self.objdir = os.path.join(self.test_dir, 'obj')
+        os.mkdir(self.objdir)
+        self.symboldir = os.path.join(self.test_dir, 'symbols')
+        os.mkdir(self.symboldir)
+
+    @patch("subprocess.Popen")
+    def testFileMapping(self, mock_Popen):
+        files = [('a/b', 'mozilla/b'),
+                 ('c/d', 'foo/d')]
+        if os.sep != '/':
+            files = [[f.replace('/', os.sep) for f in x] for x in files]
+        file_mapping = {}
+        dumped_files = []
+        expected_files = []
+        for s, o in files:
+            srcfile = os.path.join(self.srcdir, s)
+            expected_files.append(srcfile)
+            file_mapping[os.path.join(self.objdir, o)] = srcfile
+            dumped_files.append(os.path.join(self.objdir, 'x', 'y',
+                                             '..', '..', o))
+        # mock the dump_syms output
+        file_id = ("X" * 33, 'somefile')
+        def mk_output(files):
+            return iter(
+                [
+                    'MODULE os x86 %s %s\n' % file_id
+                ] +
+                [
+                    'FILE %d %s\n' % (i,s) for i, s in enumerate(files)
+                ] +
+                [
+                    'PUBLIC xyz 123\n'
+                ]
+            )
+        mock_Popen.return_value.stdout = mk_output(dumped_files)
+
+        d = symbolstore.Dumper('dump_syms', self.symboldir,
+                               file_mapping=file_mapping)
+        f = os.path.join(self.objdir, 'somefile')
+        open(f, 'wb').write('blah')
+        d.Process(f)
+        d.Finish(stop_pool=False)
+        expected_output = ''.join(mk_output(expected_files))
+        symbol_file = os.path.join(self.symboldir,
+                                   file_id[1], file_id[0], file_id[1] + '.sym')
+        self.assertEqual(open(symbol_file, 'r').read(), expected_output)
+
 if __name__ == '__main__':
     # use the multiprocessing.dummy module to use threading wrappers so
     # that our mocking/module-patching works