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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Thanks. Generally LG



================
Comment at: llvm/lib/Debuginfod/Debuginfod.cpp:195
+  SmallVector<std::string, 0> Headers;
+  for (StringRef Line : split((*HeadersFile)->getBuffer(), '\n')) {
+    if (!isHeader(Line))
----------------
For StringExtras STLExtras, `llvm::` is common, `llvm::split`


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


================
Comment at: llvm/test/tools/llvm-debuginfod-find/Inputs/capture_req.py:20
+  t = threading.Thread(target=httpd.serve_forever).start()
+  os.environ['DEBUGINFOD_URLS'] =f"http://localhost:{port}"
+  subprocess.run(sys.argv[1:], capture_output = True)
----------------
Single quotes are more common


================
Comment at: llvm/test/tools/llvm-debuginfod-find/headers.test:13
+NO-HEADERS-NOT: A: B
+HEADERS: A: B
+HEADERS: C: D
----------------
The check can be made stricter
```
HEADERS:      Accept: */*
HEADERS-NEXT: A: B
HEADERS-NEXT: ...
HEADERS-NOT:  {{.}}
```

Then `NO-HEADERS-NOT` can be removed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136303