Thread: Possible explanation for readline configuration problems
We've gotten several different reports lately of peculiar compilation
errors and warnings involving readline in 7.1. They look like configure
is actively doing the wrong thing --- for example, how could we see
reports like this:
tab-complete.c:734: `filename_completion_function' undeclared (first use in this function)
tab-complete.c:734: (Each undeclared identifier is reported only once
tab-complete.c:734: for each function it appears in.)
when the code makes a point of providing a declaration for
filename_completion_function if configure didn't see it in the headers?
After eyeballing the code I think I have a theory. psql/input.h will
preferentially include <readline/readline.h>, not <readline.h>, if both
HAVE_READLINE_READLINE_H and HAVE_READLINE_H are defined. But the tests
in configure make the opposite choice! Maybe the people who are having
trouble have two different versions of readline header files visible at
those two names, leading to configure's results being wrong for the
header file that input.h actually selects?
In normal situations this still wouldn't matter because configure only
defines one of the two symbols HAVE_READLINE_READLINE_H and HAVE_READLINE_H.
BUT: suppose someone runs configure, then installs a newer libreadline
and runs configure again? I think caching of configure results could
lead to both symbols becoming defined, if both headers are out there.
It's a bit of a reach, but I'm having a hard time seeing how configure
could produce the wrong results otherwise. Thoughts?
Andrea and Kevin, what do your src/include/config.h files have for
these symbols?
regards, tom lane
> We've gotten several different reports lately of peculiar compilation > errors and warnings involving readline in 7.1. They look like configure > is actively doing the wrong thing --- for example, how could we see > reports like this: > > tab-complete.c:734: `filename_completion_function' undeclared (first use in this function) > tab-complete.c:734: (Each undeclared identifier is reported only once > tab-complete.c:734: for each function it appears in.) > > when the code makes a point of providing a declaration for > filename_completion_function if configure didn't see it in the headers? > > After eyeballing the code I think I have a theory. psql/input.h will > preferentially include <readline/readline.h>, not <readline.h>, if both > HAVE_READLINE_READLINE_H and HAVE_READLINE_H are defined. But the tests > in configure make the opposite choice! Maybe the people who are having > trouble have two different versions of readline header files visible at > those two names, leading to configure's results being wrong for the > header file that input.h actually selects? This sounds like an excellent guess. Hard to imagine how readline has gotten such a bizarre list of configurations. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
I wrote:
> ... how could we see reports like this:
> tab-complete.c:734: `filename_completion_function' undeclared (first use in this function)
> when the code makes a point of providing a declaration for
> filename_completion_function if configure didn't see it in the headers?
Never mind ...
I pulled down readline 4.2, and the answer is depressingly clear: the
Readline boys have decided to rename filename_completion_function to
rl_filename_completion_function. This graphically illustrates the
fundamental bogosity of AC_EGREP_HEADER: it still finds a match,
blithely ignoring the fact that it matched only part of an identifier.
Most of the other compiler warnings that we've been hearing about arise
because the readline headers have been const-ified. Suppressing these
warnings across both old and new readlines will be a pain in the neck :-(
regards, tom lane
Tom Lane writes: > We've gotten several different reports lately of peculiar compilation > errors and warnings involving readline in 7.1. They look like configure > is actively doing the wrong thing --- for example, how could we see > reports like this: > > tab-complete.c:734: `filename_completion_function' undeclared (first use in this function) > tab-complete.c:734: (Each undeclared identifier is reported only once > tab-complete.c:734: for each function it appears in.) > > when the code makes a point of providing a declaration for > filename_completion_function if configure didn't see it in the headers? Because (a) the readline dudes changed the type of filename_completion_function in version 4.2 from extern char *filename_completion_function __P((char *, int)); to extern char *filename_completion_function __P((const char *, int)); and (b) the readline dudes commented out (#if 0) the declaration of filename_completion_function in the header in favour of the new rl_filename_completion_function (but left the symbol in the library). Other symbols were treated similarly, leading to implicit declarations with ints. Needless to say, the readline developers deserve to be shot for doing this in a minor release. -- Peter Eisentraut peter_e@gmx.net http://yi.org/peter-e/
Peter Eisentraut <peter_e@gmx.net> writes:
> (b) the readline dudes commented out (#if 0) the declaration of
> filename_completion_function in the header in favour of the new
> rl_filename_completion_function (but left the symbol in the library).
The "#if 0" silliness is not what's confusing configure however,
because what it's grepping is preprocessor output; it will not see the
commented-out declaration of filename_completion_function. What it
*does* see is the declaration of rl_filename_completion_function, and
because of the lack of defense against partial-word matches, mistakes
that for filename_completion_function. Our code would actually work if
AC_EGREP_HEADER weren't so nonrobust about what it will take as a match.
> Needless to say, the readline developers deserve to be shot for doing this
> in a minor release.
I don't suppose our life would be any simpler if they'd called it 5.0
instead of 4.2, though. Visions of mayhem aside, we ought to try to
persuade our code to work cleanly with 4.2 as well as earlier releases.
Do you have time to work on that for 7.1.1 (ie, end of the month or so)?
regards, tom lane