Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Simon Marchi <simon.marchi@ericsson.com>, gdb-patches@sourceware.org
Subject: Re: [PATCH] Add mi-threads-interrupt.exp test (PR 20039)
Date: Wed, 04 May 2016 19:27:00 -0000	[thread overview]
Message-ID: <1c2b69f6-6bd0-862e-cbce-e1be4dd7df15@redhat.com> (raw)
In-Reply-To: <572A399C.60902@ericsson.com>

On 05/04/2016 07:04 PM, Simon Marchi wrote:

> Why would it make sense to set async on in a board file?  My understanding is
> that the board file defines the debug target side of things, but the fact that MI
> is sync or async (or that we use MI at all) is not decided by the debug target.

Either setting GDBFLAGS on the board file or on the command line, it's
really the same thing: an expedient way to run the whole testsuite in a 
certain mode.  I've found it quite useful over the years, while
working on e.g., "set breakpoints always-inserted on", or when
doing all the work toward making "set target-async on" the default.
In either case, this was not a setting that users really should really
need to be toggling themselves.

> I would think that it's up to the tests to set async however they want it,
> depending on what they test.  In this particular case, wouldn't it be better
> to run both the test in sync and async modes, as we do often for non-stop?

A bit of history here.  We nowadays have two distinct commands:

 #1 - "set mi-async on"

 #2 - "maint set target-async on"

The mi-async command makes all MI execution commands be background
commands.  That is, e.g., "-exec-continue" turns into "continue&" rather
than "continue".  (Plus some other minor differences.)
It doesn't make much sense for MI execution commands to ever be sync,
but, frontends that are not prepared for async would break, thus the need
for the switch.

The "maint set target-async on" command changes the way the
target backend works, but other than _enabling_ accessing to 
background commands, it should have no user-visible behavior.
The only reason for the command to exist, is convenience, for
emulating targets that don't do async yet, on GNU/Linux.

While a few releases back we had a single "set target-async" that
conflicted both #1 and #2.  And at the time, because that command
actually changed how the target_ops behaves, it was sort of
a maintenance command in disguise, it didn't make sense to 
change _every_ test in the testsuite to try
both "set target-async on/off".  So years ago (circa 2007/2008, I believe),
what we did was make it possible to force "set target-async on", and run
the testsuite that way.  And then since MI changed output when tested
that way, we made the testsuite cope.

And then nobody bothered yet to make all _MI_ tests run with
"set mi-async on/off", while leaving all CLI tests alone at the
same time.

> 
> So, for example:
> 
> proc test_continue_interrupt { async } {
>   with_test_prefix "async=$async" {
>     ...
>   }
> }
> 
> foreach async {on off} {
>   test_continue_interrupt $async
> }
> 
> It hacked the test quickly, and it shouldn't be too hard to do.
> 

Yeah.  But in reality, we should really be doing that to
all MI tests.

For the console series, I'm adding another test axis to consider,
which is to run all MI tests with MI forced to a separate tty.
This has been _very_ convenient to catch problems.  For now,
I'm adding a way to force that from the command line, but I could
see us run all MI tests always in both normal MI, and separate
MI modes.  It's easy to explode the number of testing axes though...

This test in particular though, maybe it just doesn't make
sense to run in async mode at all, as sending a ctrl-c to
gdb in that case _should_ result in a Quit.

Thanks,
Pedro Alves


  reply	other threads:[~2016-05-04 19:27 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-03 20:00 Simon Marchi
2016-05-03 21:57 ` Pedro Alves
2016-05-03 21:59   ` Pedro Alves
2016-05-03 22:04   ` Pedro Alves
2016-05-04  9:20   ` Pedro Alves
2016-05-04 18:04     ` Simon Marchi
2016-05-04 19:27       ` Pedro Alves [this message]
2016-05-04 14:34   ` Simon Marchi
2016-05-04 15:07     ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1c2b69f6-6bd0-862e-cbce-e1be4dd7df15@redhat.com \
    --to=palves@redhat.com \
    --cc=gdb-patches@sourceware.org \
    --cc=simon.marchi@ericsson.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox