Fix --without-readline and link with curses/terminfo/termcap correctly

memreflect commented 5 days ago

This corrects a bug where --with-readline=no/--without-readline doesn't actually disable readline. These days, shared libs are also used, so -lreadline alone is usually sufficient with the shared libreadline pulling in the necessary curses/terminfo/termcap lib (that's likely how this bug went unnoticed for so long). In the event that static linking is required, however, linker tests are performed to determine which lib to link with, adding the first of -lcurses, -ltinfo, -lterminfo, or -ltermcap that links with a readline program and -lreadline successfully.

If --with-readline=yes/--with-readline is specified and the linker test fails in all of the above cases, which may also be due to incorrect CFLAGS resulting in failing to find readline/readline.h and readline/history.h, then configure fails with an error message. The default is --with-readline=check so that configure can proceed without readline, and es can be built.

Support for a READLINE_CFLAGS variable is made available, so you can do something like

./configure READLINE_CFLAGS=-I/usr/local/include

without overriding your default CFLAGS.

And if you already know which linker flags to use, you can override the (potentially) time-consuming linker test using READLINE_LIBS without overriding your default LDFLAGS. For example, either of the following works with the devel/readline port in FreeBSD 14, in case your compiler doesn't automatically search /usr/local for headers and libs:

# shared libs
./configure \
    READLINE_CFLAGS=-I/usr/local/include \
    READLINE_LIBS='-L/usr/local/lib -lreadline'

# static libs
./configure \
    READLINE_CFLAGS='--static -I/usr/local/include' \
    READLINE_LIBS='-L/usr/local/lib -lreadline -ltinfow'

jpco commented 4 days ago

Wow, this is slick! At first I was surprised and dismayed that it seems to require 120 lines of custom logic to do this, but I see now there's quite a bit of handling of the READLINE_CFLAGS and READLINE_LIBS variables. It took me all day to piece it together because m4 is ... difficult.

From what I can tell, the logic is good. Maybe some additional indentation could help with legibility? I don't know how risky that is with m4 syntax, but I ended up adding indentation myself and it still seems to work and is easier for me to parse.

As an aside, how did you come up with the list of libraries (tinfow, tinfo, terminfo, termcapw, termcap) to test?

memreflect commented 4 days ago

Oops! The comment mentioning tinfow and termcapw was from an older version of the macros. That has been fixed along with some indentation improvements (unless you were thinking 2 spaces for indentation is too shallow).

I came up with that list (tinfow, ...) based on the fact that ldd /usr/local/lib/libreadline.so on FreeBSD 14.1 prints libtinfow.so.9, and further investigation revealed that there's also a libtermcapw.so library installed. However, these two libs seem to be specific to that system, and regular old -ltinfo or -lcurses works just fine anyway there.

There were also a couple of things that i could have sworn i already fixed surrounding $es_cv_readline_cflags, hence the additional changes.

jpco commented 3 days ago

That indentation is exactly what I had in mind, thanks.

jpco commented 3 days ago

Oh, would you mind updating .gitignore to add these files? (Just copy-pasting from my git status output:)

    m4/libtool.m4
    m4/ltoptions.m4
    m4/ltsugar.m4
    m4/ltversion.m4
    m4/lt~obsolete.m4

memreflect commented 3 days ago

Oh, would you mind updating .gitignore to add these files? (Just copy-pasting from my git status output:)
  m4/libtool.m4
  m4/ltoptions.m4
  m4/ltsugar.m4
  m4/ltversion.m4
  m4/lt~obsolete.m4

Done. I also ignored configure~ and config.h.in~ to further reduce the noise.

wryun / es-shell

Fix --without-readline and link with curses/terminfo/termcap correctly #120