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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits