ajordanr-google wrote:

> This isn't using TINFO_LIBRARIES anywhere. Presumably you'll need to add it 
> to LLDB_CURSES_LIBS like the original spack patch.

Hmm. This patch does seem to work locally, so it's not _required_. I would have 
thought the logic is entirely in the `find_package` invocation, and the 
variables are just for the `if` check. But no issue adding it to the 
`LLDB_CURSES_LIBS`, will do that now!

---

I understand the issue in point two--this patch may unnecessarily require a 
separate tinfo library even when the headers and objects are available via 
ncurses. But could you explain more on your recommended solutions, particularly 
on why we would need to set an optional variable? 

If we're going to require a CMAKE flag anyways to show a warning, why not just 
guard it on a `LLDB_USE_SEPARATE_TERMINFO=ON` flag and not worry about 
autodetection? It's already breaking with a link time error, surely that's a 
louder signal than a configure-time warning.

https://github.com/llvm/llvm-project/pull/126810
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to