Bug 1585993 - Resolve symlinks when generating file paths for breakpad symbol file r=gsvelto
authorCalixte Denizet <cdenizet@mozilla.com>
Thu, 03 Oct 2019 19:32:02 +0000
changeset 496227 973cd615817e91f1249dc19d2c1774e154031c85
parent 496226 f4dd9d524447fc0c34020e12d4f6a05fd42a6754
child 496228 46c1d4bde8d44e821ad01e39aae82b4ae7f72f2d
push id36647
push usernerli@mozilla.com
push dateFri, 04 Oct 2019 04:09:18 +0000
treeherdermozilla-central@678d4d2c3c4d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgsvelto
bugs1585993
milestone71.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 1585993 - Resolve symlinks when generating file paths for breakpad symbol file r=gsvelto Some paths contain symlinks and Python normpath doesn't resolve them. So it results on having wrong paths in generated breakpad file. Differential Revision: https://phabricator.services.mozilla.com/D48075
toolkit/crashreporter/tools/symbolstore.py
toolkit/crashreporter/tools/unit-symbolstore.py
--- a/toolkit/crashreporter/tools/symbolstore.py
+++ b/toolkit/crashreporter/tools/symbolstore.py
@@ -248,17 +248,17 @@ class GitFileInfo(VCSFileInfo):
 
 # Utility functions
 
 # A cache of files for which VCS info has already been determined. Used to
 # prevent extra filesystem activity or process launching.
 vcsFileInfoCache = {}
 
 if platform.system() == 'Windows':
-    def normpath(path):
+    def realpath(path):
         '''
         Normalize a path using `GetFinalPathNameByHandleW` to get the
         path with all components in the case they exist in on-disk, so
         that making links to a case-sensitive server (hg.mozilla.org) works.
 
         This function also resolves any symlinks in the path.
         '''
         # Return the original path if something fails, which can happen for paths that
@@ -293,17 +293,17 @@ if platform.system() == 'Windows':
                                                                 0) > 0:
                 # The return value of GetFinalPathNameByHandleW uses the
                 # '\\?\' prefix.
                 result = buf.value.encode(sys.getfilesystemencoding())[4:]
             ctypes.windll.kernel32.CloseHandle(handle)
         return result
 else:
     # Just use the os.path version otherwise.
-    normpath = os.path.normpath
+    realpath = os.path.realpath
 
 def IsInDir(file, dir):
     # the lower() is to handle win32+vc8, where
     # the source filenames come out all lowercase,
     # but the srcdir can be mixed case
     return os.path.abspath(file).lower().startswith(os.path.abspath(dir).lower())
 
 def GetVCSFilenameFromSrcdir(file, srcdir):
@@ -376,19 +376,19 @@ def validate_install_manifests(install_m
 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'):
-                # Any paths that get compared to source file names need to go through normpath.
-                abs_dest = normpath(os.path.join(destination, dst))
-                file_mapping[abs_dest] = normpath(src.path)
+                # Any paths that get compared to source file names need to go through realpath.
+                abs_dest = realpath(os.path.join(destination, dst))
+                file_mapping[abs_dest] = realpath(src.path)
     return file_mapping
 
 @memoize
 def get_generated_file_s3_path(filename, rel_path, bucket):
     """Given a filename, return a path formatted similarly to
     GetVCSFilename but representing a file available in an s3 bucket."""
     with open(filename, 'rb') as f:
         path = get_filename_with_digest(rel_path, f.read())
@@ -446,18 +446,18 @@ class Dumper:
         # 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()]
-        # Any paths that get compared to source file names need to go through normpath.
-        self.srcdirs = [normpath(s) for s in srcdirs]
+        # Any paths that get compared to source file names need to go through realpath.
+        self.srcdirs = [realpath(s) for s in srcdirs]
         self.copy_debug = copy_debug
         self.vcsinfo = vcsinfo
         self.srcsrv = srcsrv
         self.generated_files = generated_files or {}
         self.s3_bucket = s3_bucket
         self.file_mapping = file_mapping or {}
         # Add a static mapping for Rust sources. Since Rust 1.30 official Rust builds map
         # source paths to start with "/rust/<sha>/".
@@ -546,17 +546,17 @@ class Dumper:
                 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)
                         # We want original file paths for the source server.
                         sourcepath = filename
