-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[lldb] Fix manual CURSES_LIBRARIES tinfo finding #128245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-lldb Author: Jordan R AW (ajordanr-google) ChangesAt present, we automatically detect terminfo symbols in CURSES_LIBRARIES after it is found through This commit preemptively attempts to fix issues encountered on systems that manually pass CURSES_LIBRARIES which already contain the necessary terminfo symbols (e.g. 'acs_map'). Additionally, let's not make CURSES_LIBRARIES a list. That was odd to begin with. See this breakage in Google Fuchsia: References Issue #101368 Full diff: https://github.com/llvm/llvm-project/pull/128245.diff 1 Files Affected:
diff --git a/lldb/cmake/modules/FindCursesAndPanel.cmake b/lldb/cmake/modules/FindCursesAndPanel.cmake
index 75ebaa35d7ea1..315810c87b67b 100644
--- a/lldb/cmake/modules/FindCursesAndPanel.cmake
+++ b/lldb/cmake/modules/FindCursesAndPanel.cmake
@@ -6,15 +6,24 @@
include(CMakePushCheckState)
-function(lldb_check_curses_tinfo CURSES_LIBRARIES CURSES_HAS_TINFO)
+function(lldb_check_curses_tinfo CURSES_HEADER CURSES_LIBRARIES CURSES_HAS_TINFO)
cmake_reset_check_state()
set(CMAKE_REQUIRED_LIBRARIES "${CURSES_LIBRARIES}")
# acs_map is one of many symbols that are part of tinfo but could
# be bundled in curses.
- check_symbol_exists(acs_map "curses.h" CURSES_HAS_TINFO)
+ check_symbol_exists(acs_map "${CURSES_HEADER}" CURSES_HAS_TINFO)
endfunction()
-if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND TINFO_LIBRARIES AND PANEL_LIBRARIES)
+if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND PANEL_LIBRARIES)
+ if(NOT HAS_TERMINFO_SYMBOLS)
+ lldb_check_curses_tinfo("${CURSES_INCLUDE_DIRS}/curses.h"
+ "${CURSES_LIBRARIES}"
+ CURSES_HAS_TINFO)
+ if (NOT CURSES_HAS_TINFO)
+ message(WARNING "CURSES_LIBRARIES was provided manually but is missing terminfo symbols")
+ endif()
+ mark_as_advanced(CURSES_HAS_TINFO)
+ endif()
set(CURSESANDPANEL_FOUND TRUE)
else()
find_package(Curses QUIET)
@@ -25,7 +34,9 @@ else()
# Sometimes the curses libraries define their own terminfo symbols,
# other times they're extern and are defined by a separate terminfo library.
# Auto-detect which.
- lldb_check_curses_tinfo("${CURSES_LIBRARIES}" CURSES_HAS_TINFO)
+ lldb_check_curses_tinfo("${CURSES_INCLUDE_DIRS}/curses.h"
+ "${CURSES_LIBRARIES}"
+ CURSES_HAS_TINFO)
if (NOT CURSES_HAS_TINFO)
message(STATUS "curses library missing terminfo symbols, looking for tinfo separately")
find_library(TINFO_LIBRARIES NAMES tinfo DOC "The curses tinfo library" QUIET)
|
At present, we automatically detect terminfo symbols in CURSES_LIBRARIES after it is found through `find_package`. However, by introducing a check for TINFO_LIBRARIES, we break systems which pass all of CURSES_INCLUDE_DIRS, CURSES_LIBRARIES, and PANEL_LIBRARIES individually without passing TINFO_LIBRARIES. We'd rather not expose TINFO_LIBRARIES at all. This commit preemptively attempts to fix issues encountered on systems that manually pass CURSES_LIBRARIES which already contain the necessary terminfo symbols (e.g. 'acs_map'). Additionally, let's not make CURSES_LIBRARIES a list. That was odd to begin with. See this breakage in Google Fuchsia: https://issues.fuchsia.dev/397455029 References Issue llvm#101368
Rebasing to fix merge conflict... |
9b67d21
to
f0d81ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
(cherry-pick from commit: bb6a273)
(cherry picked from commit bb6a273)
At present, we automatically detect terminfo symbols in CURSES_LIBRARIES after it is found through
find_package
. However, by introducing a check for TINFO_LIBRARIES, we break systems which pass all of CURSES_INCLUDE_DIRS, CURSES_LIBRARIES, and PANEL_LIBRARIES individually without passing TINFO_LIBRARIES. We'd rather not expose TINFO_LIBRARIES at all.This commit preemptively attempts to fix issues encountered on systems that manually pass CURSES_LIBRARIES which already contain the necessary terminfo symbols (e.g. 'acs_map').
See this breakage in Google Fuchsia:
https://issues.fuchsia.dev/397455029
References Issue #101368