bug 764671 - Stop uploading symbols for test programs/libs to the symbol server. r=nthomas
authorTed Mielczarek <ted.mielczarek@gmail.com>
Thu, 24 May 2012 11:58:35 -0400
changeset 97021 602cfef02b3d33cfe6a9f4de8c8062ba3eed3902
parent 97020 c26acecbad4de4582a38a38951f6741cfb69eac0
child 97022 38fc0b6b4e6255bfc6a0185f57f85cdc5cbedf63
push id10827
push usertmielczarek@mozilla.com
push dateTue, 19 Jun 2012 14:12:09 +0000
treeherdermozilla-inbound@f1a89d9008c8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersnthomas
bugs764671
milestone16.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 764671 - Stop uploading symbols for test programs/libs to the symbol server. r=nthomas
Makefile.in
toolkit/crashreporter/Makefile.in
toolkit/crashreporter/tools/symbolstore.py
toolkit/crashreporter/tools/unit-symbolstore.py
--- a/Makefile.in
+++ b/Makefile.in
@@ -161,16 +161,17 @@ ifdef USE_ELF_HACK
 endif
 	echo building symbol store
 	$(RM) -r $(DIST)/crashreporter-symbols
 	$(RM) "$(DIST)/$(SYMBOL_ARCHIVE_BASENAME).zip"
 	$(NSINSTALL) -D $(DIST)/crashreporter-symbols
 	OBJCOPY="$(OBJCOPY)" \
 	$(PYTHON) $(topsrcdir)/toolkit/crashreporter/tools/symbolstore.py \
 	  $(MAKE_SYM_STORE_ARGS)                                          \
+	  --exclude="*test*" --exclude="*Test*"                           \
 	  $(foreach dir,$(SYM_STORE_SOURCE_DIRS),-s $(dir))               \
 	  $(DUMP_SYMS_BIN)                                                \
 	  $(DIST)/crashreporter-symbols                                   \
 	  $(MAKE_SYM_STORE_PATH) >                                        \
 	  $(DIST)/crashreporter-symbols/$(SYMBOL_INDEX_NAME)
 	echo packing symbols
 	$(NSINSTALL) -D $(DIST)/$(PKG_PATH)
 	cd $(DIST)/crashreporter-symbols && \
--- a/toolkit/crashreporter/Makefile.in
+++ b/toolkit/crashreporter/Makefile.in
@@ -93,8 +93,11 @@ EXTRA_JS_MODULES = \
 
 ifdef ENABLE_TESTS
 TOOL_DIRS = test
 endif
 
 include $(topsrcdir)/config/config.mk
 include $(topsrcdir)/ipc/chromium/chromium-config.mk
 include $(topsrcdir)/config/rules.mk
+
+check::
+	$(PYTHON) $(srcdir)/tools/unit-symbolstore.py
--- a/toolkit/crashreporter/tools/symbolstore.py
+++ b/toolkit/crashreporter/tools/symbolstore.py
@@ -21,16 +21,17 @@
 #                    generate relative filenames.
 
 import sys
 import platform
 import os
 import re
 import shutil
 import textwrap
+import fnmatch
 from subprocess import call, Popen, PIPE, STDOUT
 from optparse import OptionParser
 
 # Utility classes
 
 class VCSFileInfo:
     """ A base class for version-controlled file information. Ensures that the
         following attributes are generated only once (successfully):
@@ -385,36 +386,42 @@ class Dumper:
     and an option to copy debug info files alongside the dumped
     symbol files--|copy_debug|, mostly useful for creating a
     Microsoft Symbol Server from the resulting output.
 
     You don't want to use this directly if you intend to call
     ProcessDir.  Instead, call GetPlatformSpecificDumper to
     get an instance of a subclass."""
     def __init__(self, dump_syms, symbol_path,
-                 archs=None, srcdirs=None, copy_debug=False, vcsinfo=False, srcsrv=False):
+                 archs=None,
+                 srcdirs=None,
+                 copy_debug=False,
+                 vcsinfo=False,
+                 srcsrv=False,
+                 exclude=[]):
         # 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()]
         if srcdirs is not None:
             self.srcdirs = [os.path.normpath(a) for a in srcdirs]
         else:
             self.srcdirs = None
         self.copy_debug = copy_debug
         self.vcsinfo = vcsinfo
         self.srcsrv = srcsrv