-                        filename = normpath(filename)
+                        filename = realpath(filename)
                         if filename in self.file_mapping:
                             filename = self.file_mapping[filename]
                         if self.vcsinfo:
                             gen_path = self.generated_files.get(filename)
                             if gen_path and self.s3_bucket:
                                 filename = get_generated_file_s3_path(filename, gen_path, self.s3_bucket)
                                 rootname = ''
                             else:
@@ -953,18 +953,18 @@ to canonical locations in the source rep
         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)
-    # Any paths that get compared to source file names need to go through normpath.
-    generated_files = {normpath(os.path.join(buildconfig.topobjdir, f)): f
+    # Any paths that get compared to source file names need to go through realpath.
+    generated_files = {realpath(os.path.join(buildconfig.topobjdir, f)): f
                        for (f, _) in get_generated_sources()}
     _, bucket = get_s3_region_and_bucket()
     dumper = GetPlatformSpecificDumper(dump_syms=args[0],
                                        symbol_path=args[1],
                                        copy_debug=options.copy_debug,
                                        archs=options.archs,
                                        srcdirs=options.srcdir,
                                        vcsinfo=options.vcsinfo,
--- a/toolkit/crashreporter/tools/unit-symbolstore.py
+++ b/toolkit/crashreporter/tools/unit-symbolstore.py
@@ -16,17 +16,17 @@ import tempfile
 import unittest
 import buildconfig
 
 from mock import patch
 from mozpack.manifests import InstallManifest
 import mozpack.path as mozpath
 
 import symbolstore
-from symbolstore import normpath
+from symbolstore import realpath
 
 # 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):
@@ -250,36 +250,36 @@ class TestGeneratedFilePath(HelperMixin,
         with open(g, 'wb') as f:
             pass
         expected = 's3:bucket:{}/{}:'.format(EMPTY_SHA512,
                                              rel_path)
         self.assertEqual(expected, symbolstore.get_generated_file_s3_path(g, rel_path, 'bucket'))
 
 
 if target_platform() == 'WINNT':
-    class TestNormpath(HelperMixin, unittest.TestCase):
-        def test_normpath(self):
+    class TestRealpath(HelperMixin, unittest.TestCase):
+        def test_realpath(self):
             # self.test_dir is going to be 8.3 paths...
             junk = os.path.join(self.test_dir, 'x')
             with open(junk, 'wb') as o:
                 o.write('x')
-            fixed_dir = os.path.dirname(normpath(junk))
+            fixed_dir = os.path.dirname(realpath(junk))
             files = [
                 'one\\two.c',
                 'three\\Four.d',
                 'Five\\Six.e',
                 'seven\\Eight\\nine.F',
             ]
             for rel_path in files:
                 full_path = os.path.normpath(os.path.join(self.test_dir,
                                                           rel_path))
                 self.make_dirs(full_path)
                 with open(full_path, 'wb') as o:
                     o.write('x')
-                fixed_path = normpath(full_path.lower())
+                fixed_path = realpath(full_path.lower())
                 fixed_path = os.path.relpath(fixed_path, fixed_dir)
                 self.assertEqual(rel_path, fixed_path)
 
     class TestSourceServer(HelperMixin, unittest.TestCase):
         @patch("subprocess.call")
         @patch("subprocess.Popen")
         def test_HGSERVER(self, mock_Popen, mock_call):
             """
@@ -335,18 +335,18 @@ class TestInstallManifest(HelperMixin, u
         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 = normpath(os.path.join(self.srcdir, s))
-            objfile = normpath(os.path.join(self.objdir, s))
+            srcfile = realpath(os.path.join(self.srcdir, s))
+            objfile = realpath(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.
@@ -416,20 +416,20 @@ class TestFileMapping(HelperMixin, unitt
             files = [[f.replace('/', os.sep) for f in x] for x in files]
         file_mapping = {}
         dumped_files = []
         expected_files = []
         self.make_dirs(os.path.join(self.objdir, 'x', 'y'))
         for s, o in files:
             srcfile = os.path.join(self.srcdir, s)
             self.make_file(srcfile)
-            expected_files.append(normpath(srcfile))
+            expected_files.append(realpath(srcfile))
             objfile = os.path.join(self.objdir, o)
             self.make_file(objfile)
-            file_mapping[normpath(objfile)] = normpath(srcfile)
+            file_mapping[realpath(objfile)] = realpath(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