[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: PATCH: add search path for tool init file (part of fix for Automake

From: Jacob Bachmeyer
Subject: Re: PATCH: add search path for tool init file (part of fix for Automake misintegration)
Date: Wed, 06 Mar 2019 17:49:24 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20090807 MultiZilla/ SeaMonkey/1.1.17 Mnenhy/

Andrew Burgess wrote:
* Jacob Bachmeyer <address@hidden> [2018-11-14 21:50:20 -0600]:
This patch fixes the problem of the first "make check" after running
configure complaining about being unable to find tool init files that I have
previously described.

ChangeLog entry
        * runtest.exp (load_lib): Clean up whitespace.
        (load_tool_init): Search for tool init file instead of assuming
        exactly one location.  Backwards compatibility is maintained.

This patch, combined with a recent change in GDB, means that GDB
testing no longer works.

Er... oops?  ...maybe?

I can see what the problem is, but I was
hoping to get your guidance for what the best fix is.

I've explained more inline....

[response inline with your explanation]

diff --git a/runtest.exp b/runtest.exp
index 84e3e6b..5ced8ab 100644
--- a/runtest.exp
+++ b/runtest.exp
@@ -929,25 +929,21 @@ proc load_tool_init { file } {
     global srcdir
     global loaded_libs
- if {[info exists loaded_libs($file)]} {
+    if {[info exists loaded_libs(tool/$file)]} {
- set loaded_libs($file) ""
-    verbose "Looking for tool init file $srcdir/lib/$file"
+    set loaded_libs(tool/$file) ""

This change is the one that causes problems.  Previously the call to
'load_tool_init' resulted in the tool init file being recorded in
'loaded_libs' just like any other library file - using just the name
of the file, so for GDB this was 'gdb.exp'.

After this patch the tool init file is recorded as 'tool/gdb.exp'.

The patch also adds a new directory at the beginning of the search path for tool init files: testsuite/lib/tool. This allows a testsuite to have both a library module for using a tool as part of other tests and a separate tool init file for testing the tool itself. There was previously no elegant solution for this case.

GDB (probably wrongly) includes, from its target init files (for
example gdb/testsuite/config/unix.exp), the tool init file using
'load_lib gdb.exp'.

I concur that this is incorrect; the tool init file is something that the DejaGnu core is supposed to handle, is firmly on the "testsuite" side of the testsuite/testing lab distinction, and is the first piece from the testsuite that DejaGnu loads. I argue that attempting to load the tool init file from any other init file is a bug in the testsuite and/or site configuration.

The problem is that gdb.exp now ends up getting included twice, first
time it is included and recorded as 'tool/gdb.exp' and the second time
as just 'gdb.exp'.  A recent change means that this file can _only_ be
parsed once (a use of rename is the blocker).

As an alternative, could the rename be made conditional on the result of [info commands NEW_NAME]?

I have had similar problems with another project, where I used Tcl namespaces in the test drivers, then switched to multipass testing. I had to add "namespace forget *" just ahead of each file's "namespace import" block, along with other bits here and there to make sure that various other internal states are reset before being loaded.

Prior to this patch the second attempt to include 'gdb.exp' would be
ignored as 'gdb.exp' was already in loaded_libs.

The obvious fix is probably just, remove the 'load_lib gdb.exp' calls
from the GDB testsuite,

Did the GDB testsuite ever actually need those calls? Presumably, DejaGnu has always correctly loaded gdb.exp as the tool init file, well before loading the target init file from setup_target_hook.

(Side note: I am currently working on moving all actual code out of the {setup,cleanup}_{build,host,target}_hook procedures and establishing those as pure customization points for additional processing that a testing lab may want to insert. If the GDB testsuite is depending on overriding any of those, now would be a good time to fix those bugs.)


It's not clear from this patch why this change was necessary, could we
revert to the old behaviour?

The patch also allows tool init files to appear in testsuite/lib/tool, a new feature that allows both a library and tool init file to exist under the same name. For this to work, the tool init file must be recorded separately from a same-named library module.

There's many calls to 'search_and_load_file' in dejagnu (the core of
things like load_lib and load_tool_init), but only some of them are
guarded with a check of loaded_libs.  Would there be an advantage of
moving the loaded_libs check into 'search_and_load_file', and placing
the absolute path to the file being loaded into loaded_libs?

That might be a good idea, but I have been told (<URL:>) that the "*load*" procedures should be considered stable API and that refactoring them is ill-advised without great care due to their use in board description files, only some of which are shipped with DejaGnu.

Thanks for your time,


-- Jacob

reply via email to

[Prev in Thread] Current Thread [Next in Thread]