bug 462159 - Use install manifests to track header files from dist/include back to srcdir in symbolstore.py. r=gps
authorTed Mielczarek <ted@mielczarek.org>
Fri, 18 Jul 2014 16:33:34 -0400
changeset 196190 a660ebbe6ff97b428c78f5f4e5921b094e57afb1
parent 196189 c809045c156ab35b73ab6732c5acb45b4b556ef8
child 196191 8613370d058a04d07d44460bbe74686cac152f04
push id46801
push usertmielczarek@mozilla.com
push dateSat, 26 Jul 2014 00:41:27 +0000
treeherdermozilla-inbound@8613370d058a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgps
bugs462159
milestone34.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 462159 - Use install manifests to track header files from dist/include back to srcdir in symbolstore.py. r=gps
Makefile.in
python/mozbuild/mozpack/manifests.py
python/mozbuild/mozpack/test/test_manifests.py
toolkit/crashreporter/tools/symbolstore.py
toolkit/crashreporter/tools/unit-symbolstore.py
--- a/Makefile.in
+++ b/Makefile.in
@@ -195,16 +195,17 @@ MAKE_SYM_STORE_PATH := $(DIST)/bin
 endif
 DUMP_SYMS_BIN ?= $(DIST)/host/bin/dump_syms
 endif
 ifeq (,$(filter-out Linux SunOS,$(OS_ARCH)))
 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)
 
 ifndef JS_STANDALONE
 include $(topsrcdir)/toolkit/mozapps/installer/package-name.mk
 
 ifdef MOZ_SYMBOLS_EXTRA_BUILDID
 EXTRA_BUILDID := -$(MOZ_SYMBOLS_EXTRA_BUILDID)
--- a/python/mozbuild/mozpack/manifests.py
+++ b/python/mozbuild/mozpack/manifests.py
@@ -110,17 +110,17 @@ class InstallManifest(object):
         if path or fileobj:
             with _auto_fileobj(path, fileobj, 'rb') as fh:
                 self._source_file = fh.name
                 self._load_from_fileobj(fh)
 
     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:
             line = line.rstrip()
 
             fields = line.split(self.FIELD_SEPARATOR)
 
             record_type = int(fields[0])
--- a/python/mozbuild/mozpack/test/test_manifests.py
+++ b/python/mozbuild/mozpack/test/test_manifests.py
@@ -9,25 +9,32 @@ import os
 import mozunit
 
 from mozpack.copier import (
     FileCopier,
     FileRegistry,
 )
 from mozpack.manifests import (
     InstallManifest,
+    UnreadableInstallManifest,
 )
 from mozpack.test.test_files import TestWithTmpDir
 
 
 class TestInstallManifest(TestWithTmpDir):
     def test_construct(self):
         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')
         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')
--- a/toolkit/crashreporter/tools/symbolstore.py
+++ b/toolkit/crashreporter/tools/symbolstore.py
@@ -15,30 +15,37 @@
 #   Parameters accepted:
 #     -c           : Copy debug info files to the same directory structure
 #                    as sym files
 #     -a "<archs>" : Run dump_syms -a <arch> for each space separated
 #                    cpu architecture in <archs> (only on OS X)
 #     -s <srcdir>  : Use <srcdir> as the top source directory to
 #                    generate relative filenames.
 
