Re: [PATCH] Revert "Pass GUILE down to subdirectories"

[Andrew Burgess <aburgess@redhat.com>]

Tom Tromey <tom@tromey.com> writes:

> This reverts commit b7e5a29602143b53267efcd9c8d5ecc78cd5a62f.
>
> This patch caused problems for some users when building gdb, because
> it would cause 'guild' to be invoked with the wrong versin of guile.
> On the whole it seems simpler to just back this out.

Tom,

After once again forgetting to add GUILE=guile2.2 to my GDB build I was
thinking about this issue again.

Given that GDB has a --with-guile=... configure option, and that our
configure scripts try to identify a matching version of guild and a
shared library to link GDB against, I wondered why we don't just force
the use of a matching version of guile.

I guess I'm suggesting that for building GDB's guile components we
should respect either the --with-guile=... configure option, or what the
configure script auto-detected, and should not be picking up any build
time GUILE=... flag -- setting GUILE=... isn't going to change the
version of guild used, nor is it going to change the shared library that
GDB links against, so just changing the guile version seems like a
recipe for incompatibility problems.

Below is a patch that forces GDB to compile our guile scripts using a
suitable version of guile.  This could be applied irrespective of
whether you revert b7e5a29602143b53267efcd9c8d5ecc78cd5a62f or not.

What do you think?

Thanks,
Andrew

---

commit 6d326010217a2c94ad27d4063f90f689f7c11615
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Tue Mar 5 12:35:05 2024 +0000

    gdb/data-directory: set GUILE for compiling GDB's guile scripts
    
    After this commit:
    
      commit d006ec41c448d9f4a84df50391e87cbf0aa8c152
      Date:   Thu Dec 28 14:08:39 2023 -0700
    
          Pass GUILE down to subdirectories
    
    I have had issues with GDB's guile support.  Commit d006ec41c448 sets
    the GUILE variable in the top-level Makefile to the default value
    'guile'.
    
    The problem this causes for me is that 'guile' on my system is
    2.0.14.  I also have guile2.2 available on my system, and this is
    version 2.2.6.
    
    When GDB configures it selects looks for a suitable guile version to
    use, and it looks in the order:
    
      guile-3.0
      guile-2.2
      guile-2.0
    
    This means that on my system GDB chooses to use the guile2.2 library
    to link against, and uses guild2.2 as the compiler to pre-compile
    GDB's guile scripts.
    
    However, guild (and guile2.2) check for the GUILE environment
    variable, and use the value of this variable as the version of guile
    interpreter which is used when compiling things.
    
    What this means is that after commit d006ec41c448, I am now using the
    guild2.2 binary (version 2.2.6), but this is then invoking the guile
    binary (version 2.0.14).  The compiled scripts are then loaded by GDB,
    which is linking against libguile-2.2.so.1, this results in the
    following error:
    
      $ ./gdb/gdb --data-directory ./gdb/data-directory
      Exception caught while booting Guile.
      Error in function "load-thunk-from-memory":
      not an ELF file
      ./gdb/gdb: warning: Could not complete Guile gdb module initialization from:
      /tmp/binutils-gdb/build/gdb/data-directory/guile/gdb/boot.scm.
      Limited Guile support is available.
      Suggest passing --data-directory=/path/to/gdb/data-directory.
      GDB Version: 15.1
    
      (gdb)
    
    We could fix this by reverting some parts of d006ec41c448,
    specifically, the part which sets 'guile' as the default value of
    GUILE in the top-level Makefile.  However, I wonder if there's a
    better solution.
    
    GDB already has a configure flag `--with-guile=...` if this isn't
    provided then GDB looks for a sensible default.
    
    I think that when compiling GDB's guile files we should use a
    consistent set of tools, i.e. a matching version of guild and guile,
    both of which correspond to the shared library GDB will link against.
    
    I propose that this should be done by setting the GUILE environment
    variable in gdb/data-directory/Makefile.in based on the version of
    guile that GDB's configure script selected.
    
    This does mean that doing:
    
      make all-gdb GUILE=....
    
    will no longer work, GDB will ignore this setting, however, if we
    really want to compile GDB's guile scripts using a different guile
    version then surely we should also be linking to a corresponding
    shared library, and (maybe) using the corresponding guild version too?
    This can be achieved by configuring GDB using `--with-guile=....`.
    
    After this commit I no longer have issues compiling GDB with guile
    support.

diff --git a/gdb/acinclude.m4 b/gdb/acinclude.m4
index 915082056dd..6e4b96f0acf 100644
--- a/gdb/acinclude.m4
+++ b/gdb/acinclude.m4
@@ -284,7 +284,29 @@ AC_DEFUN([GDB_GUILE_PROGRAM_NAMES], [
   fi
 
   GUILD="$ac_cv_guild_program_name"
+
+  AC_CACHE_CHECK([for the absolute file name of the 'guile' command],
+    [ac_cv_guile_program_name],
+    [ac_cv_guile_program_name="`$1 --variable guile $2`"
+
+     # In Guile up to 2.0.11 included, guile-2.0.pc would not define
+     # the 'guile' and 'bindir' variables.  In that case, try to guess
+     # what the program name is, at the risk of getting it wrong if
+     # Guile was configured with '--program-suffix' or similar.
+     if test "x$ac_cv_guile_program_name" = "x"; then
+       guile_exec_prefix="`$1 --variable exec_prefix $2`"
+       ac_cv_guile_program_name="$guile_exec_prefix/bin/guile"
+     fi
+  ])
+
+  if ! "$ac_cv_guile_program_name" --version >&AS_MESSAGE_LOG_FD 2>&AS_MESSAGE_LOG_FD; then
+    AC_MSG_ERROR(['$ac_cv_guile_program_name' appears to be unusable])
+  fi
+
+  GUILE="$ac_cv_guile_program_name"
+
   AC_SUBST([GUILD])
+  AC_SUBST([GUILE])
 ])
 
 dnl GDB_GUILD_TARGET_FLAG
