[PATCH] D136303: [Debuginfod] DEBUGINFOD_HEADERS_FILE environment variable.

[Daniel Thornburgh via Phabricator via llvm-commits]

mysterymath marked 4 inline comments as done.
mysterymath added a comment.

In D136303#3910194 <https://reviews.llvm.org/D136303#3910194>, @MaskRay wrote:

> I'd use llvm-debuginfod instead of Debuginfod as the tag to make it clear the one in llvm-project is an alternative implementation.

I would, but there's already a llvm-debuginfod command line utility in LLVM, which makes this ambiguous.



================
Comment at: llvm/lib/Debuginfod/Debuginfod.cpp:196
+  for (StringRef Line : split((*HeadersFile)->getBuffer(), '\n')) {
+    if (!isHeader(Line))
+      continue;
----------------
MaskRay wrote:
> Should there be a warning for non-header lines? If comments are useful, we can allow `#` comment markers.
Added a warning; this should help keep the semantics of this file very simple.


================
Comment at: llvm/test/tools/llvm-debuginfod-find/headers.test:13
+NO-HEADERS-NOT: A: B
+HEADERS: A: B
+HEADERS: C: D
----------------
MaskRay wrote:
> The check can be made stricter
> ```
> HEADERS:      Accept: */*
> HEADERS-NEXT: A: B
> HEADERS-NEXT: ...
> HEADERS-NOT:  {{.}}
> ```
> 
> Then `NO-HEADERS-NOT` can be removed
Mostly done, but I think we still do want a check of some kind in the NO-HEADERS case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136303