Re: RFA (testsuite, mi testsuite): Support testing gdbserver

[Elena Zannoni <ezannoni at redhat dot com>]

Daniel Jacobowitz writes:
 > On Thu, Jul 11, 2002 at 12:20:44PM -0400, Elena Zannoni wrote:
 > > Daniel Jacobowitz writes:
 > >  > If this patch looks familiar, it's not because you're imagining things. 
 > >  > I've posted it I believe four times now, and filed a PR about it (it's
 > >  > gdb/440), and never gotten a single comment.  I'd appreciate it if one of
 > >  > our busy maintainers could look over it, as I'm tired of carrying it around
 > >  > locally.  Is either our primary testsuite maintainer or his stand-in still
 > >  > here?  The last thing I saw from Fernando was his statement on May 1st that
 > >  > he'd be gone until the 21st, and the last thing I saw from Michael Chastain
 > >  > was on June 1st...
 > >  > 
 > >  > The changes in this patch:
 > >  >   - Run gdbserver, a target binary, on the target instead of the host
 > > 
 > > I think this is not mine to review. However, if you think it's ok for
 > > gdbserver and it doesn't break anything else, I think it could go
 > > in. But it would need to be a separate patch. I assume this is
 > > independent on mi being involved, right?
 > 
 > Yep.  I'll split it out, sorry about that.
 > 

thanks. Could you check that in?

 > >  >   - Load mi-support.exp from lib/gdb.exp instead of from the MI tests,
 > >  >    so that config/* have a chance to override it.
 > >  >   - Add support for MI in config/gdbserver.exp and config/monitor.exp,
 > >  >    which required some substantial changes in mi-support.exp.
 > > 
 > > I see your problem. Yes, it's a ugly one. I have thought of an
 > > alternative approach. The thing I like about mi-support.exp is that it
 > > is self contained, and all the bits for the running the testsuite with
 > > MI are in there. With your change, the mi support procedures start
 > > appearing in other files, and basically having an mi-support.exp file
 > > becomes useless. Instead of putting MI support into the gdbserver and
 > > monitor files, could we do the opposite: i.e. add mi specific
 > > gdbserver variants in mi-support.exp? The problem would remain
 > > of how to know that we are running gdbserver, instead of, say, the
 > > sim.  In mi-support.exp the procedure mi_gdb_load would need to know
 > > that.  What if we do the same trick that sid uses? I.e. identify a
 > > unique procedure name in the gdbserver.exp file, so that you know it
 > > has been loaded and then add another case to the if-else in
 > > mi_gdb_load to call a mi_gdbserver_load function, which would be in
 > > mi-support.exp?
 > > 
 > > something like 
 > > if [info procs gdbserver_gdb_load] then call mi_gdbserver_gdb_load
 > > 
 > > The mi_gdb_target_cmd could also be included in mi-support.exp.
 > > 
 > > Of course you would have to add the definition of a gdbserver.exp
 > > specific procedure.
 > > 
 > > This way the mi support would remain isolated, and only be used when
 > > running mi tests. And we wouldn't need to touch any of the mi test files.
 > > 
 > > I like the breaking of the mi_gdb_load function into 2 functions.
 > > It makes it much cleaner.
 > 
 > I don't really like this idea... let me try to explain why.
 > 
 > - First of all, the mi-support code is present when running non-MI
 > tests too anyway.  DejaGNU runs every test in the same evaluator
 > context.  This means that one test can affect a later test.  I
 > personally think this reeks of bad design... search the archives for
 > dbx.exp for an example of the problems this can cause.
 > 

dbx.exp renames a generic function (gdb_load) which is used in the
rest of the testsuite.  The mi-support.exp file doesn't do that. It
has its own functions prefixed by mi_ and the gdb.mi files use those
exclusively. The mi-support file gets loaded only when gdb.mi tests
are executed. I agree that it stays around for the tests that are run
after gdb.mi, but none of those tests should use the mi_* procs.

 > - Second, right now all of the target-specific code is in config/ in
 > its own little files, in principal.  Apparently some of the SID code
 > has migrated out to mi-support.exp, but there's an awful lot of other
 > targets in config/, and cluttering mi-support.exp with support code for
 > each of them seems like a really bad idea.  Most of them just punt to
 > monitor.exp, but there's gdbserver, netware, sid, sim, slite, sparclet,
 > udi, vx...
 > 

mi-support.exp was introduced so that all the mi specific stuff would
be in its own file.  I would prefer if it remained like that.  (Sid
was the first target that mi was tested with.)

 > The fact that these need to parse GDB messages and are incompatible
 > with MI suggests to me that we desperately need another abstraction
 > somewhere, but keeping the GDB support code for a particular target
 > isolated to that target seems like the path of reasonable segmentation.
 > 
 > In short, I believe that the benefits of keeping target monitor code
 > isolated outweigh the benefits of keeping MI (a fundamental part of the
 > GDB that we're testing, and growing more so) isolated.

I don't want to get into a religious war about this. :-) I agree that
the testsuite design could be improved. Relying on binding of tcl
variables is pretty fragile.  But that's beyond the scope of this
particular discussion.

 > 
 > > (what is use_default_mi_gdb_load that you mention in the ChangeLog?)
 > 
 > A remnant from a different approach; removed.
 > 

thanks.

 > -- 
 > Daniel Jacobowitz                           Carnegie Mellon University
 > MontaVista Software                         Debian GNU/Linux Developer


Elena