Re: [PATCH] [gdb/build] Check if libsource-highlight is usable

[Tom de Vries <tdevries@suse.de>]

On 11/19/21 4:16 PM, Lancelot SIX wrote:
>> diff --git a/gdb/configure.ac b/gdb/configure.ac
>> index d4cfb6a7624..e4fed3c575c 100644
>> --- a/gdb/configure.ac
>> +++ b/gdb/configure.ac
>> @@ -1248,11 +1248,48 @@ either use --disable-source-highlight or dnl
>>            ;;
>>        esac
>>  
>> -      SRCHIGH_CFLAGS=`${pkg_config_prog_path} --cflags source-highlight`
>> -      SRCHIGH_LIBS=`${pkg_config_prog_path} --libs source-highlight`
>> -      AC_DEFINE([HAVE_SOURCE_HIGHLIGHT], 1,
>> -                [Define to 1 if the source-highlight library is available])
>> -      AC_MSG_RESULT([yes])
>> +      srchigh_pkg_cflags=`${pkg_config_prog_path} --cflags source-highlight`
>> +      srchigh_pkg_libs=`${pkg_config_prog_path} --libs source-highlight`
>> +
>> +      # Now that we have found a source-highlight library, check if we can use
>> +      # it.  In particular, we're trying to detect the situation that the
>> +      # library is using the new libstdc++ library abi ( see
>> +      # https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_dual_abi.html )
>> +      # while the compiler being used to compile gdb is using the old abi.
>> +      # Such a situation will result in an undefined reference to
>> +      # srchilite::SourceHighlight::SourceHighlight(std::string const&).
>> +      # This situation can occur when f.i. using a source highlight library
>> +      # compiled with g++ 7.5.0 while building gdb with g++ 4.8.5.
>> +      AC_LANG_PUSH(C++)
>> +      save_CFLAGS=$CFLAGS
>> +      save_LDFLAGS=$LDFLAGS
>> +      CFLAGS="$CFLAGS $srchigh_pkg_cflags"
>> +      LDFLAGS="$LDFLAGS $srchigh_pkg_libs"
>> +      AC_LINK_IFELSE(
>> +        [AC_LANG_PROGRAM(
>> +          [#include <srchilite/sourcehighlight.h>],
>> +          [std::string outlang = "esc.outlang";
>> +           new srchilite::SourceHighlight (outlang);]
>> +        )],
>> +        [have_usable_source_highlight=yes],
>> +        [have_usable_source_highlight=no]
>> +      )
>> +      CFLAGS="$SAVE_CFLAGS"
>> +      LDFLAGS="$SAVE_LDFLAGS"
>> +      AC_LANG_POP(C++)
>> +
>> +      if test "${have_usable_source_highlight}" = "yes"; then
>> +        AC_DEFINE([HAVE_SOURCE_HIGHLIGHT], 1,
>> +                  [Define to 1 if the source-highlight library is available])
>> +        AC_MSG_RESULT([yes])
>> +        SRCHIGH_CFLAGS="$srchigh_pkg_cflags"
>> +        SRCHIGH_LIBS="$srchigh_pkg_libs"
>> +      else
>> +	AC_MSG_RESULT([no])
>> +	if test "${enable_source_highlight}" = "yes"; then
> 
> Hi,
> 
> I do not know what it the indentation policy for the configure.ac file,

Me neither, I just used what I saw around.

> but you use tabs in the 'AC_MSG_RESULT' and 'if test' lines above (2
> lines above), and nowhere else.
> 

Ack, fixed now, I guess I forgot to untabify those.  Thanks for noticing.

> The rest of the file does not seem to be completely consistent regarding
> this particular subject, so I do not know if there is a preferred
> formatting that should be used, but I guess that having the same across
> the patch could make sense.
> 

Agreed.

> Otherwise, and for what it is worth, this looks reasonable to me.
> 

Thanks for the review,
- Tom

> Best,
> Lancelot.
> 
>> +          AC_MSG_ERROR([source-highlight in your system could not be used])
>> +        fi
>> +      fi 
>>      else
>>        AC_MSG_RESULT([no])
>>        if test "${enable_source_highlight}" = "yes"; then
>>
>> base-commit: fd0ff19bf435b267caae6a1ae04e7b4a4ba64f5b
>> -- 
>> 2.26.2
>>