[PATCH] Warn about static libs vs. source-highlight only when necessary

[PATCH] Warn about static libs vs. source-highlight only when necessary

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 469 bytes --]

Hi,

this is a small improvement regarding the error message when static system libraries are
used, but source-highlight does not support that.

I have however right now a question about our use of pkg-config.

That is: what happens if I build gcc for build != host ?
will pkg-config then find the build libraries, and we use them
for host, where they might be unavailable?
That is probably bound to fail, isn't it?

Anyway, is this change OK for trunk?

Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Warn-about-static-libs-vs.-source-highlight-only-whe.patch --]
[-- Type: text/x-patch; name="0001-Warn-about-static-libs-vs.-source-highlight-only-whe.patch", Size: 2644 bytes --]

From 4086ea744e5eca51db3298a73684045b08e0b588 Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Wed, 16 Dec 2020 19:47:18 +0100
Subject: [PATCH] Warn about static libs vs. source-highlight only when
 necessary

This avoids the error message when pkg-config is available but
source-highlight is not installed.

2020-12-16  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* configure.ac: Move the static libs vs. source-highlight
	error message to a better place.
	* configure: Regenerate.
---
 gdb/configure    | 12 ++++++------
 gdb/configure.ac | 12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/gdb/configure b/gdb/configure
index 2bf8014..5819002 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -12083,13 +12083,13 @@ $as_echo "no - pkg-config not found" >&6; }
       as_fn_error $? "pkg-config was not found in your system" "$LINENO" 5
     fi
   else
-    case "$LDFLAGS" in
-      *static-libstdc*)
-        as_fn_error $? "source highlight is incompatible with -static-libstdc++; either use --disable-source-highlight or --without-static-standard-libraries" "$LINENO" 5
-        ;;
-    esac
-
     if ${pkg_config_prog_path} --exists source-highlight; then
+      case "$LDFLAGS" in
+        *static-libstdc*)
+          as_fn_error $? "source highlight is incompatible with -static-libstdc++; either use --disable-source-highlight or --without-static-standard-libraries" "$LINENO" 5
+          ;;
+      esac
+
       SRCHIGH_CFLAGS=`${pkg_config_prog_path} --cflags source-highlight`
       SRCHIGH_LIBS=`${pkg_config_prog_path} --libs source-highlight`
 
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 376643c..f1c233c 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1248,15 +1248,15 @@ if test "${enable_source_highlight}" != "no"; then
       AC_MSG_ERROR([pkg-config was not found in your system])
     fi
   else
-    case "$LDFLAGS" in
-      *static-libstdc*)
-        AC_MSG_ERROR([source highlight is incompatible with -static-libstdc++; dnl
+    if ${pkg_config_prog_path} --exists source-highlight; then
+      case "$LDFLAGS" in
+        *static-libstdc*)
+          AC_MSG_ERROR([source highlight is incompatible with -static-libstdc++; dnl
 either use --disable-source-highlight or dnl
 --without-static-standard-libraries])
-        ;;
-    esac
+          ;;
+      esac
 
-    if ${pkg_config_prog_path} --exists source-highlight; then
       SRCHIGH_CFLAGS=`${pkg_config_prog_path} --cflags source-highlight`
       SRCHIGH_LIBS=`${pkg_config_prog_path} --libs source-highlight`
       AC_DEFINE([HAVE_SOURCE_HIGHLIGHT], 1,
-- 
1.9.1

[PATCHv2] Warn about static libs vs. source-highlight only when necessary

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 819 bytes --]

On 12/16/20 8:57 PM, Bernd Edlinger wrote:
> Hi
> 
> this is a small improvement regarding the error message when static system libraries are
> used, but source-highlight does not support that.
> 
> I have however right now a question about our use of pkg-config.
> 
> That is: what happens if I build gcc for build != host ?
> will pkg-config then find the build libraries, and we use them
> for host, where they might be unavailable?
> That is probably bound to fail, isn't it?
> 

I think if gdb is configured for a host != build machine,
the pkg-config is not really useful.
If it finds some libraies, they will be for the build machine
not hot the host.  So they can only cause problems.

So I added a check for that and set the pkg-config path
to "missing" if build != host.


Is it OK for trunk?


Thanks
Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Warn-about-static-libs-vs.-source-highlight-only-whe.patch --]
[-- Type: text/x-patch; name="0001-Warn-about-static-libs-vs.-source-highlight-only-whe.patch", Size: 3437 bytes --]

