ashgti wrote: > The more I think about it, the less I like the idea of using `LLDBLog` in > `lldb-dap`. Initially, I liked the idea of reusing our existing logging > implementation, although I have expressed my concern about relying on > `liblldb` in other PRs. But the issues and workarounds we're talking about > here sound to me like signs that we're taking it too far.
I think I also ran into this issue when using some of the Host types in lldb-dap where we had two different 'global' instances of a value due to the static linking. Specifically this was the in the `lldb_private::FileSystem` API and its when I was trying to handling some defaults correctly across different platforms. I ended up re-writing the patch to not use that API specifically. > > I think if we want to do this right, we have two options, and they're both > pretty drastic: > > 1. We expose `LLDBLog` from the SB API. This is something that has come up in > the past in the context of people writing LLDB scripts. Similar to exposing > the ability to generate progress events from the SB API, I think we need to > be careful about this and clearly distinguish between unvetted logs, coming > from scripts and vetted logs, coming from LLDB itself. > 2. We give up on trying to build lldb-dap on top the SB API and move the DAP > functionality into liblldb, allowing us to use `lldb_private` and exposing a > simple interface that gets set up in the `lldb-dap` binary, similar to the > command line driver. On the one hand I think it'd be sad to give up on > building on top of the SB API. I always liked the idea that in theory, you > could use `lldb-dap` with a different version of `liblldb`. In practice, I > don't know if anyone does. We've already been cheating in that regard but > we've gotten away with it because of static linking and I have a feeling this > is the direction we'll likely keep on going. It's just too hard to pass up on > reusing existing functionality. At Google, we do actually do this to support building lldb-dap at head with the LLDB.framework linked from Xcode. This is a pretty specific use case because we are doing that so we can support mobile devices. And every once in a while when lldb-dap uses a new SB API that isn't in the internally supported Xcode release I end up writing either a back port or a fallback implementation. > There is of course the third option which is designing a dedicated logging > framework for `lldb-dap`. Although `LLDBLog` has is upsides, it also has its > drawbacks, most notably that everything is global. If you change something in > one `(SB)Debugger` instance, it is reflected in all the other ones. We'd have > a similar problem for `lldb-dap` where someone changing the logging settings > in one instance will affect all instances in the same process, which will be > especially problematic in server mode. I think there are some improvements we could make to the logging incrementally, instead of just passing a `std::ofstream *` around. Thinking about these issues, I agree that we may not want to use the LLDBLog api and instead maybe we can make some incremental improvements to the existing system instead. I may revisit this after I get my other patch in to refactor the IOStream into a Transport abstraction and then I had two other follow ups for better type handling and cancel request support. For now I think I'll just close this PR. https://github.com/llvm/llvm-project/pull/129294 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
