[Top][All Lists]

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

Re: GDB testsuite overrides default_target_compile and breaks

From: Jacob Bachmeyer
Subject: Re: GDB testsuite overrides default_target_compile and breaks
Date: Fri, 19 Jun 2020 19:00:30 -0500
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20090807 MultiZilla/ SeaMonkey/1.1.17 Mnenhy/

Tom Tromey wrote:
Jacob> What is needed in brief:
Jacob> (1) Merge the features of default_target_compile that the GDB
Jacob> testsuite depends on upstream for 1.6.3.
Jacob> (2) Find the actual extensibility that GDB needs here and add that
Jacob> support to the default_target_compile rewrite slated for 1.6.4.

Sounds reasonable to me.

Jacob> The changes were to an internal component and broke an out-of-tree
Jacob> copy of same.  I have to draw a line somewhere, and "monkeypatching
Jacob> the core is not supported and can break" is necessary for DejaGnu to
Jacob> develop at all.

FWIW this makes sense to me.  I think it's never been a great idea to
have this override in gdb.  Dejagnu wasn't very actively maintained for
a long time, and had low engagement from gdb developers, so it was more
like a convenient way to move forward.

From how it was explained to me, that may have been the only way forward at the time.

Jacob> Monkeypatching the DejaGnu core is not supported and cannot be
Jacob> supported long-term.

gdb does it other spots as well.  For example, check-test-names.exp
overrides log_summary and reset_vars that gdb can show duplicated test
names, and gdb.exp renames "cd" to avoid problems with relative log file

I will look into these and see what we can do to move generally useful functionality upstream. (There are many features in the GDB testsuite that I eventually want in DejaGnu, like parallel testing, although the core implementation of that might be somewhat different as I want it to be as close to transparent as possible.)

Quick checks on those examples suggest that they will not cause problems (until the overridden procedures disappear into an internal namespace) because check-test-names.exp wraps log_summary and reset_vars instead of replacing them with out-of-tree copies. Wrapping "cd" avoids problems for similar reasons (technically that one is monkeypatching Tcl itself rather than DejaGnu) but DejaGnu's own initialization procedures should probably ensure that the log file is active with an absolute file name.

I might have to add a "monkeypatch the monkeypatches" module, loaded after the init files, to maintain backwards compatibility with the recent GDB releases, but that is a last resort.

Jacob> GDB seems to have also extended default_target_compile, so a
Jacob> discussion on this is needed because there seems to be a need for some
Jacob> kind of language extensibility feature to allow new
Jacob> language-default-selector options to be defined without overriding the
Jacob> entire default_target_compile procedure

Yes, that would be good to have.  I think this has been patched when a
new language is added to gdb; changing this function to work in some
extensible way would be a nice improvement... while there are no
concrete plans I know of to add new languages to gdb, one never knows
when someone may show up with patches.

The rewritten default_target_compile will be less of an "if forest" and more table-driven. Language defaults then become entries in an internal table, so all we need is an API call for init files to add entries to that table.

Jacob> If I understand correctly, merging the features currently carried in
Jacob> gdb/testsuite/lib/future.exp upstream will cause all existing GDB Jacob> releases to correctly use the upstream version of
Jacob> default_target_compile, making the partial reversion patch
Jacob> unnecessary.

Yes, assuming that gdb's detection code continues to work.  It does

    if {[info procs find_gnatmake] == ""} {
        rename gdb_find_gnatmake find_gnatmake
        set use_gdb_compile 1
    # ... sequence of such checks ...
    if {$use_gdb_compile} {
        catch {rename default_target_compile {}}
        rename gdb_default_target_compile default_target_compile

That code is testing the existence of API procedures to decide if it wants to override an internal procedure. There are no plans to remove find_* prior to 2.0; even if a better API ("testsuite tool"?) is introduced, compatibility shims will remain. (Now that I think about it, even 2.0 may still carry a "compatibility mode" for the 1.x API.)

Anyway, I compared gdb_default_target_compile with
default_target_compile.  I think there are two kinds of changes.


I can write some patches to merge this stuff in.

Many thanks for the patch set. I will merge them and add documentation and tests over the weekend or so.

-- Jacob

reply via email to

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