[Lldb-commits] [PATCH] D71306: [RFC] Change how we deal with optional dependencies

Fri Dec 20 01:43:18 PST 2019

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