Fix "stdscr" configure failure & ncursest support #1558

Explorer09 · 2024-10-19T04:29:58Z

This fixes the issue of #1548, while adding minor improvements to the ncurses detecttion code since #1512.

BenBE

LGTM.

Document the #if defined(HAVE_CONFIG_H) to be used for reference inside configure.ac.

Also, if not done, have the ncursest specific changes re macros and the ProvideCurses changes live inside separate commits (though that already seems to be the case AFAICS).

BenBE · 2024-10-25T14:19:41Z

configure.ac

+#ifndef _XOPEN_SOURCE_EXTENDED
+#undef _XOPEN_SOURCE_EXTENDED
+#endif


I think you got the check inverted by accident …

Suggested change

#ifndef _XOPEN_SOURCE_EXTENDED

#undef _XOPEN_SOURCE_EXTENDED

#endif

#ifdef _XOPEN_SOURCE_EXTENDED

#undef _XOPEN_SOURCE_EXTENDED

#endif

No. It is not an error. It's the peculiarity of the config.h.in format. The #undef is technically the placeholder that would be replaced by #define during configure.

TIL. Please add a small note about this to document this somewhat. TIA.

configure.ac

Explorer09 · 2024-10-27T16:55:04Z

@BenBE

I have a idea when trying to improve configure in detecting ncurses header, but am not sure whether such a change is acceptable:

--- a/ProvideCurses.h
+++ b/ProvideCurses.h
@@ -16,16 +16,13 @@ in the source distribution for its full text.
 
 // IWYU pragma: begin_exports
 
-#if defined(HAVE_NCURSESW_CURSES_H)
-#include <ncursesw/curses.h>
-#elif defined(HAVE_NCURSES_NCURSES_H)
-#include <ncurses/ncurses.h>
-#elif defined(HAVE_NCURSES_CURSES_H)
-#include <ncurses/curses.h>
-#elif defined(HAVE_NCURSES_H)
-#include <ncurses.h>
-#elif defined(HAVE_CURSES_H)
-#include <curses.h>
+// The CURSES_HEADER_NAME macro may be defined by configure script to any of these:
+// <ncursesw/curses.h>
+// <ncurses/curses.h>
+// <ncurses.h>
+// <curses.h>
+#if defined(CURSES_HEADER_NAME)
+#include CURSES_HEADER_NAME
 #endif
 
 #ifdef HAVE_LIBNCURSESW

Instead of having a lot of boolean HAVE_*CURSES_H macro tokens, I think it would be better to have one token for curses/ncurses header name. This form of include directive is supposed to work with any C89-compliant compiler, but programmers might be unfamiliar with this form of include.

BenBE · 2024-10-27T16:59:43Z

I considered it briefly when that header was created, but decided against it, as we hate the ProvideCurses.h anyway to deal with all the other aspects of curses that come bundled with terminal control stuff. And since you get those booleans for free™ from autoconf there's no real reason not using them in favour of some little-used feature of the pre-processor. So keep it as-is.

Explorer09 · 2024-10-27T23:46:44Z

I considered it briefly when that header was created, but decided against it, as we hate the ProvideCurses.h anyway to deal with all the other aspects of curses that come bundled with terminal control stuff. And since you get those booleans for free™ from autoconf there's no real reason not using them in favour of some little-used feature of the pre-processor. So keep it as-is.

The problem is, the list of possible header names for ncurses is non-exhaustive. We only check header names that are common in distributions (<ncursesw/curses.h>, <ncurses.h>, etc.), but I've experimented with ncurses' configure (upstream), and it technically allows builders to specify any suffix for the library and header directory. So the header could be ncurseswfoo/curses.h (if ncurses is configured with --with-extra-suffix=foo), ncurseswbar/curses.h (if --with-extra-suffix=bar) and so on.

I know these are custom builds and niche use cases, but as I introduced --with-curses=XXX in htop's configure, I think I should allow builders to specify custom header for ncurses (to free them from needing to hack the ProvideCurses.h). It won't be a configure option, but a CPPFLAGS='-DCURSES_HEADER_NAME=<ncurseswfoo/curses.h>' override.

BenBE · 2024-10-28T08:10:23Z

People who mess around with the header names may also just mess around with the code at that point. I doubt most programs using (n)curses at that point would even compile properly with these custom header names. That's some additional complexity added that doesn't provide reasonable benefit for the average case or is common enough to warrant going that extra kilometer.

People messing with their header names can feel lucky, that it's basically just one place they have to mess with in our code …

It makes no sense to error out if the builder runs "./configure --with-curses" and no option argument. "--with-curses" without an option argument now keeps using the default name list for detecting ncurses library. Signed-off-by: Kang-Che Sung <[email protected]>

When "configure" suggests user to install pkg-config, output the '*curses*.pc' file detected in the config.log. This aids diagnosing the configure script. Signed-off-by: Kang-Che Sung <[email protected]>

It is slightly more robust to grep for the 'pkg_m4_included' token when running "make dist". Also, shorten the "make dist" warning message, and tell users if they need to regenerate configure when we recommend them to install pkg-config. Signed-off-by: Kang-Che Sung <[email protected]>

When ncurses is compiled with reentrant support, the "stdscr" may be only available as a macro and not a symbol for direct linking. We can check for "stdscr" only after the curses headers are included. For now we can remove the "stdscr" test when checking for keypad() function. Signed-off-by: Kang-Che Sung <[email protected]>

According to ncurses man page, mvadd_wchnstr() may be implemented as a macro. Signed-off-by: Kang-Che Sung <[email protected]>

