[Lldb-commits] [PATCH] D72823: [Reproducers] Add a tool to transparently capture and replay lldb sessions

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added inline comments.


================
Comment at: lldb/tools/lldb-repro/lldb-repro.h.cmake:12
+
+#cmakedefine LLDB_TEST_EXECUTABLE "${LLDB_TEST_EXECUTABLE}"
+
----------------
JDevlieghere wrote:
> labath wrote:
> > JDevlieghere wrote:
> > > labath wrote:
> > > > labath wrote:
> > > > > Are you sure this will work fine with multi-config generators? You might be better off just relying on the fact that the lldb executable will sit right next to this binary...
> > > > Actually how, is this thing going to be invoked exactly? Couldn't the path to lldb be passed simply as argv[1]?
> > > It just needs patching up like lldb-dotest and lit. Assuming you mean `argv[0]`, it think we could make that work if I replace "%lldb" with a path to lldb-repro.
> > No, I really meant argv[1]. :)
> > 
> > The idea was that `%lldb` would expand to `/src/path/to/lldb-repro.py /build/path/to/lldb.exe --whatever`. That way, you wouldn't need to rely on the "same directory" trick and could get rid of all the cmake code. In fact, we could even throw in a `--capture/--replay` argument to the command line, and ditch the environment variables too...
> I like the idea but FindTool is a class that's resolved by lit, and the arguments are strings. So I kept the current approach that expects to find `lldb` next to `lldb-repro`.
Ah, yes, that does make things tricky.. Yeah, having both things in the same directory seems fine...


================
Comment at: lldb/utils/lldb-repro/lldb-repro.py:24
+def help():
+    print("usage: {}: /path/to/lldb capture|replay [args]".fmt(sys.argv[0]))
+
----------------
I guess you don't need the `/path/to/lldb` part then?


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

https://reviews.llvm.org/D72823