Handle symlinks in module search by deduplicating at the right place.

1. What do we need to dedup?
Only directories, not files.

Example 1: Need to deduplicate directories.
Assume:
  sys.path = ['a/b', 'd']
Where:
  a/b/c.py
  d -> a/b (symlink)

Then, a breakpoint at 'c.py:12' must have only 1 match: a/b/c.py

Example 2:
Assume:
  sys.path = ['.', 'a/b']
Where:
  a/b/c.py
  d.py -> a/b/c.py

Then, a breakpoint at 'c.py:12' must have only 1 match: a/b/c.py
And, a breakpoint at 'd.py:23' must have only 1 match: d.py
(Python compiler/interpreter treats d.py as a separate module)

2. Where do we dedup?
We can dedup in two places:
  (a) When gathering all directories we explore.
  (b) When adding a path to 'candidates' set.

Solution (a) requires remembering all directories we have already explored.
Solution (b) is simpler, but is computationally more expensive as we search
same directories multiple times. This CL uses solution (b).

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=167213003

Author: emrekultursay

src/googleclouddebugger/imphook.py

@@ -18,6 +18,8 @@
 import sys  # Must be imported, otherwise import hooks don't work.
 import threading
 
+import module_utils
+
 # Callbacks to invoke when a module is imported.
 _import_callbacks = {}
 _import_callbacks_lock = threading.Lock()
@@ -189,12 +191,13 @@ def _InvokeImportCallback(module):
   if not module:
     return
 
-  path = getattr(module, '__file__', None)
-  if not path:
+  mod_path = getattr(module, '__file__', None)
+  if not mod_path:
     return
 
-  path, unused_ext = os.path.splitext(os.path.abspath(path))
-  callbacks = _import_callbacks.get(path)
+  mod_abspath = module_utils.GetAbsolutePath(mod_path)
+  mod_abspath, unused_ext = os.path.splitext(mod_abspath)
+  callbacks = _import_callbacks.get(mod_abspath)
   if not callbacks:
     return  # Common code path.
 

src/googleclouddebugger/module_search.py

@@ -20,6 +20,7 @@
 import time
 
 import cdbg_native as native
+import module_utils
 
 
 def _CommonPathSuffixLen(paths):
@@ -43,13 +44,15 @@ def FindMatchingFiles(location_path):
   """Returns a list of absolute filenames of best matching modules/packages."""
 
   def AddCandidate(mod_path):
-    suffix_len = _CommonPathSuffixLen([src_path, mod_path])
+    # We must sanitize the module path before using it for proper deduplication.
+    mod_abspath = module_utils.GetAbsolutePath(mod_path)
+    suffix_len = _CommonPathSuffixLen([src_path, mod_abspath])
     if suffix_len < longest_suffix_len[0]:
       return
     if suffix_len > longest_suffix_len[0]:
       candidates.clear()
       longest_suffix_len[0] = suffix_len
-    candidates.add(mod_path)
+    candidates.add(mod_abspath)
 
   # We measure the time it takes to execute the scan.
   start_time = time.time()
@@ -63,54 +66,54 @@ def AddCandidate(mod_path):
 
   # Using mutable vars to make them available in nested functions.
 
-  # The set of module/package path w/ no extension. Contains absolute paths.
+  # The set of module/package path w/ no extension. Use AddCandidate() to insert
+  # into this set.
   candidates = set()
 
   # Init longest_suffix_len to 1 to avoid inserting zero length suffixes.
   longest_suffix_len = [1]
 
   # Search paths for modules and packages, init with system search paths.
-  search_abspaths = set(os.path.abspath(path) for path in sys.path)
+  search_paths = set(path for path in sys.path)
 
   # Add search paths from the already loaded packages and add matching modules
   # or packages to the candidates list.
   for module in sys.modules.values():
     # Extend the search paths with packages path and modules file directory.
-    # Note that __path__ only exist for packages, but does not have to be
-    # absolute path as the user can overwrite it.
-    search_abspaths |= frozenset(
-        os.path.abspath(path) for path in getattr(module, '__path__', []))
+    # Note that __path__ only exist for packages.
+    search_paths |= frozenset(getattr(module, '__path__', []))
     mod_path = os.path.splitext(getattr(module, '__file__', ''))[0]
