[1/3] GDB: Run `pkg-config' with `--static' to pull libguile dependencies

Message ID alpine.DEB.2.20.2211241907010.19931@tpp.orcam.me.uk
State New
Headers
Series GDB: Fix `pkg-config' issues with configuring for Guile |

Commit Message

Maciej W. Rozycki Nov. 24, 2022, 11:51 p.m. UTC
  Fix a configuration error:

checking whether to use guile... guile-2.2
checking for pkg-config... /usr/bin/pkg-config
checking for usable guile from /usr/bin/pkg-config... checking for scm_set_automatic_finalization_enabled... no
configure: error: in `.../gdb':
configure: error: linking guile version guile-2.2 test program failed
See `config.log' for more details
make[1]: *** [Makefile:12160: configure-gdb] Error 1

coming from link errors as recorded in `config.log' referred:

configure:19349: checking for scm_set_automatic_finalization_enabled
configure:19349: gcc -o conftest -pipe -O2      -pthread -I.../include/guile/2.2   conftest.c -lncursesw -lm -ldl  -L.../lib -lguile-2.2 -lgc >&5
/usr/bin/ld: .../lib/libguile-2.2.a(libguile_2.2_la-ports.o): in function `scm_ungetc':
(.text+0x5561): undefined reference to `u32_conv_to_encoding'
/usr/bin/ld: (.text+0x56e1): undefined reference to `u32_to_u8'

etc., etc., in a valid configuration where a static version of libguile 
has only been installed, which is a pefectly valid system configuration.  
This is due to symbols being required from libguile's dependencies that 
have not been included in the linker invocation.

In configurations using the ELF format dynamic libguile implicitly pulls 
indirect dependencies in the link, but to satisfy static libguile they 
need to be named explicitly.  These dependencies have been recorded and 
can be supplied by `pkg-config', but for that to happen the tool has to 
be invoked with the `--static' option in addition to `--libs'.  Moreover 
it is recommended, at least with systems using the ELF format, to have 
indirect dependencies included with static linker invocation even where 
they all are satisfied by dynamic libraries.

Therefore fix the issue by using the `--static' option unconditionally 
with `pkg-config', adding the dependencies required:

configure:19349: gcc -o conftest -pipe -O2      -pthread -I.../include/guile/2.2   conftest.c -lncursesw -lm -ldl  -L.../lib -lguile-2.2 -lgc -lgmp -lltdl -lffi -lunistring -lcrypt -lm >&5

and removing the errors quoted above.
---
 gdb/configure    |   10 +++++-----
 gdb/configure.ac |    6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

gdb-pkgconfig-static.diff
  

Comments

Eli Zaretskii Nov. 25, 2022, 7:56 a.m. UTC | #1
> Date: Thu, 24 Nov 2022 23:51:09 +0000 (GMT)
> From: "Maciej W. Rozycki" <macro@embecosm.com>
> 
> Fix a configuration error:
> 
> checking whether to use guile... guile-2.2
> checking for pkg-config... /usr/bin/pkg-config
> checking for usable guile from /usr/bin/pkg-config... checking for scm_set_automatic_finalization_enabled... no
> configure: error: in `.../gdb':
> configure: error: linking guile version guile-2.2 test program failed
> See `config.log' for more details
> make[1]: *** [Makefile:12160: configure-gdb] Error 1
> 
> coming from link errors as recorded in `config.log' referred:
> 
> configure:19349: checking for scm_set_automatic_finalization_enabled
> configure:19349: gcc -o conftest -pipe -O2      -pthread -I.../include/guile/2.2   conftest.c -lncursesw -lm -ldl  -L.../lib -lguile-2.2 -lgc >&5
> /usr/bin/ld: .../lib/libguile-2.2.a(libguile_2.2_la-ports.o): in function `scm_ungetc':
> (.text+0x5561): undefined reference to `u32_conv_to_encoding'
> /usr/bin/ld: (.text+0x56e1): undefined reference to `u32_to_u8'
> 
> etc., etc., in a valid configuration where a static version of libguile 
> has only been installed, which is a pefectly valid system configuration.  
> This is due to symbols being required from libguile's dependencies that 
> have not been included in the linker invocation.
> 
> In configurations using the ELF format dynamic libguile implicitly pulls 
> indirect dependencies in the link, but to satisfy static libguile they 
> need to be named explicitly.  These dependencies have been recorded and 
> can be supplied by `pkg-config', but for that to happen the tool has to 
> be invoked with the `--static' option in addition to `--libs'.  Moreover 
> it is recommended, at least with systems using the ELF format, to have 
> indirect dependencies included with static linker invocation even where 
> they all are satisfied by dynamic libraries.