This allows configure tests to utilize the macro. Also explain the reasons that we define _XOPEN_SOURCE_EXTENDED and not _XOPEN_SOURCE. Signed-off-by: Kang-Che Sung <[email protected]>

The main thing I intended to test is "stdscr", but it is also useful to test ncurses functions that might be implemented as macros, such as refresh() and mvadd_wchnstr(). Signed-off-by: Kang-Che Sung <[email protected]>

curses.h may define its own "bool" type, because the X/Open Curses standard defines "bool" that predates C99 "bool". If curses.h "bool" is not compatible with ISO C, fail at configure time. (The C23 standard now makes "bool" a keyword so that a "typedef /*whatever*/ bool;" is no longer portable. This is a bug that curses implementations should fix.) Solaris 11 is known to ship with a broken curses.h header (the default curses, not ncurses). Signed-off-by: Kang-Che Sung <[email protected]>

Now it can guess the terminfo library name based on the ncurses library file name. E.g. "-ltinfow" for the corresponding"-lncursesw". If the guessed library name doesn't link, try the "-ltinfo" name then. Signed-off-by: Kang-Che Sung <[email protected]>

"ncursest" and "ncursestw" are multi-threaded variants of the ncurses library. The "t" library names are only used in operating systems that do not support weak symbols (so you won't see such names in Linux), but the names are documented in ncurses anyway. Adding them to the list of names to check won't hurt. * See the curs_threads(3X) man page, and the INSTALL file from ncurses package. Notably the "--enable-reentrant", "--with-pthread" and "--enable-weak-symbols" options in ncurses. Signed-off-by: Kang-Che Sung <[email protected]>

Explorer09 · 2024-11-02T10:03:10Z

I think I would defer the header detection changes to another PR. The rest of the changes are ready.

BenBE added enhancement Extension or improvement to existing feature build system 🔧 Affects the build system rather then the user experience Linux 🐧 Linux related issues labels Oct 19, 2024

BenBE requested changes Oct 19, 2024

View reviewed changes

Explorer09 force-pushed the ncursest-support branch 2 times, most recently from 5f0d8f1 to 8eac24b Compare October 25, 2024 10:14

BenBE reviewed Oct 25, 2024

View reviewed changes

configure.ac Outdated Show resolved Hide resolved

Explorer09 force-pushed the ncursest-support branch 2 times, most recently from 845bc0c to 6a4b665 Compare October 26, 2024 07:57

BenBE reviewed Oct 26, 2024

View reviewed changes

configure.ac Outdated Show resolved Hide resolved

Explorer09 force-pushed the ncursest-support branch 2 times, most recently from d5320f9 to e3f2bb2 Compare October 26, 2024 10:04

Explorer09 added 2 commits October 28, 2024 19:24

build: Log the detected '*curses*.pc' file into config.log

c3c8108

When "configure" suggests user to install pkg-config, output the '*curses*.pc' file detected in the config.log. This aids diagnosing the configure script. Signed-off-by: Kang-Che Sung <[email protected]>

Explorer09 force-pushed the ncursest-support branch 2 times, most recently from 2c4e92e to eb2ee64 Compare October 28, 2024 11:29

Explorer09 added 6 commits October 29, 2024 07:57

build: check for wadd_wchnstr() rather than mvadd_wchnstr()

b1e0b32

According to ncurses man page, mvadd_wchnstr() may be implemented as a macro. Signed-off-by: Kang-Che Sung <[email protected]>

build: Move _XOPEN_SOURCE_EXTENDED define to config.h

93f71c9

This allows configure tests to utilize the macro. Also explain the reasons that we define _XOPEN_SOURCE_EXTENDED and not _XOPEN_SOURCE. Signed-off-by: Kang-Che Sung <[email protected]>

build: Add a test for whether curses.h works.

7dfd4dd

The main thing I intended to test is "stdscr", but it is also useful to test ncurses functions that might be implemented as macros, such as refresh() and mvadd_wchnstr(). Signed-off-by: Kang-Che Sung <[email protected]>

Explorer09 force-pushed the ncursest-support branch from eb2ee64 to a1f2175 Compare October 28, 2024 23:59

Explorer09 added 2 commits November 2, 2024 17:12

Explorer09 marked this pull request as ready for review November 2, 2024 10:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix "stdscr" configure failure & ncursest support #1558

Fix "stdscr" configure failure & ncursest support #1558

Explorer09 commented Oct 19, 2024 •

edited

Loading

BenBE left a comment

BenBE Oct 25, 2024

Explorer09 Oct 25, 2024

BenBE Oct 25, 2024

Explorer09 commented Oct 27, 2024

BenBE commented Oct 27, 2024

Explorer09 commented Oct 27, 2024

BenBE commented Oct 28, 2024

Explorer09 commented Nov 2, 2024

Fix "stdscr" configure failure & ncursest support #1558

Are you sure you want to change the base?

Fix "stdscr" configure failure & ncursest support #1558

Conversation

Explorer09 commented Oct 19, 2024 • edited Loading

BenBE left a comment

Choose a reason for hiding this comment

BenBE Oct 25, 2024

Choose a reason for hiding this comment

Explorer09 Oct 25, 2024

Choose a reason for hiding this comment

BenBE Oct 25, 2024

Choose a reason for hiding this comment

Explorer09 commented Oct 27, 2024

BenBE commented Oct 27, 2024

Explorer09 commented Oct 27, 2024

BenBE commented Oct 28, 2024

Explorer09 commented Nov 2, 2024

Explorer09 commented Oct 19, 2024 •

edited

Loading