Check for ncursesw first when searching for "tgetent"

Message ID 20180131210430.23787-1-sergiodj@redhat.com
State New, archived
Headers

Commit Message

Sergio Durigan Junior Jan. 31, 2018, 9:04 p.m. UTC
  Commit 5007d765ae09c10c7f3b18bb16841b9d2d59e181 ("Allow linking GDB
with ncursesw") modified our configure.ac and included the check for
"ncursesw" when searching for "waddstr".  However, there's one more
place where we should check for "ncursesw" first:

  AC_SEARCH_LIBS(tgetent, [termcap tinfo curses ncursesw ncurses])

This patch changes the order of the libraries to be searched when
looking for "tgetent", and puts "ncursesw" before "curses ...".

This is another patch we carry on Fedora GDB.

gdb/ChangeLog:
2018-01-31  Sergio Durigan Junior  <sergiodj@redhat.com>

	* configure.ac: Check for "ncursesw" first when searching for
	"tgetent".
	* configure: Regenerate.
---
 gdb/configure    | 2 +-
 gdb/configure.ac | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
  

Comments

Sergio Durigan Junior Feb. 10, 2018, 1:43 a.m. UTC | #1
On Wednesday, January 31 2018, I wrote:

> Commit 5007d765ae09c10c7f3b18bb16841b9d2d59e181 ("Allow linking GDB
> with ncursesw") modified our configure.ac and included the check for
> "ncursesw" when searching for "waddstr".  However, there's one more
> place where we should check for "ncursesw" first:
>
>   AC_SEARCH_LIBS(tgetent, [termcap tinfo curses ncursesw ncurses])
>
> This patch changes the order of the libraries to be searched when
> looking for "tgetent", and puts "ncursesw" before "curses ...".
>
> This is another patch we carry on Fedora GDB.

Ping.

> gdb/ChangeLog:
> 2018-01-31  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	* configure.ac: Check for "ncursesw" first when searching for
> 	"tgetent".
> 	* configure: Regenerate.
> ---
>  gdb/configure    | 2 +-
>  gdb/configure.ac | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gdb/configure b/gdb/configure
> index 81b35af521..c552c1ab1f 100755
> --- a/gdb/configure
> +++ b/gdb/configure
> @@ -8912,7 +8912,7 @@ return tgetent ();
>    return 0;
>  }
>  _ACEOF
> -for ac_lib in '' termcap tinfo curses ncursesw ncurses; do
> +for ac_lib in '' termcap tinfo ncursesw curses ncurses; do
>    if test -z "$ac_lib"; then
>      ac_res="none required"
>    else
> diff --git a/gdb/configure.ac b/gdb/configure.ac
> index 125e5f30e6..a73f72d0a8 100644
> --- a/gdb/configure.ac
> +++ b/gdb/configure.ac
> @@ -609,7 +609,7 @@ case $host_os in
>  esac
>  
>  # These are the libraries checked by Readline.
> -AC_SEARCH_LIBS(tgetent, [termcap tinfo curses ncursesw ncurses])
> +AC_SEARCH_LIBS(tgetent, [termcap tinfo ncursesw curses ncurses])
>  
>  if test "$ac_cv_search_tgetent" = no; then
>    CONFIG_OBS="$CONFIG_OBS stub-termcap.o"
> -- 
> 2.14.3
  
Simon Marchi Feb. 13, 2018, 4:58 a.m. UTC | #2
On 2018-01-31 04:04 PM, Sergio Durigan Junior wrote:
> Commit 5007d765ae09c10c7f3b18bb16841b9d2d59e181 ("Allow linking GDB
> with ncursesw") modified our configure.ac and included the check for
> "ncursesw" when searching for "waddstr".  However, there's one more
> place where we should check for "ncursesw" first:
> 
>   AC_SEARCH_LIBS(tgetent, [termcap tinfo curses ncursesw ncurses])
> 
> This patch changes the order of the libraries to be searched when
> looking for "tgetent", and puts "ncursesw" before "curses ...".
> 
> This is another patch we carry on Fedora GDB.

I think it makes sense, but can you expand on why this is needed?

Thanks,

Simon
  
Sergio Durigan Junior Feb. 13, 2018, 7:04 p.m. UTC | #3
On Monday, February 12 2018, Simon Marchi wrote:

> On 2018-01-31 04:04 PM, Sergio Durigan Junior wrote:
>> Commit 5007d765ae09c10c7f3b18bb16841b9d2d59e181 ("Allow linking GDB
>> with ncursesw") modified our configure.ac and included the check for
>> "ncursesw" when searching for "waddstr".  However, there's one more
>> place where we should check for "ncursesw" first:
>> 
>>   AC_SEARCH_LIBS(tgetent, [termcap tinfo curses ncursesw ncurses])
>> 
>> This patch changes the order of the libraries to be searched when
>> looking for "tgetent", and puts "ncursesw" before "curses ...".
>> 
>> This is another patch we carry on Fedora GDB.
>
> I think it makes sense, but can you expand on why this is needed?

The rationale for this patch was:

  https://bugzilla.redhat.com/show_bug.cgi?id=1270534

This is a bug that happened on Fedora GDB when linking against ncurses,
but not ncursesw.  The bug has been fixed upstream by commit
5007d765ae09c10c7f3b18bb16841b9d2d59e181, but the AC_SEARCH_LIBS line
for tgetent has not been modified, so, for the sake of completeness, I
think it makes sense to leave configure.ac in a consistent state (i.e.,
requiring ncursesw over ncurses whenever applicable).

I guess that's the gist of it.  There's not much that can be said, the
patch is really simple and its intention is to make things more uniform.

I can include the link to the Red Hat bug in the commit message, if you
want.

Thanks,
  
Sergio Durigan Junior Feb. 14, 2018, 1:28 a.m. UTC | #4
On Tuesday, February 13 2018, I wrote:

> On Monday, February 12 2018, Simon Marchi wrote:
>
>> On 2018-01-31 04:04 PM, Sergio Durigan Junior wrote:
>>> Commit 5007d765ae09c10c7f3b18bb16841b9d2d59e181 ("Allow linking GDB
>>> with ncursesw") modified our configure.ac and included the check for
>>> "ncursesw" when searching for "waddstr".  However, there's one more
>>> place where we should check for "ncursesw" first:
>>> 
>>>   AC_SEARCH_LIBS(tgetent, [termcap tinfo curses ncursesw ncurses])
>>> 
>>> This patch changes the order of the libraries to be searched when
>>> looking for "tgetent", and puts "ncursesw" before "curses ...".
>>> 
>>> This is another patch we carry on Fedora GDB.
>>
>> I think it makes sense, but can you expand on why this is needed?
>
> The rationale for this patch was:
>
>   https://bugzilla.redhat.com/show_bug.cgi?id=1270534
>
> This is a bug that happened on Fedora GDB when linking against ncurses,
> but not ncursesw.  The bug has been fixed upstream by commit
> 5007d765ae09c10c7f3b18bb16841b9d2d59e181, but the AC_SEARCH_LIBS line
> for tgetent has not been modified, so, for the sake of completeness, I
> think it makes sense to leave configure.ac in a consistent state (i.e.,
> requiring ncursesw over ncurses whenever applicable).
>
> I guess that's the gist of it.  There's not much that can be said, the
> patch is really simple and its intention is to make things more uniform.
>
> I can include the link to the Red Hat bug in the commit message, if you
> want.

After talking to Pedro in private, I was convinced that this is not the
right explanation for the patch, either.  Therefore, I will write a
better commit message and submit a v2, hopefully addressing your
concerns.

Thanks,
  

Patch

diff --git a/gdb/configure b/gdb/configure
index 81b35af521..c552c1ab1f 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -8912,7 +8912,7 @@  return tgetent ();
   return 0;
 }
 _ACEOF
-for ac_lib in '' termcap tinfo curses ncursesw ncurses; do
+for ac_lib in '' termcap tinfo ncursesw curses ncurses; do
   if test -z "$ac_lib"; then
     ac_res="none required"
   else
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 125e5f30e6..a73f72d0a8 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -609,7 +609,7 @@  case $host_os in
 esac
 
 # These are the libraries checked by Readline.
-AC_SEARCH_LIBS(tgetent, [termcap tinfo curses ncursesw ncurses])
+AC_SEARCH_LIBS(tgetent, [termcap tinfo ncursesw curses ncurses])
 
 if test "$ac_cv_search_tgetent" = no; then
   CONFIG_OBS="$CONFIG_OBS stub-termcap.o"