If only a static version of the Guile library is installed, how come
"pkg-config --libs" doesn't produce the list of all the dependency libraries
for static linking?  Isn't that a bug in Guile installation on that system?

> Therefore fix the issue by using the `--static' option unconditionally 
> with `pkg-config', adding the dependencies required:

I don't think I've every seen a configure script that explicitly includes
the --static switch to pkg-config.  Did you?  If not, how do they deal with
this issue?  Almost any external library with which GDB is linked can be
installed as a static-only library, so do we need to use --static in all
pkg-config tests?  I see that your proposed patch only changes the tests for
Guile and for source-highlight libraries.

Thanks.
  
Maciej W. Rozycki Nov. 28, 2022, 12:59 p.m. UTC | #2
On Fri, 25 Nov 2022, Eli Zaretskii wrote:

> > In configurations using the ELF format dynamic libguile implicitly pulls 
> > indirect dependencies in the link, but to satisfy static libguile they 
> > need to be named explicitly.  These dependencies have been recorded and 
> > can be supplied by `pkg-config', but for that to happen the tool has to 
> > be invoked with the `--static' option in addition to `--libs'.  Moreover 
> > it is recommended, at least with systems using the ELF format, to have 
> > indirect dependencies included with static linker invocation even where 
> > they all are satisfied by dynamic libraries.
> 
> If only a static version of the Guile library is installed, how come
> "pkg-config --libs" doesn't produce the list of all the dependency libraries
> for static linking?  Isn't that a bug in Guile installation on that system?

 If anything, it's a bug in Guile itself, or more likely a design issue 
with pkg-config.  Looking at its .pc template I can see no provision for a 
different library arrangement depending on whether `--enable-shared' or 
`--disable-shared' is in effect with `configure':

Libs: -L${libdir} -lguile-@GUILE_EFFECTIVE_VERSION@ @BDW_GC_LIBS@
Libs.private: @LIB_CLOCK_GETTIME@ @LIBGMP@ @LIBLTDL@ @LIBFFI_LIBS@	\
  @LIBUNISTRING@ @GUILE_LIBS@ @LIBICONV@ @LIBINTL@ @LIBSOCKET@		\
  @SERVENT_LIB@ @HOSTENT_LIB@ @GETADDRINFO_LIB@ @INET_NTOP_LIB@		\
  @INET_PTON_LIB@

 I have skimmed over the very few .pc templates I could find around on my 
system and they all seem to do essentially the same.  For example have a 
look at zlib/contrib/minizip/minizip.pc.in in our very own tree.  While 
not used by us it's taken verbatim from the upstream zlib distribution and 
supplied with libminizip, which has a dependency on libz.

 But the template has just:

Libs: -L${libdir} -lminizip
Libs.private: -lz

i.e. it's all fixed with no `configure' processing whatsoever and there is 
no provision for pulling `-lz' unless `--static' has been explicitly given 
to `pkg-config'.

 But any such provision looks to me like a lot of effort for software 
writers, to take away which is the very purpose of pkg-config.  So what I 
think pkg-config should do is to provide a flag for the .pc file to be set 
by `configure' or otherwise, which would tell pkg-config that said package 
has been built with no shared libraries available, via `--disable-shared' 
or an equivalent.

 In such a case pkg-config would always include Libs.private along with 
Libs when requested by `--libs', even if no `--static' option has been 
given.  Which leads me to a conclusion it's an oversight in the pkg-config 
design that it does not provide for the static-only link scenario in a 
reasonable way.

> > Therefore fix the issue by using the `--static' option unconditionally 
> > with `pkg-config', adding the dependencies required:
> 
> I don't think I've every seen a configure script that explicitly includes
> the --static switch to pkg-config.  Did you?  If not, how do they deal with
> this issue?  Almost any external library with which GDB is linked can be
> installed as a static-only library, so do we need to use --static in all
> pkg-config tests?  I see that your proposed patch only changes the tests for
> Guile and for source-highlight libraries.

 It seems to me that distributions universally build libraries as shared 