+        self.exclude = exclude[:]
 
     # subclasses override this
     def ShouldProcess(self, file):
-        return False
+        return not any(fnmatch.fnmatch(os.path.basename(file), exclude) for exclude in self.exclude)
 
     # and can override this
     def ShouldSkipDir(self, dir):
         return False
 
     def RunFileCommand(self, file):
         """Utility function, returns the output of file(1)"""
         try:
@@ -540,16 +547,18 @@ class Dumper:
 # logic to determine what files to extract symbols from.
 
 class Dumper_Win32(Dumper):
     fixedFilenameCaseCache = {}
 
     def ShouldProcess(self, file):
         """This function will allow processing of pdb files that have dll
         or exe files with the same base name next to them."""
+        if not Dumper.ShouldProcess(self, file):
+            return False
         if file.endswith(".pdb"):
             (path,ext) = os.path.splitext(file)
             if os.path.isfile(path + ".exe") or os.path.isfile(path + ".dll"):
                 return True
         return False
 
     def FixFilenameCase(self, file):
         """Recent versions of Visual C++ put filenames into
@@ -611,16 +620,18 @@ class Dumper_Win32(Dumper):
 
 class Dumper_Linux(Dumper):
     objcopy = os.environ['OBJCOPY'] if 'OBJCOPY' in os.environ else 'objcopy'
     def ShouldProcess(self, file):
         """This function will allow processing of files that are
         executable, or end with the .so extension, and additionally
         file(1) reports as being ELF files.  It expects to find the file
         command in PATH."""
+        if not Dumper.ShouldProcess(self, file):
+            return False
         if file.endswith(".so") or os.access(file, os.X_OK):
             return self.RunFileCommand(file).startswith("ELF")
         return False
 
     def CopyDebug(self, file, debug_file, guid):
         # We want to strip out the debug info, and add a
         # .gnu_debuglink section to the object, so the debugger can
         # actually load our debug info later.
@@ -649,26 +660,30 @@ class Dumper_Solaris(Dumper):
         except:
             return ""
 
     def ShouldProcess(self, file):
         """This function will allow processing of files that are
         executable, or end with the .so extension, and additionally
         file(1) reports as being ELF files.  It expects to find the file
         command in PATH."""
+        if not Dumper.ShouldProcess(self, file):
+            return False
         if file.endswith(".so") or os.access(file, os.X_OK):
             return self.RunFileCommand(file).startswith("ELF")
         return False
 
 class Dumper_Mac(Dumper):
     def ShouldProcess(self, file):
         """This function will allow processing of files that are
         executable, or end with the .dylib extension, and additionally
         file(1) reports as being Mach-O files.  It expects to find the file
         command in PATH."""
+        if not Dumper.ShouldProcess(self, file):
+            return False
         if file.endswith(".dylib") or os.access(file, os.X_OK):
             return self.RunFileCommand(file).startswith("Mach-O")
         return False
 
     def ShouldSkipDir(self, dir):
         """We create .dSYM bundles on the fly, but if someone runs
         buildsymbols twice, we should skip any bundles we created
         previously, otherwise we'll recurse into them and try to 
@@ -731,16 +746,19 @@ def main():
                       action="append", dest="srcdir", default=[],
                       help="Use SRCDIR to determine relative paths to source files")
     parser.add_option("-v", "--vcs-info",
                       action="store_true", dest="vcsinfo",
                       help="Try to retrieve VCS info for each FILE listed in the output")
     parser.add_option("-i", "--source-index",
                       action="store_true", dest="srcsrv", default=False,
                       help="Add source index information to debug files, making them suitable for use in a source server.")