diff --git a/gdb/configure b/gdb/configure
index d0fd1760b88..6a780810bc8 100755
--- a/gdb/configure
+++ b/gdb/configure
@@ -721,6 +721,7 @@ HAVE_GUILE_TRUE
 GUILE_LIBS
 GUILE_CPPFLAGS
 GUILD_TARGET_FLAG
+GUILE
 GUILD
 pkg_config_prog_path
 HAVE_PYTHON_FALSE
@@ -11499,7 +11500,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11502 "configure"
+#line 11503 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -11605,7 +11606,7 @@ else
   lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
   lt_status=$lt_dlunknown
   cat > conftest.$ac_ext <<_LT_EOF
-#line 11608 "configure"
+#line 11609 "configure"
 #include "confdefs.h"
 
 #if HAVE_DLFCN_H
@@ -28780,6 +28781,34 @@ $as_echo "$ac_cv_guild_program_name" >&6; }
 
   GUILD="$ac_cv_guild_program_name"
 
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for the absolute file name of the 'guile' command" >&5
+$as_echo_n "checking for the absolute file name of the 'guile' command... " >&6; }
+if ${ac_cv_guile_program_name+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ac_cv_guile_program_name="`"${with_guile}" --variable guile "${guile_version}"`"
+
+     # In Guile up to 2.0.11 included, guile-2.0.pc would not define
+     # the 'guile' and 'bindir' variables.  In that case, try to guess
+     # what the program name is, at the risk of getting it wrong if
+     # Guile was configured with '--program-suffix' or similar.
+     if test "x$ac_cv_guile_program_name" = "x"; then
+       guile_exec_prefix="`"${with_guile}" --variable exec_prefix "${guile_version}"`"
+       ac_cv_guile_program_name="$guile_exec_prefix/bin/guile"
+     fi
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_guile_program_name" >&5
+$as_echo "$ac_cv_guile_program_name" >&6; }
+
+  if ! "$ac_cv_guile_program_name" --version >&5 2>&5; then
+    as_fn_error $? "'$ac_cv_guile_program_name' appears to be unusable" "$LINENO" 5
+  fi
+
+  GUILE="$ac_cv_guile_program_name"
+
+
+
 
     ;;
   *)
@@ -28810,6 +28839,34 @@ $as_echo "$ac_cv_guild_program_name" >&6; }
 
   GUILD="$ac_cv_guild_program_name"
 
+  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for the absolute file name of the 'guile' command" >&5
+$as_echo_n "checking for the absolute file name of the 'guile' command... " >&6; }
+if ${ac_cv_guile_program_name+:} false; then :
+  $as_echo_n "(cached) " >&6
+else
+  ac_cv_guile_program_name="`"${pkg_config_prog_path}" --variable guile "${guile_version}"`"
+
+     # In Guile up to 2.0.11 included, guile-2.0.pc would not define
+     # the 'guile' and 'bindir' variables.  In that case, try to guess
+     # what the program name is, at the risk of getting it wrong if
+     # Guile was configured with '--program-suffix' or similar.
+     if test "x$ac_cv_guile_program_name" = "x"; then
+       guile_exec_prefix="`"${pkg_config_prog_path}" --variable exec_prefix "${guile_version}"`"
+       ac_cv_guile_program_name="$guile_exec_prefix/bin/guile"
+     fi
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_guile_program_name" >&5
+$as_echo "$ac_cv_guile_program_name" >&6; }
+
+  if ! "$ac_cv_guile_program_name" --version >&5 2>&5; then
+    as_fn_error $? "'$ac_cv_guile_program_name' appears to be unusable" "$LINENO" 5
+  fi
+
+  GUILE="$ac_cv_guile_program_name"
+
+
+
 
     ;;
   esac
diff --git a/gdb/configure.ac b/gdb/configure.ac
index aa91bfb3a17..b18187c6d7e 100644
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1104,6 +1104,7 @@ dnl        pkg-config --exists $version
 dnl        pkg-config --cflags $version
 dnl        pkg-config --libs $version
 dnl        pkg-config --variable guild $version
+dnl        pkg-config --variable guile $version
 dnl        The script will be called with $version having each value in
 dnl        $try_guile_versions until --exists indicates success.
 
diff --git a/gdb/data-directory/Makefile.in b/gdb/data-directory/Makefile.in
index 2a40be4ade0..c1aded24ca8 100644
--- a/gdb/data-directory/Makefile.in
+++ b/gdb/data-directory/Makefile.in
@@ -147,6 +147,7 @@ GUILE_COMPILED_FILES = \
 
 GUILD = @GUILD@
 GUILD_TARGET_FLAG = @GUILD_TARGET_FLAG@
+GUILE = @GUILE@
 
 # Flags passed to 'guild compile'.
 # Note: We can't use -Wunbound-variable because all the variables