and do not provide for installing static libraries without their shared 
counterparts.  And that people hardly ever build from sources nowadays.

 So it looks to me like this issue is not dealt with at all, and while we 
might try to fix the world outside, it could be a futile effort.  And in 
any case it'd be long until any solution lands downstream.  Then as I say 
indirect dependencies are best included in ELF links involving dynamic 
shared objects anyway, so I think using `--static' is in my opinion a 
reasonable solution.  I think it's also needed for the `CFLAGS=-static' 
case too.  Cf. <https://bugs.freedesktop.org/show_bug.cgi?id=19541>.

 I haven't looked at pieces coming from outside gdb/, and I can see now 
that the debuginfod library isn't handled at this point (I don't know if 
it has any indirect dependencies).  It's use here is a strong argument in 
favour of 3/3, so that we don't use $pkg_config and $PKG_CONFIG variables 
both at a time for the same thing in a single script (what a mess!).

 Do you find my observations a sufficient justification for my proposed 
change now?  Thank you for your insights!

  Maciej
  
Eli Zaretskii Nov. 28, 2022, 1:35 p.m. UTC | #3
> Date: Mon, 28 Nov 2022 12:59:52 +0000 (GMT)
> From: "Maciej W. Rozycki" <macro@embecosm.com>
> cc: gdb-patches@sourceware.org
> 
> > If only a static version of the Guile library is installed, how come
> > "pkg-config --libs" doesn't produce the list of all the dependency libraries
> > for static linking?  Isn't that a bug in Guile installation on that system?
> 
>  If anything, it's a bug in Guile itself

Well, "bug in Guile installation" and "bug in Guile itself" are almost
synonymous...

> or more likely a design issue with pkg-config.

That I don't think.

Basically, I think a static-library only installation should have the same
list of libraries in Libs and in Libs.private.  And this should be produced
by the build when the user selects --disable-shared.

>  But any such provision looks to me like a lot of effort for software 
> writers, to take away which is the very purpose of pkg-config.  So what I 
> think pkg-config should do is to provide a flag for the .pc file to be set 
> by `configure' or otherwise, which would tell pkg-config that said package 
> has been built with no shared libraries available, via `--disable-shared' 
> or an equivalent.

That "flag" is the value of Libs, AFAIU.

>  In such a case pkg-config would always include Libs.private along with 
> Libs when requested by `--libs', even if no `--static' option has been 
> given.  Which leads me to a conclusion it's an oversight in the pkg-config 
> design that it does not provide for the static-only link scenario in a 
> reasonable way.

I'm not sure it's an oversight in the design of pkg-config, but I'm not an
expert on that enough to be sure.

> > > Therefore fix the issue by using the `--static' option unconditionally 
> > > with `pkg-config', adding the dependencies required:
> > 
> > I don't think I've every seen a configure script that explicitly includes
> > the --static switch to pkg-config.  Did you?  If not, how do they deal with
> > this issue?  Almost any external library with which GDB is linked can be
> > installed as a static-only library, so do we need to use --static in all
> > pkg-config tests?  I see that your proposed patch only changes the tests for
> > Guile and for source-highlight libraries.
> 
>  It seems to me that distributions universally build libraries as shared 
> and do not provide for installing static libraries without their shared 
> counterparts.  And that people hardly ever build from sources nowadays.

If that is what happens, maybe we shouldn't support this use case so easily
and OOTB?

>  So it looks to me like this issue is not dealt with at all, and while we 
> might try to fix the world outside, it could be a futile effort.  And in 
> any case it'd be long until any solution lands downstream.  Then as I say 
> indirect dependencies are best included in ELF links involving dynamic 
> shared objects anyway, so I think using `--static' is in my opinion a 
> reasonable solution.  I think it's also needed for the `CFLAGS=-static' 
> case too.  Cf. <https://bugs.freedesktop.org/show_bug.cgi?id=19541>.
> 
>  I haven't looked at pieces coming from outside gdb/, and I can see now 
> that the debuginfod library isn't handled at this point (I don't know if 
> it has any indirect dependencies).  It's use here is a strong argument in 
> favour of 3/3, so that we don't use $pkg_config and $PKG_CONFIG variables 
> both at a time for the same thing in a single script (what a mess!).
> 
>  Do you find my observations a sufficient justification for my proposed 
> change now?  Thank you for your insights!