From aa09937ac0b977a25eea440c4585d0b5a8177784 Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Wed, 16 Dec 2020 19:47:18 +0100
Subject: [PATCH] Warn about static libs vs. source-highlight only when
 necessary

This avoids the error message when pkg-config is available but
source-highlight is not installed.

Also for cross-builds with build != host, don't use pkg-config
at all, as it always gives us libraries for the build machine.

2020-12-16  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* configure.ac: Move the static libs vs. source-highlight
	error message to a better place.
	Don't use pkg-config for cross-builds.
	* configure: Regenerate.
---
 gdb/configure    | 17 +++++++++++------
 gdb/configure.ac | 17 +++++++++++------
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/gdb/configure b/gdb/configure
index 2bf8014..cd9b6e3 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -11565,6 +11565,11 @@ fi
 
 
 
+# don't trust pkg-config for cross-builds
+if test "${build}" != "${host}"; then
+  pkg_config_prog_path="missing"
+fi
+
 try_guile_versions="guile-3.0 guile-2.2 guile-2.0"
 have_libguile=no
 case "${with_guile}" in
@@ -12083,13 +12088,13 @@ $as_echo "no - pkg-config not found" >&6; }
       as_fn_error $? "pkg-config was not found in your system" "$LINENO" 5
     fi
   else
-    case "$LDFLAGS" in
-      *static-libstdc*)
-        as_fn_error $? "source highlight is incompatible with -static-libstdc++; either use --disable-source-highlight or --without-static-standard-libraries" "$LINENO" 5
-        ;;
-    esac
-
     if ${pkg_config_prog_path} --exists source-highlight; then
+      case "$LDFLAGS" in
+        *static-libstdc*)
+          as_fn_error $? "source highlight is incompatible with -static-libstdc++; either use --disable-source-highlight or --without-static-standard-libraries" "$LINENO" 5
+          ;;
+      esac
+
       SRCHIGH_CFLAGS=`${pkg_config_prog_path} --cflags source-highlight`
       SRCHIGH_LIBS=`${pkg_config_prog_path} --libs source-highlight`
 
diff --git a/gdb/configure.ac b/gdb/configure.ac
index 376643c..e18555d 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1138,6 +1138,11 @@ AC_MSG_RESULT([$with_guile])
 dnl We check guile with pkg-config.
 AC_PATH_PROG(pkg_config_prog_path, pkg-config, missing)
 
+# don't trust pkg-config for cross-builds
+if test "${build}" != "${host}"; then
+  pkg_config_prog_path="missing"
+fi
+
 try_guile_versions="guile-3.0 guile-2.2 guile-2.0"
 have_libguile=no
 case "${with_guile}" in
@@ -1248,15 +1253,15 @@ if test "${enable_source_highlight}" != "no"; then
       AC_MSG_ERROR([pkg-config was not found in your system])
     fi
   else
