Re: [PATCH] gdb/testsuite: Prepare for DejaGnu 1.6.2

[Sergio Durigan Junior <sergiodj@redhat.com>]

On Thursday, March 07 2019, Andrew Burgess wrote:

> Changes in DejaGnu 1.6.2 mean that our testsuite will no longer run.
> This is because of some confusion over how the gdb.exp file is
> handled.
>
> The gdb.exp file is really the tool init file, which is loaded from
> within the DejaGnu core, and it should not be loaded directly from any
> other file in the testsuite.
>
> DejaGnu tries to prevent the same library being loaded twice by
> remembering the names of library files as they are loaded.  Until
> recently loading the tool init file in DejaGnu was very similar to
> loading a library file, as a result, loading the gdb.exp tool init
> file simply recorded 'gdb.exp' as having been loaded, future attempts
> to load 'gdb.exp' as a library would then be ignored (as the file was
> marked as already loaded).
>
> DejaGnu has now changed so that it supports having both a tool init
> file and a library with the same name, something that was not possible
> before.  What this means however is that when the core loads the
> 'gdb.exp' tool init file it no longer marks the library 'gdb.exp' as
> having been loaded.  When we then execute 'load_lib gdb.exp' we then
> try to reload the 'gdb.exp' file.
>
> Unfortunately our gdb.exp file can only be loaded once.  It use of
> 'rename cd builtin_cd' means that a second attempt to load this file
> will fail.
>
> This was discussed on the DejaGnu list here:
>    http://lists.gnu.org/archive/html/dejagnu/2019-03/msg00000.html
>
> and the suggested advice is that, unless we have some real requirement
> to load the tool init file twice, we should remove calls to 'load_lib
> gdb.exp' and rely on DejaGnu to load the file for us, which is what
> this patch does.
>
> I've tested with native X86-64/GNU Linux and see no regressions.

FWIW, I've always found these "load_lib gdb.exp" to be remnants of an
old past, and not needed anymore (or even buggy).  Thanks for the
investigation; IMO, the patch is fine could almost be considered an
"obvious" cleanup.  I'm not a global maintainer, though.

Thanks,

> gdb/testsuite/ChangeLog:
>
> 	* config/default.exp: Remove 'load_lib gdb.exp'.
> 	* config/monitor.exp: Likewise.
> 	* config/sid.exp: Likewise.
> 	* config/sim.exp: Likewise.
> 	* config/slite.exp: Likewise.
> 	* config/unix.exp: Likewise.
> 	* gdb.base/default.exp: Remove unhelpful comment.
> ---
>  gdb/testsuite/ChangeLog            | 10 ++++++++++
>  gdb/testsuite/config/default.exp   |  2 +-
>  gdb/testsuite/config/monitor.exp   |  3 ---
>  gdb/testsuite/config/sid.exp       |  2 --
>  gdb/testsuite/config/sim.exp       |  2 --
>  gdb/testsuite/config/slite.exp     |  2 +-
>  gdb/testsuite/config/unix.exp      |  2 --
>  gdb/testsuite/gdb.base/default.exp |  2 --
>  8 files changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/gdb/testsuite/config/default.exp b/gdb/testsuite/config/default.exp
> index 8b70ee4b507..325a58851ec 100644
> --- a/gdb/testsuite/config/default.exp
> +++ b/gdb/testsuite/config/default.exp
> @@ -13,4 +13,4 @@
>  # You should have received a copy of the GNU General Public License
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
> -load_lib gdb.exp
> +# Nothing extra is needed here.
> diff --git a/gdb/testsuite/config/monitor.exp b/gdb/testsuite/config/monitor.exp
> index be8d8429b8d..48a01feb706 100644
> --- a/gdb/testsuite/config/monitor.exp
> +++ b/gdb/testsuite/config/monitor.exp
> @@ -14,9 +14,6 @@
>  # You should have received a copy of the GNU General Public License
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
> -load_lib gdb.exp
> -# puts "***** DID USE MONITOR ******"
> -
>  #
>  # gdb_target_cmd
>  # Send gdb the "target" command
> diff --git a/gdb/testsuite/config/sid.exp b/gdb/testsuite/config/sid.exp
> index 3c92a4fe9f5..17a3ad568d9 100644
> --- a/gdb/testsuite/config/sid.exp
> +++ b/gdb/testsuite/config/sid.exp
> @@ -14,8 +14,6 @@
>  # You should have received a copy of the GNU General Public License
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
> -load_lib gdb.exp
> -
>  proc sid_start {} {
>      global verbose
>  
> diff --git a/gdb/testsuite/config/sim.exp b/gdb/testsuite/config/sim.exp
> index dafb1a26063..8d87e3089b0 100644
> --- a/gdb/testsuite/config/sim.exp
> +++ b/gdb/testsuite/config/sim.exp
> @@ -14,8 +14,6 @@
>  # You should have received a copy of the GNU General Public License
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  
> -load_lib gdb.exp
> -
>  #
>  # gdb_target_sim
>  # Set gdb to target the simulator
> diff --git a/gdb/testsuite/config/slite.exp b/gdb/testsuite/config/slite.exp
> index 0fa472ee0cb..ffbd9c811e8 100644
> --- a/gdb/testsuite/config/slite.exp
> +++ b/gdb/testsuite/config/slite.exp
> @@ -27,7 +27,7 @@
>  # rather than being displayed by gdb.
>  
>  load_lib remote.exp
> -load_lib gdb.exp
> +
>  set gdb_prompt "\\(gdb\\)"
>  
>  #
> diff --git a/gdb/testsuite/config/unix.exp b/gdb/testsuite/config/unix.exp
> index 2ab1d9abf0c..770e69b4c32 100644
> --- a/gdb/testsuite/config/unix.exp
> +++ b/gdb/testsuite/config/unix.exp
> @@ -21,5 +21,3 @@
>  # does not seem to be enough.  Try starting with 60.
>  set timeout 60
>  verbose "Timeout is now $timeout seconds" 2
> -
> -load_lib gdb.exp
> diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp
> index 8a11401c79f..ece1428e617 100644
> --- a/gdb/testsuite/gdb.base/default.exp
> +++ b/gdb/testsuite/gdb.base/default.exp
> @@ -25,8 +25,6 @@ set timeout 60
>  # test default actions of gdb commands
>  #
>  
> -#load_lib gdb.exp
> -
>  gdb_test "add-symbol-file" "add-symbol-file takes a file name and an address" "add-symbol-file"
>  
>  # test append
> -- 
> 2.14.5

-- 
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF  31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/