[PATCH] D112758: [llvm] [Debuginfo] Debuginfod client library.

[Noah Shutty via Phabricator via llvm-commits]

noajshu added inline comments.


================
Comment at: llvm/lib/Debuginfod/CMakeLists.txt:1
+if(LLVM_ENABLE_CURL)
+  add_llvm_component_library(LLVMDebuginfod
----------------
dblaikie wrote:
> labath wrote:
> > This may be the first case, where the very existence of a component library is predicated on the availability of a third party dep. That's not necessarily a bad thing (it'd also be strange to enable it when it is completely non-functional), but it did catch my eye.
> Yeah, I had mixed feelings about this with the machine learning - if the API is going to always be exposed, maybe the API should be in the library, and it gets compiled into a trivially uninteresting library that always fails the query - rather than having the entry point outside the library, and having that entry point not call into the library if it isn't available/couldn't do something real?
It should work this way right now. When built without curl, the debuginfod client will still be able to retrieve artifacts from the local cache.


================
Comment at: llvm/lib/Debuginfod/Debuginfod.cpp:34
+// Uses lowercase for compatibility with common debuginfod servers.
+std::string buildIDToString(const ArrayRef<uint8_t> BuildID) {
+  return llvm::toHex(BuildID, /*LowerCase=*/true);
----------------
dblaikie wrote:
> You can drop the top level const here and in similar places.
Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112758/new/

https://reviews.llvm.org/D112758