[Lldb-commits] [llvm] [lldb] Added settings for DEBUGINFOD cache location and timeout (PR #78605)

[Kevin Frei via lldb-commits]

================
@@ -112,31 +141,52 @@ SymbolLocator *SymbolLocatorDebuginfod::CreateInstance() {
   return new SymbolLocatorDebuginfod();
 }
 
-static std::optional<FileSpec> GetFileForModule(
-    const ModuleSpec &module_spec,
-    std::function<llvm::Expected<std::string>(llvm::object::BuildIDRef)>
-        PullFromServer) {
-  if (!ModuleList::GetGlobalModuleListProperties().GetEnableExternalLookup())
-    return {};
+static std::optional<FileSpec>
+GetFileForModule(const ModuleSpec &module_spec,
+                 std::function<std::string(llvm::object::BuildID)> UrlBuilder) {
   const UUID &module_uuid = module_spec.GetUUID();
-  if (module_uuid.IsValid() && llvm::canUseDebuginfod()) {
-    llvm::object::BuildID build_id(module_uuid.GetBytes());
-    llvm::Expected<std::string> result = PullFromServer(build_id);
-    if (result)
-      return FileSpec(*result);
-    // An error here should be logged as a failure in the Debuginfod library,
-    // so just consume it here
-    consumeError(result.takeError());
-  }
+  // Don't bother if we don't have a valid UUID, Debuginfod isn't available,
+  // or if the 'symbols.enable-external-lookup' setting is false.
+  if (!module_uuid.IsValid() || !llvm::canUseDebuginfod() ||
+      !ModuleList::GetGlobalModuleListProperties().GetEnableExternalLookup())
+    return {};
+
+  // Grab LLDB's Debuginfod overrides from the
+  // plugin.symbol-locator.debuginfod.* settings.
+  PluginProperties &plugin_props = GetGlobalPluginProperties();
+  llvm::Expected<std::string> cache_path_or_err = plugin_props.GetCachePath();
+  // A cache location is *required*.
+  if (!cache_path_or_err)
+    return {};
+  std::string cache_path = *cache_path_or_err;
+  llvm::SmallVector<llvm::StringRef> debuginfod_urls =
+      llvm::getDefaultDebuginfodUrls();
+  std::chrono::milliseconds timeout = plugin_props.GetTimeout();
+
+  // We're ready to ask the Debuginfod library to find our file.
+  llvm::object::BuildID build_id(module_uuid.GetBytes());
+  std::string url_path = UrlBuilder(build_id);
+  std::string cache_key = llvm::getDebuginfodCacheKey(url_path);
+  llvm::Expected<std::string> result = llvm::getCachedOrDownloadArtifact(
+      cache_key, url_path, cache_path, debuginfod_urls, timeout);
+  if (result)
+    return FileSpec(*result);
+
+  Log *log = GetLog(LLDBLog::Symbols);
+  auto err_message = llvm::toString(result.takeError());
+  LLDB_LOGV(log,
+            "[Debuginfod] Failed to download symbol artifact {0} "
----------------
kevinfrei wrote:

> I'd drop the `[Debuginfod]` as it's not something other plugins do and you can see where the log message came from which will make it obvious that that's coming from the debuginfod plugin. If you really want to include it, I'd do something like "Debuginfod failed".

I can't see that it's coming from the Debuginfod plugin: 
```
(lldb) log enable lldb symbol --verbose
(lldb) target create /bin/tar
Debuginfod failed to download symbol artifact buildid/b4e36170d228ca4ae032f06b16f69adb48d07c50/debuginfo with error curl_easy_perform() failed: Timeout was reached
```

so I'm inclined to just switch it to...what you can see I switched it to. If you want me to create a new LLDBLog category, I do believe *that* would make it more obvious. I just reused LLDBLog::Symbols.

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