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

[Vedant Kumar via Phabricator via lldb-commits]

vsk added a comment.

>> 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@`.

Not sure I follow: why would introducing an lldb-env tool make it necessary for the top-level lit.site.cfg.py to be configured by the build (i.e. why would it preclude using a hand-written lit.site.cfg.py)?

Or: if you meant, the need to configure lldb-env is in and of itself an issue, not sure I follow that either. We rely on other configured files (e.g. the lit.site.cfg.py's) - is the issue that the configuration logic in lldb-dotest/CMakeLists.txt is especially hard to maintain?

> 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.

I think it's possible. The lit.site.cfg.py we want should have a known location relative to the lldb executable, so an lldb-env tool could import it. Perhaps I'm underestimating the complexity (there are actually at least two possible locations, `$(dirname path/to/lldb)/{../tools/lldb/test, ../test}/lit.site.cfg.py`).

>>> 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).

I'm not sure what the suggestion is here exactly. Is it to move the lldb-env tool under TEST_EXEC_ROOT? That should work for what I'm trying to do -- from that location, lldb-env can look up all the build info it needs by importing the adjacent lit.site.cfg.py, without needing to be configured.

Happy to rework things, ultimately I'm just looking for a reliable/consistent way to invoke bin/lldb without "being" a test. Also happy to discuss further offline: it may turn out that we don't really want/need to support running the just-built lldb outside of test/. After all we're interested in setting up a test in CI that runs the lldb driver -- perhaps this can just live in lldb/test/Shell.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90276