[Lldb-commits] [PATCH] D90276: [lldb/utils] Add the lldb-env tool

Tue Oct 27 20:31:58 PDT 2020

JDevlieghere added a comment.

In D90276#2358016 <https://reviews.llvm.org/D90276#2358016>, @vsk wrote:

> In D90276#2357978 <https://reviews.llvm.org/D90276#2357978>, @JDevlieghere wrote:
>
>> Can you explain the added value of the tool a bit more. I can see how it's nice to be able to run `export $(lldb-env); path/to/lldb` but I wonder how often you'd end up doing that. When I invoke lldb directly, it's mostly to debug a test case, in which case I'd need to run lldb under lldb and set the environment variables myself. It also means we have to configure an additional file which goes against my effort of making the lit.site.cfg file self contained (to be able to run the test suite against a different toolchain).
>
> The specific workflow I'm interested in is invoking lldb on a test program, then importing a script to drive a debug session in CI. Having the settings I need defined in one place is nice, this means I can avoid duplicating logic from a lit.cfg in my workflow. It also prevents the settings from getting out of sync (looking at this -- https://github.com/apple/llvm-project/blob/swift/main/lldb/test/Shell/lit.cfg.py#L45).

Another way to solve that would be to move it up to the top level lit configuration rather than doing it once for the shell tests and once for the API tests.

> Re: making the lit.site.cfg self-contained, IIUC the situation is that there are a couple places where we shell out via `subprocess`, but that you'd like to get rid of these? I wonder whether we could bundle up the necessary scripts along with lit.site.cfg instead.

My remark was not so much about the need for another tool but rather the need to configure it. Right now I can hand-write a `lit.site.cfg.py`, omit a bunch of values, and point lit to it. With this tool I'd need to also configure it (i.e. replacing `@LLVM_USE_SANITIZER@` and `@CMAKE_CXX_COMPILER@`. I wonder if there's any way to make `lldb-dotest` and this tool load the `lit.site.cfg.py` and use the variables that we put in the config there.

>> FWIW my plan was to deprecate `lldb-dotest` at some in favor of either using `llvm-lit` directly or by wrapping it. I hate maintaining the code in `lldb-dotest/CMakeLists.txt` because I always break the standalone build when I forget to add a variable.
>
> My intention was to make sure we get an lldb-env everywhere we already expect a lldb-dotest. If that's not required, I'd be fine with moving the new tool elsewhere.

I think that if we were to move this under `lldb/test` we might get that for free if we use the same `@`-variable names as the `lit.site.cfg.py`. I think that'd be a good idea regardless (and also most certainly a different patch).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90276