+
     if not mod_path:
       continue
-    mod_abspath = os.path.abspath(mod_path)
-    search_abspaths.add(os.path.dirname(mod_abspath))
+
+    search_paths.add(os.path.dirname(mod_path))
     # Add loaded modules to the candidates set.
-    if (src_ispkg, src_name) == _GetIsPackageAndModuleName(mod_abspath):
-      AddCandidate(mod_abspath)
+    if (src_ispkg, src_name) == _GetIsPackageAndModuleName(mod_path):
+      AddCandidate(mod_path)
 
   # Walk the aggregated search path and loook for modules or packages.
   # By searching one path at the time we control the module file name
   # without having to load it.
   # TODO(erezh): consider using the alternative impl in cr/165133821 which
   # only uses os file lookup and not using pkgutil. The alternative is faster
   # but is making many more assuptions that this impl does not.
-  while search_abspaths:
+  while search_paths:
     num_dirs_scanned += 1
-    abspath = search_abspaths.pop()
-    for unused_importer, mod_name, mod_ispkg in pkgutil.iter_modules([abspath]):
-      mod_abspath = os.path.join(abspath, mod_name)
+    path = search_paths.pop()
+    for unused_importer, mod_name, mod_ispkg in pkgutil.iter_modules([path]):
+      mod_path = os.path.join(path, mod_name)
       if mod_ispkg:
-        search_abspaths.add(mod_abspath)
-        mod_abspath = os.path.join(mod_abspath, '__init__')
+        search_paths.add(mod_path)
+        mod_path = os.path.join(mod_path, '__init__')
       if src_ispkg == mod_ispkg and src_name == mod_name:
-        AddCandidate(mod_abspath)
+        AddCandidate(mod_path)
 
   # Sort the list to return a stable result to the user.
   # TODO(erezh): No need to add the .py extenssion, this is done just for
   # compatabilty with current code. Once refactored not to use file extension
   # this code can be removed to just return the sorted candidates.
-  candidates = sorted(abspath + '.py' for abspath in candidates)
+  candidates = sorted(path + '.py' for path in candidates)
 
   # Log scan stats, without the files list to avoid very long output as well as
   # the potential leak of system files that the user has no access to.

src/googleclouddebugger/module_utils.py

@@ -0,0 +1,40 @@
+# Copyright 2015 Google Inc. All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS-IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Provides utility functions for module path processing.
+"""
+
+
+import os
+
+
+def GetAbsolutePath(mod_path):
+  """Flattens symlinks and indirections in the module path.
+
+  To uniquely identify each module file, the file path must be sanitized
+  by following all symbolic links and normalizing to an absolute path.
+
+  Note that the module file (i.e., .py/.pyc/.pyo file) itself can be a
+  symbolic link, but we must *NOT* follow that symbolic link.
+
+  Args:
+    mod_path: A path that represents a module file.
+
+  Returns:
+    The sanitized version of mod_path.
+  """
+  pkg_path, file_name = os.path.split(mod_path)
+  pkg_path = os.path.abspath(os.path.realpath(pkg_path))
+  return os.path.join(pkg_path, file_name)
+

src/googleclouddebugger/python_breakpoint.py

@@ -25,6 +25,7 @@
 import imphook
 import module_explorer
 import module_search
+import module_utils
 
 # TODO(vlif): move to messages.py module.
 # Use the following schema to define breakpoint error message constant:
@@ -95,9 +96,8 @@ def _GetLoadedModuleByPath(abspath):
     if not path:
       continue  # This is a built-in module.
 
-    # module.__file__ may be relative to the current directory, so we first
-    # convert it into absolute path.
-    module_abspath = os.path.abspath(path)
+    # module.__file__ may be relative or may contain symlinks inside it.
+    module_abspath = module_utils.GetAbsolutePath(path)
 
     # Ignore file extension while comparing the file paths (e.g., /foo/bar.py vs
     # /foo/bar.pyc should match).