I'm not opposed to installing your changes, as they cannot do any harm,
AFAICT.  maybe just add enough commentary about this unusual practice to
explain why we do it.

Thanks.
  

Patch

Index: src/gdb/configure
===================================================================
--- src.orig/gdb/configure
+++ src/gdb/configure
@@ -22917,7 +22917,7 @@  $as_echo_n "checking for usable guile fr
     if test $? != 0; then
       as_fn_error $? "failure running pkg-config --cflags ${guile_version}" "$LINENO" 5
     fi
-    new_LIBS=`${pkg_config} --libs ${guile_version}`
+    new_LIBS=`${pkg_config} --static --libs ${guile_version}`
     if test $? != 0; then
       as_fn_error $? "failure running pkg-config --libs ${guile_version}" "$LINENO" 5
     fi
@@ -23001,7 +23001,7 @@  $as_echo_n "checking for usable guile fr
     if test $? != 0; then
       as_fn_error $? "failure running pkg-config --cflags ${guile_version}" "$LINENO" 5
     fi
-    new_LIBS=`${pkg_config} --libs ${guile_version}`
+    new_LIBS=`${pkg_config} --static --libs ${guile_version}`
     if test $? != 0; then
       as_fn_error $? "failure running pkg-config --libs ${guile_version}" "$LINENO" 5
     fi
@@ -23082,7 +23082,7 @@  $as_echo_n "checking for usable guile fr
     if test $? != 0; then
       as_fn_error $? "failure running pkg-config --cflags ${guile_version}" "$LINENO" 5
     fi
-    new_LIBS=`${pkg_config} --libs ${guile_version}`
+    new_LIBS=`${pkg_config} --static --libs ${guile_version}`
     if test $? != 0; then
       as_fn_error $? "failure running pkg-config --libs ${guile_version}" "$LINENO" 5
     fi
@@ -23173,7 +23173,7 @@  $as_echo_n "checking for usable guile fr
     if test $? != 0; then
       as_fn_error $? "failure running pkg-config --cflags ${guile_version}" "$LINENO" 5
     fi
-    new_LIBS=`${pkg_config} --libs ${guile_version}`
+    new_LIBS=`${pkg_config} --static --libs ${guile_version}`
     if test $? != 0; then
       as_fn_error $? "failure running pkg-config --libs ${guile_version}" "$LINENO" 5
     fi
@@ -23418,7 +23418,7 @@  $as_echo "no - pkg-config not found" >&6
       esac
 
       srchigh_pkg_cflags=`${pkg_config_prog_path} --cflags source-highlight`
-      srchigh_pkg_libs=`${pkg_config_prog_path} --libs source-highlight`
+      srchigh_pkg_libs=`${pkg_config_prog_path} --static --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
Index: src/gdb/configure.ac
===================================================================
--- src.orig/gdb/configure.ac
+++ src/gdb/configure.ac
@@ -1020,7 +1020,7 @@  AC_DEFUN([AC_TRY_LIBGUILE],
     if test $? != 0; then
       AC_MSG_ERROR([failure running pkg-config --cflags ${guile_version}])
     fi
-    new_LIBS=`${pkg_config} --libs ${guile_version}`
+    new_LIBS=`${pkg_config} --static --libs ${guile_version}`
     if test $? != 0; then
       AC_MSG_ERROR([failure running pkg-config --libs ${guile_version}])
     fi
@@ -1082,7 +1082,7 @@  dnl        NOTE: This needn't be the "re
 dnl        It could be a shell script.  It is invoked as:
 dnl        pkg-config --exists $version
 dnl        pkg-config --cflags $version
-dnl        pkg-config --libs $version
+dnl        pkg-config --static --libs $version
 dnl        pkg-config --variable guild $version
 dnl        The script will be called with $version having each value in
 dnl        $try_guile_versions until --exists indicates success.
@@ -1212,7 +1212,7 @@  either use --disable-source-highlight or
       esac
 
       srchigh_pkg_cflags=`${pkg_config_prog_path} --cflags source-highlight`
-      srchigh_pkg_libs=`${pkg_config_prog_path} --libs source-highlight`
+      srchigh_pkg_libs=`${pkg_config_prog_path} --static --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