+    parser.add_option("-x", "--exclude",
+                      action="append", dest="exclude", default=[], metavar="PATTERN",
+                      help="Skip processing files matching PATTERN.")
     (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)
@@ -750,15 +768,16 @@ def main():
         exit(1)
 
     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)
+                                       srcsrv=options.srcsrv,
+                                       exclude=options.exclude)
     for arg in args[2:]:
         dumper.Process(arg)
 
 # run main if run directly
 if __name__ == "__main__":
     main()
new file mode 100644
--- /dev/null
+++ b/toolkit/crashreporter/tools/unit-symbolstore.py
@@ -0,0 +1,96 @@
+#!/usr/bin/env python
+# 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/.
+
+import os, tempfile, unittest, shutil, struct, platform
+import symbolstore
+
+# 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))
+
+def write_pdb(filename):
+    open(filename, "w").write("aaa")
+    # write out a fake DLL too
+    open(os.path.splitext(filename)[0] + ".dll", "w").write("aaa")
+
+writer = {'Windows': write_pdb,
+          'Microsoft': write_pdb,
+          'Linux': write_elf,
+          'Sunos5': write_elf,
+          'Darwin': write_macho}[platform.system()]
+extension = {'Windows': ".pdb",
+             'Microsoft': ".pdb",
+             'Linux': ".so",
+             'Sunos5': ".so",
+             'Darwin': ".dylib"}[platform.system()]
+
+def add_extension(files):
+    return [f + extension for f in files]
+
+class TestExclude(unittest.TestCase):
+    """
+    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 += "/"
+        
+    def tearDown(self):
+        shutil.rmtree(self.test_dir)
+
+    def add_test_files(self, files):
+        for f in files:
+            f = os.path.join(self.test_dir, f)
+            d = os.path.dirname(f)
+            if d and not os.path.exists(d):
+                os.makedirs(d)
+            writer(f)
+
+    def test_exclude_wildcard(self):
+        """
+        Test that using an exclude list with a wildcard pattern works.
+        """
+        processed = []
+        def mock_process_file(filename):
+            processed.append((filename[len(self.test_dir):] if filename.startswith(self.test_dir) else filename).replace('\\', '/'))
+            return True
+        self.add_test_files(add_extension(["foo", "bar", "abc/xyz", "abc/fooxyz", "def/asdf", "def/xyzfoo"]))
+        d = symbolstore.GetPlatformSpecificDumper(dump_syms="dump_syms",
+                                                  symbol_path="symbol_path",
+                                                  exclude=["*foo*"])
+        d.ProcessFile = mock_process_file
+        self.assertTrue(d.Process(self.test_dir))
+        processed.sort()
+        expected = add_extension(["bar", "abc/xyz", "def/asdf"])
+        expected.sort()
+        self.assertEqual(processed, expected)
+
+    def test_exclude_filenames(self):
+        """
+        Test that excluding a filename without a wildcard works.
+        """
+        processed = []
+        def mock_process_file(filename):
+            processed.append((filename[len(self.test_dir):] if filename.startswith(self.test_dir) else filename).replace('\\', '/'))
+            return True
+        self.add_test_files(add_extension(["foo", "bar", "abc/foo", "abc/bar", "def/foo", "def/bar"]))
+        d = symbolstore.GetPlatformSpecificDumper(dump_syms="dump_syms",
+                                                  symbol_path="symbol_path",
+                                                  exclude=add_extension(["foo"]))
+        d.ProcessFile = mock_process_file
+        self.assertTrue(d.Process(self.test_dir))
+        processed.sort()
+        expected = add_extension(["bar", "abc/bar", "def/bar"])
+        expected.sort()
+        self.assertEqual(processed, expected)
+
+if __name__ == '__main__':
+  unittest.main()