[PATCH] D111252: [llvm] [Support] [Debuginfo] Add http and debuginfod client libraries and llvm-debuginfod-find tool

[Pavel Labath via Phabricator via llvm-commits]

labath added inline comments.


================
Comment at: llvm/test/tools/llvm-debuginfod/debuginfod-find.test:1-2
+# RUN: python %S/debuginfod-find.test %S/Inputs llvm-debuginfod-find
+import threading
+import http.server
----------------
This seems fine for now, though if you start having more of these, it would definitely be good to factor some of this out.

You can consider renaming the file to .py to get syntax highlighting.


================
Comment at: llvm/test/tools/llvm-debuginfod/debuginfod-find.test:21
+        for args in [
+            [tool_path, 'fake_build_id', '--executable'],
+            [tool_path, 'fake_build_id', '--source=/directory/file.c'],
----------------
Where will this be storing the downloaded files? It should probably be `%t` or some derivative. And you should clean the folder before the test.


================
Comment at: llvm/test/tools/llvm-debuginfod/debuginfod-find.test:28-29
+                'DEBUGINFOD_URLS': f'http://localhost:{port}'})
+            if process.wait() != 0:
+                return 1
+
----------------
The way this is written now, the test would pass if I replaced `debuginfod-find` with `/bin/true`. Is there anything else you can check here?


================
Comment at: llvm/tools/llvm-debuginfod/llvm-debuginfod-find.cpp:94
+  } else {
+    errs() << "Must specify exactly one of --executable, "
+              "--source=/path/to/file, or --debuginfo.";
----------------
You're only checking that the user specified _at least_ one of the arguments.


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

https://reviews.llvm.org/D111252