+import errno
 import sys
 import platform
 import os
 import re
 import shutil
 import textwrap
 import fnmatch
 import subprocess
 import urlparse
 import multiprocessing
 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:
     """ A base class for version-controlled file information. Ensures that the
         following attributes are generated only once (successfully):
 
             self.root
             self.clean_root
@@ -273,16 +280,49 @@ def GetVCSFilename(file, srcdirs):
 
     if fileInfo:
         file = fileInfo.filename
         root = fileInfo.root
 
     # 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."""
     # Python 2.5 has a bug where platform.system() returns 'Microsoft'.
     # Remove this when we no longer support Python 2.5.
     return {'Windows': Dumper_Win32,
             'Microsoft': Dumper_Win32,
             'Linux': Dumper_Linux,
@@ -338,32 +378,34 @@ class Dumper:
     instances."""
     def __init__(self, dump_syms, symbol_path,
                  archs=None,
                  srcdirs=[],
                  copy_debug=False,
                  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
         if archs is None:
             # makes the loop logic simpler
             self.archs = ['']
         else:
             self.archs = ['-a %s' % a for a in archs.split()]
         self.srcdirs = [os.path.normpath(a) for a in srcdirs]
         self.copy_debug = copy_debug
         self.vcsinfo = vcsinfo
         self.srcsrv = srcsrv
         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 = {}
         self.jobs_record = collections.defaultdict(int)
 
     @classmethod
     def GlobalInit(cls, module=multiprocessing):
         """Initialize the class globals for the multiprocessing setup; must
@@ -588,31 +630,27 @@ class Dumper:
                         pass
                     f = open(full_path, "w")
                     f.write(module_line)
                     # now process the rest of the output
                     for line in proc.stdout:
                         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)
                                 # sets vcs_root in case the loop through files were to end on an empty rootname
                                 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"
                             f.write("FILE %s %s\n" % (index, filename))
                         else:
                             # pass through all other lines unchanged
                             f.write(line)
                             # we want to return true only if at least one line is not a MODULE or FILE line
@@ -877,38 +915,52 @@ def main():
     parser.add_option("-x", "--exclude",
                       action="append", dest="exclude", default=[], metavar="PATTERN",
                       help="Skip processing files matching PATTERN.")
     parser.add_option("--repo-manifest",
                       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()
 
     #check to see if the pdbstr.exe exists
     if options.srcsrv:
         pdbstr = os.environ.get("PDBSTR_PATH")
         if not os.path.exists(pdbstr):
             print >> sys.stderr, "Invalid path to pdbstr.exe - please set/check PDBSTR_PATH.\n"
             sys.exit(1)
 
     if len(args) < 3:
         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,
                                        archs=options.archs,
                                        srcdirs=options.srcdir,
                                        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()
 
 # run main if run directly
 if __name__ == "__main__":
     # set up the multiprocessing infrastructure before we start;
     # note that this needs to be in the __main__ guard, or else Windows will choke
--- a/toolkit/crashreporter/tools/unit-symbolstore.py
+++ b/toolkit/crashreporter/tools/unit-symbolstore.py
@@ -3,16 +3,18 @@
 # 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/.
 
 import os, tempfile, unittest, shutil, struct, platform, subprocess, multiprocessing.dummy
 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.
 def write_elf(filename):
     open(filename, "wb").write(struct.pack("<7B45x", 0x7f, ord("E"), ord("L"), ord("F"), 1, 1, 1))
 
 def write_macho(filename):
     open(filename, "wb").write(struct.pack("<I28x", 0xfeedface))
@@ -37,18 +39,18 @@ def add_extension(files):
     return [f + extension for f in files]
 
 class HelperMixin(object):
     """
     Test that passing filenames to exclude from processing works.
     """
     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 = {}
 
     def tearDown(self):
         shutil.rmtree(self.test_dir)
         symbolstore.srcdirRepoInfo = {}
         symbolstore.vcsFileInfoCache = {}
 
@@ -289,16 +291,136 @@ if platform.system() in ("Windows", "Mic
             d.CopyDebug = lambda *args: True
             d.Process(self.test_dir)
             d.Finish(stop_pool=False)
             self.assertNotEqual(srcsrv_stream, None)
             hgserver = [x.rstrip() for x in srcsrv_stream.splitlines() if x.startswith("HGSERVER=")]
             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
     symbolstore.Dumper.GlobalInit(module=multiprocessing.dummy)
 
     unittest.main()
 
     symbolstore.Dumper.pool.close()