Commit <74416b1d4849ef77ef31de5344dd75f03094434b> added check for
rl_completion_quote_character to make sure we have correct readline
library. Commit <a9443bc9a9ef451b46306e66ed3b706756fc1414> added
inaccurate comment that it's a function.

We need to check for generic symbol instead of checking for function.
In addition the readline/readline.h file requires stdio.h to by included
beforehand which was done in autotools but I dropped it in meson.

And lastly the final condition to print error or disable readline was
broken as well by replacing the readline_dep every time if readline was
not explicitly enabled.

Reported-by: Erik Skultety <eskultet@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 meson.build | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/meson.build b/meson.build
index ad269640ba..19b4795527 100644
--- a/meson.build
+++ b/meson.build
@@ -1268,17 +1268,19 @@ if not readline_dep.found()
   readline_dep = cc.find_library('readline', required: get_option('readline'))
 
   if readline_dep.found()
-    # This function is present in all reasonable (5.0+) readline versions;
+    # This variable is present in all reasonable (5.0+) readline versions;
     # however, the macOS base system contains a library called libedit which
     # takes over the readline name despite lacking many of its features. We
     # want to make sure we only enable readline support when linking against
     # the actual readline library, and the availability of this specific
-    # functions is as good a witness for that fact as any.
-    correct_rl = cc.has_function('rl_completion_quote_character', prefix: '#include <readline/readline.h>')
-    if not correct_rl and get_option('readline').enabled()
-      error('readline is missing rl_completion_quote_character')
-    else
-      readline_dep = dependency('', required: false)
+    # variable is as good a witness for that fact as any.
+    correct_rl = cc.has_header_symbol('readline/readline.h', 'rl_completion_quote_character', prefix: '#include <stdio.h>')
+    if not correct_rl
+      if get_option('readline').enabled()
+        error('readline is missing rl_completion_quote_character')
+      else
+        readline_dep = dependency('', required: false)
+      endif
     endif
   endif
 endif
-- 
2.26.2

On Wed, 2020-08-05 at 12:51 +0200, Pavel Hrdina wrote:
> Commit <74416b1d4849ef77ef31de5344dd75f03094434b> added check for
> rl_completion_quote_character to make sure we have correct readline
> library. Commit <a9443bc9a9ef451b46306e66ed3b706756fc1414> added
> inaccurate comment that it's a function.

So it was my fault after all O:-)

> +    correct_rl = cc.has_header_symbol('readline/readline.h', 'rl_completion_quote_character', prefix: '#include <stdio.h>')
> +    if not correct_rl
> +      if get_option('readline').enabled()
> +        error('readline is missing rl_completion_quote_character')

This matches the original error message, but thinking about it again
I would suggest changing it to something like

  error('readline >= @0@ was not found'.format(readline_version))

because the specifics of the symbol are not important here, we just
happen to be using it as a witness.

Either way,

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

and thanks for taking care of this :)

-- 
Andrea Bolognani / Red Hat / Virtualization

On Wed, Aug 05, 2020 at 01:16:01PM +0200, Andrea Bolognani wrote:
> On Wed, 2020-08-05 at 12:51 +0200, Pavel Hrdina wrote:
> > Commit <74416b1d4849ef77ef31de5344dd75f03094434b> added check for
> > rl_completion_quote_character to make sure we have correct readline
> > library. Commit <a9443bc9a9ef451b46306e66ed3b706756fc1414> added
> > inaccurate comment that it's a function.
> 
> So it was my fault after all O:-)
> 
> > +    correct_rl = cc.has_header_symbol('readline/readline.h', 'rl_completion_quote_character', prefix: '#include <stdio.h>')
> > +    if not correct_rl
> > +      if get_option('readline').enabled()
> > +        error('readline is missing rl_completion_quote_character')
> 
> This matches the original error message, but thinking about it again
> I would suggest changing it to something like
> 
>   error('readline >= @0@ was not found'.format(readline_version))
> 
> because the specifics of the symbol are not important here, we just
> happen to be using it as a witness.

Most of the error messages could use some rewording and I feel that it
should be done separately.

> Either way,
> 
>   Reviewed-by: Andrea Bolognani <abologna@redhat.com>

Thanks, I'll leave the error message cleanup for different day.

Pavel

On Wed, 2020-08-05 at 13:32 +0200, Pavel Hrdina wrote:
> On Wed, Aug 05, 2020 at 01:16:01PM +0200, Andrea Bolognani wrote:
> > On Wed, 2020-08-05 at 12:51 +0200, Pavel Hrdina wrote:
> > > +    correct_rl = cc.has_header_symbol('readline/readline.h', 'rl_completion_quote_character', prefix: '#include <stdio.h>')
> > > +    if not correct_rl
> > > +      if get_option('readline').enabled()
> > > +        error('readline is missing rl_completion_quote_character')
> > 
> > This matches the original error message, but thinking about it again
> > I would suggest changing it to something like
> > 
> >   error('readline >= @0@ was not found'.format(readline_version))
> > 
> > because the specifics of the symbol are not important here, we just
> > happen to be using it as a witness.
> 
> Most of the error messages could use some rewording and I feel that it
> should be done separately.

Right. For some reason I mistakenly thought you were re-introducing
an error message that was missing from the Meson rewrite, but I see
now you're just moving it, so I completely agree with you.

-- 
Andrea Bolognani / Red Hat / Virtualization