-    case "$LDFLAGS" in
-      *static-libstdc*)
-        AC_MSG_ERROR([source highlight is incompatible with -static-libstdc++; dnl
+    if ${pkg_config_prog_path} --exists source-highlight; then
+      case "$LDFLAGS" in
+        *static-libstdc*)
+          AC_MSG_ERROR([source highlight is incompatible with -static-libstdc++; dnl
 either use --disable-source-highlight or dnl
 --without-static-standard-libraries])
-        ;;
-    esac
+          ;;
+      esac
 
-    if ${pkg_config_prog_path} --exists source-highlight; then
       SRCHIGH_CFLAGS=`${pkg_config_prog_path} --cflags source-highlight`
       SRCHIGH_LIBS=`${pkg_config_prog_path} --libs source-highlight`
       AC_DEFINE([HAVE_SOURCE_HIGHLIGHT], 1,
-- 
1.9.1

Re: [PATCHv2] Warn about static libs vs. source-highlight only when necessary

[Simon Marchi]

On 2020-12-17 4:07 p.m., Bernd Edlinger wrote:
> On 12/16/20 8:57 PM, Bernd Edlinger wrote:
>> Hi
>>
>> this is a small improvement regarding the error message when static system libraries are
>> used, but source-highlight does not support that.
>>
>> I have however right now a question about our use of pkg-config.
>>
>> That is: what happens if I build gcc for build != host ?
>> will pkg-config then find the build libraries, and we use them
>> for host, where they might be unavailable?
>> That is probably bound to fail, isn't it?
>>
> 
> I think if gdb is configured for a host != build machine,
> the pkg-config is not really useful.
> If it finds some libraies, they will be for the build machine
> not hot the host.  So they can only cause problems.

So, basically, when you cross-compile GDB.  pkg-config is useful,
but you must configure pkg-config to use the correct sysroot.
Otherwise, as you say, it will return paths and all for the build
system.  So you must set PKG_CONFIG_SYSROOT_DIR and perhaps
PKG_CONFIG_PATH and PKG_CONFIG_LIBDIR, I don't recall which one
off the top of my head, check the man page and:

https://autotools.io/pkgconfig/cross-compiling.html

Some cross-compilation toolchains (like the ones that Buildroot
and Yocto generate, IIRC), provide a pkg-config for the target.

So I wouldn't approve this change:

+# don't trust pkg-config for cross-builds
+if test "${build}" != "${host}"; then
+  pkg_config_prog_path="missing"
+fi
+

as it would make using pkg-config for cross-compilation impossible.

The other part of the change makes sense I think.  IIUC, it's for
when you use --with-static-standard-libraries but don't have source
highlight installed.  In that case there's no problem, so we shouldn't
complain that source highlight is not compatible with static standard
libraries.  We should wait for pkg-config to tell us that source
highlight was found for that.  That sounds right?

Simon

Re: [PATCHv2] Warn about static libs vs. source-highlight only when necessary

[Bernd Edlinger]

On 12/18/20 11:00 PM, Simon Marchi wrote:
> 
> 
> On 2020-12-17 4:07 p.m., Bernd Edlinger wrote:
>> On 12/16/20 8:57 PM, Bernd Edlinger wrote:
>>> Hi
>>>
>>> this is a small improvement regarding the error message when static system libraries are
>>> used, but source-highlight does not support that.
>>>
>>> I have however right now a question about our use of pkg-config.
>>>
>>> That is: what happens if I build gcc for build != host ?
>>> will pkg-config then find the build libraries, and we use them
>>> for host, where they might be unavailable?
>>> That is probably bound to fail, isn't it?
>>>
>>
>> I think if gdb is configured for a host != build machine,
>> the pkg-config is not really useful.
>> If it finds some libraies, they will be for the build machine
>> not hot the host.  So they can only cause problems.
> 
> So, basically, when you cross-compile GDB.  pkg-config is useful,
> but you must configure pkg-config to use the correct sysroot.
> Otherwise, as you say, it will return paths and all for the build
> system.  So you must set PKG_CONFIG_SYSROOT_DIR and perhaps
> PKG_CONFIG_PATH and PKG_CONFIG_LIBDIR, I don't recall which one
> off the top of my head, check the man page and:
> 
> https://autotools.io/pkgconfig/cross-compiling.html
> 
> Some cross-compilation toolchains (like the ones that Buildroot
> and Yocto generate, IIRC), provide a pkg-config for the target.
> 
> So I wouldn't approve this change:
> 
> +# don't trust pkg-config for cross-builds
> +if test "${build}" != "${host}"; then
> +  pkg_config_prog_path="missing"
> +fi
> +
> 
> as it would make using pkg-config for cross-compilation impossible.
> 
> The other part of the change makes sense I think.  IIUC, it's for
> when you use --with-static-standard-libraries but don't have source
> highlight installed.  In that case there's no problem, so we shouldn't
> complain that source highlight is not compatible with static standard
> libraries.  We should wait for pkg-config to tell us that source
> highlight was found for that.  That sounds right?
> 

Okay, then I'll commit the previous version.


Thanks
Bernd.

> Simon
>