From fea1597a27b9d73cdceaa500b7bc34b9fb7c6edf Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sun, 4 Feb 2018 02:56:29 -0600 Subject: [PATCH 1/3] [cmake] Correct test for term.h (include curses.h first) The non-ncurses version of term.h requires that curses.h be first included. Only very recent versions of CMake include a LANGUAGE option to CHECK_INCLUDE_FILES, so we aren't using it and specifying CXX here.. --- cmake/ConfigureChecks.cmake | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmake/ConfigureChecks.cmake b/cmake/ConfigureChecks.cmake index 30cdf47a1..8919749fd 100644 --- a/cmake/ConfigureChecks.cmake +++ b/cmake/ConfigureChecks.cmake @@ -41,8 +41,10 @@ CHECK_CXX_SYMBOL_EXISTS(killpg "sys/types.h;signal.h" HAVE_KILLPG) CHECK_CXX_SYMBOL_EXISTS(lrand48_r stdlib.h HAVE_LRAND48_R) # mkostemp is in stdlib in glibc and FreeBSD, but unistd on macOS CHECK_CXX_SYMBOL_EXISTS(mkostemp "stdlib.h;unistd.h" HAVE_MKOSTEMP) +SET(HAVE_CURSES_H ${CURSES_HAVE_CURSES_H}) SET(HAVE_NCURSES_CURSES_H ${CURSES_HAVE_NCURSES_CURSES_H}) SET(HAVE_NCURSES_H ${CURSES_HAVE_NCURSES_H}) +CHECK_INCLUDE_FILES("curses.h;term.h" HAVE_TERM_H) CHECK_INCLUDE_FILE_CXX("ncurses/term.h" HAVE_NCURSES_TERM_H) CHECK_INCLUDE_FILE_CXX(siginfo.h HAVE_SIGINFO_H) CHECK_INCLUDE_FILE_CXX(spawn.h HAVE_SPAWN_H) @@ -60,7 +62,6 @@ CHECK_INCLUDE_FILE_CXX(sys/ioctl.h HAVE_SYS_IOCTL_H) CHECK_INCLUDE_FILE_CXX(sys/select.h HAVE_SYS_SELECT_H) CHECK_INCLUDE_FILES("sys/types.h;sys/sysctl.h" HAVE_SYS_SYSCTL_H) CHECK_INCLUDE_FILE_CXX(termios.h HAVE_TERMIOS_H) # Needed for TIOCGWINSZ -CHECK_INCLUDE_FILE_CXX(term.h HAVE_TERM_H) CHECK_CXX_SYMBOL_EXISTS(wcscasecmp wchar.h HAVE_WCSCASECMP) CHECK_CXX_SYMBOL_EXISTS(wcsdup wchar.h HAVE_WCSDUP) CHECK_CXX_SYMBOL_EXISTS(wcslcpy wchar.h HAVE_WCSLCPY) From a95a83b1407b0358c7a645094d3edfddda53a5f6 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sun, 4 Feb 2018 02:59:03 -0600 Subject: [PATCH 2/3] [cmake] Add missing HAVE_CURSES_H #cmakedefine --- config_cmake.h.in | 3 +++ 1 file changed, 3 insertions(+) diff --git a/config_cmake.h.in b/config_cmake.h.in index c38807eb4..f5e186c43 100644 --- a/config_cmake.h.in +++ b/config_cmake.h.in @@ -40,6 +40,9 @@ /* Define to 1 if you have the `mkostemp' function. */ #cmakedefine HAVE_MKOSTEMP 1 +/* Define to 1 if you have the header file. */ +#cmakedefine HAVE_CURSES_H 1 + /* Define to 1 if you have the header file. */ #cmakedefine HAVE_NCURSES_CURSES_H 1 From 63c8a197e5dc1c946cc9dcb789f3c3645d99fec0 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sun, 4 Feb 2018 02:59:37 -0600 Subject: [PATCH 3/3] [cmake] Clean up curses vs ncurses includes There were several issues with the way that the include tests for curses.h were being done that were ultimately causing fish to use the headers from ncurses but link against curses on platforms that provide an actual libcurses.so that isn't just a symlink to libncurses.so In particular, the old code was first testing for curses's cureses.h and then falling back to libncurses's implementation of the same - but that logic was reversed when it came to including term.h, in which case it was testing for the ncurses term.h and falling back to the curses.h header. Long story short, while cmake will link against libcurses.so if both libcurses.so and libncurses.so are present (unless CURSES_NEED_NCURSES evaluates to TRUE, but that makes ncurses a hard requirement), but we were brining in some of the defines from the ncurses headers, causing SIGSEGV panics when fish ultimately tried to access variables that weren't exported or were mapped to undefined areas of memory in the other library. Additionally it is an error to include termios.h prior to including the plain Jane curses.h (not ncurses/curses.h), causing errors about unimplemented types SGTTY/chtype. So far as I can tell, both curses.h and ncurses/curses.h pull in termios.h themselves so it shouldn't even be necessary to manually include it, but I have just moved its #include below that of curses.h --- src/builtin_set_color.cpp | 6 +++--- src/env.cpp | 6 +++--- src/fallback.cpp | 6 +++--- src/fallback.h | 3 --- src/input.cpp | 3 ++- src/output.cpp | 6 +++--- src/proc.cpp | 3 ++- src/screen.cpp | 6 +++--- 8 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/builtin_set_color.cpp b/src/builtin_set_color.cpp index b6d9e12bd..1ce2920ac 100644 --- a/src/builtin_set_color.cpp +++ b/src/builtin_set_color.cpp @@ -4,12 +4,12 @@ #include #include -#if HAVE_NCURSES_H +#if HAVE_CURSES_H +#include +#elif HAVE_NCURSES_H #include #elif HAVE_NCURSES_CURSES_H #include -#else -#include #endif #if HAVE_TERM_H #include diff --git a/src/env.cpp b/src/env.cpp index a65c1161d..f40bf5da7 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -16,12 +16,12 @@ #include #include -#if HAVE_NCURSES_H +#if HAVE_CURSES_H +#include +#elif HAVE_NCURSES_H #include #elif HAVE_NCURSES_CURSES_H #include -#else -#include #endif #if HAVE_TERM_H #include diff --git a/src/fallback.cpp b/src/fallback.cpp index 40d468a5f..16deb29fc 100644 --- a/src/fallback.cpp +++ b/src/fallback.cpp @@ -23,12 +23,12 @@ #if HAVE_GETTEXT #include #endif -#if HAVE_NCURSES_H +#if HAVE_CURSES_H +#include +#elif HAVE_NCURSES_H #include // IWYU pragma: keep #elif HAVE_NCURSES_CURSES_H #include -#else -#include #endif #if HAVE_TERM_H #include // IWYU pragma: keep diff --git a/src/fallback.h b/src/fallback.h index f0b3bb91f..c3a152793 100644 --- a/src/fallback.h +++ b/src/fallback.h @@ -15,9 +15,6 @@ // in . At least on OS X if we don't do this we get compilation errors do to the macro // substitution if wchar.h is included after this header. #include // IWYU pragma: keep -#if HAVE_NCURSES_H -#include // IWYU pragma: keep -#endif /// fish's internal versions of wcwidth and wcswidth, which can use an internal implementation if /// the system one is busted. diff --git a/src/input.cpp b/src/input.cpp index 9c2464509..6a5fcc87a 100644 --- a/src/input.cpp +++ b/src/input.cpp @@ -2,14 +2,15 @@ #include "config.h" #include -#include #include #include #if HAVE_TERM_H +#include #include #elif HAVE_NCURSES_TERM_H #include #endif +#include #include #include diff --git a/src/output.cpp b/src/output.cpp index 10009956f..b50d54bce 100644 --- a/src/output.cpp +++ b/src/output.cpp @@ -4,12 +4,12 @@ #include #include #include -#if HAVE_NCURSES_H +#if HAVE_CURSES_H +#include +#elif HAVE_NCURSES_H #include #elif HAVE_NCURSES_CURSES_H #include -#else -#include #endif #if HAVE_TERM_H #include diff --git a/src/proc.cpp b/src/proc.cpp index 42ec0730a..a9b313656 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -10,16 +10,17 @@ #include #include #include -#include #include #include #include #if HAVE_TERM_H +#include #include #elif HAVE_NCURSES_TERM_H #include #endif +#include #ifdef HAVE_SIGINFO_H #include #endif diff --git a/src/screen.cpp b/src/screen.cpp index 64fd40fa5..0e4b3e1b5 100644 --- a/src/screen.cpp +++ b/src/screen.cpp @@ -15,12 +15,12 @@ #include #include -#if HAVE_NCURSES_H +#if HAVE_CURSES_H +#include +#elif HAVE_NCURSES_H #include #elif HAVE_NCURSES_CURSES_H #include -#else -#include #endif #if HAVE_TERM_H #include