[Lldb-commits] [lldb] [lldb] Add caching and _NT_SYMBOL_PATH parsing in SymbolLocatorSymStore (PR #191782)

[Stefan Gränitz via lldb-commits]

================
@@ -162,10 +198,58 @@ def test_http_not_found(self):
     # certs, non-HTTPS redirects, etc.
     def test_http(self):
         """
-        Check that breakpoint hits with remote SymStore.
+        Check that breakpoint resolves with remote SymStore.
         """
         exe, sym = self.build_inferior()
         with MockedSymStore(self, exe, sym) as dir:
             with HTTPServer(dir) as url:
                 self.runCmd(f"settings set plugin.symbol-locator.symstore.urls {url}")
                 self.try_breakpoint(exe, should_have_loc=True)
+
+    def test_nt_symbol_path_local(self):
+        """
+        Check that breakpoint resolves with a local SymStore path in
+        _NT_SYMBOL_PATH, and that the PDB is not copied to the cache.
+        """
+        exe, sym = self.build_inferior()
+        symstore = MockedSymStore(self, exe, sym)
+        with symstore as dir:
+            # Symbol path with plain directory
+            with NtSymbolPath(dir):
+                self.try_breakpoint(exe, should_have_loc=True)
+            # Symbol path with local directory in server notation
+            with NtSymbolPath(f"srv*{dir}"):
+                self.try_breakpoint(exe, should_have_loc=True)
+            cached_files = sum(len(f) for _, _, f in os.walk(symstore.cache_dir))
+            self.assertEqual(cached_files, 0)
+
+    def test_nt_symbol_path_srv(self):
+        """
+        Check that breakpoint resolves with an HTTP symbol server in
+        _NT_SYMBOL_PATH using the srv* syntax, and that the PDB is cached.
+        """
+        exe, sym = self.build_inferior()
+        symstore = MockedSymStore(self, exe, sym)
+        with symstore as dir:
+            with HTTPServer(dir) as url:
+                with NtSymbolPath(f"srv*{url}"):
+                    self.try_breakpoint(exe, should_have_loc=True)
+            key = symstore.get_key_pdb(exe)
+            cache_file = os.path.join(symstore.cache_dir, sym, key, sym)
+            self.assertTrue(os.path.isfile(cache_file))
+            cached_files = sum(len(f) for _, _, f in os.walk(symstore.cache_dir))
+            self.assertEqual(cached_files, 1)
+
+    def test_lookup_order(self):
+        """
+        Check that _NT_SYMBOL_PATH takes precedence over symstore.urls setting.
----------------
weliveindetail wrote:

WinDBG initializes its internal symbol path with the value from the environment variable. For LLDB users setting are the familiar for such configurations and it's in sync with what we have in the Debuginfod symbol locator already. That's why I chose the explicit `symstore.urls` setting.

In my first attempt I had the setting taking precedence over the environment variable as you suggest, but then I added the cache setting and realized it should set a system default path that we take if the environment variable didn't provide a cache. So it seemed reasonable to turn `symstore.urls` into a fallback as well.

Let me state this in the property description. What do you think?

https://github.com/llvm/llvm-project/pull/191782