labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Looks fine to me. Just remove the redundant panel searching code, and please
make sure this works correctly from a clean build, per the inline comment.
================
Comment at: lldb/cmake/modules/FindCursesAndPanel.cmake:8
+if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND PANEL_LIBRARIES)
+ set(CURSES_PANEL_FOUND TRUE)
+else()
----------------
It's not fully clear to me what will happen when this code is run for the first
time (when `CURSES_INCLUDE_DIRS`, etc. is not defined yet). Who will set
`CURSES_PANEL_FOUND` in that case? Could you make sure this works correctly
when run for the first time on a fully clean build?
I don't know whether this is the standard way of writing find_package files,
but I'd consider just removing the caching and letting `find_package(Curses)`
and `find_library(panel)` just run every time -- they already contain some
internal caching so we're not saving much here anyway...
================
Comment at: lldb/cmake/modules/LLDBConfig.cmake:497
if (LLDB_ENABLE_CURSES)
- find_package(Curses REQUIRED)
find_library(CURSES_PANEL_LIBRARY NAMES panel DOC "The curses panel
library")
if (NOT CURSES_PANEL_LIBRARY)
----------------
mgorny wrote:
> Isn't this redundant now?
yep. it sure is.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71306/new/
https://reviews.llvm.org/D71306
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
