Re: [RFC] Merge mi-cli.exp and mi2-cli.exp

[Pedro Alves <palves at redhat dot com>]

On 09/05/2012 01:29 AM, Stan Shebs wrote:
> On 9/3/12 2:08 AM, Vladimir Prus wrote:
>> On 31.08.2012 17:29, Yao Qi wrote:
>>> Unless I miss something, the intention of copying tests here is to test
>>> both '-i=mi' and '-i=mi2' respectively.  However, this duplicates the code,
>>> and increase the effort to maintain, IMO.
>>
>> Yep, that was the intent -- with the extra twist that tests for MI and MI2 are not necessary
>> identical. In other words, MI2 tests are tests for MI2 when MI2 was declared "done", and the
>> idea was that the output with "-i=mi2" would remain the same for years. -i=mi is our current
>> version of MI, which may evolve, and when MI3 is declared "done", the current tests will
>> be copied to mi3-* tests to keep backward compatibility in future.
>>
>> I am not quite sure how relevant this plan is these days.
> 
> 
> That plan has pretty much fallen by the wayside.  We should probably declare the current MI behavior as the "done" form of MI3, and disallow any incompatible changes.  If someone wants to get ambitious, they are free to specify and implement MI4. :-)

I agree the plan has fallen by the wayside, but because the introduction of MI3
as a valid setting was premature and a mistake.  MI2 is "done" in the sense that we
will never change MI2's grammar and input/output in a backward incompatible way.
We're free to add new output records, new commands, etc., but that isn't considered
backward incompatible.  That's all still layered on the same protocol.

Incompatible changes would be things like what Vlad wrote in the sources,
like:

  /* To cater for older frontends, emit ^running, but do it only once
     per each command.  We do it here, since at this point we know
     that the target was successfully resumed, and in non-async mode,
     we won't return back to MI interpreter code until the target
     is done running, so delaying the output of "^running" until then
     will make it impossible for frontend to know what's going on.

     In future (MI3), we'll be outputting "^done" here.  */
  if (!running_result_record_printed && mi_proceeded)
    {
      fprintf_unfiltered (raw_stdout, "%s^running\n",
                          current_token ? current_token : "");
    }

... and ...

          fputs_unfiltered (context->token, raw_stdout);
          /* There's no particularly good reason why target-connect results
             in not ^done.  Should kill ^connected for MI3.  */
          fputs_unfiltered (strcmp (context->command, "target-select") == 0
                            ? "^connected" : "^done", raw_stdout);


I can't find anything in the code that checks for MI version being 3 and
doing things differently.  So, MI3 is pretty much still reserved for when
we really need to be backwards incompatible (kind of like an ABI break).

So I don't see the point of calling the current form of MI, MI3.  It's
still the same as MI2.

> On the original question, I tend to agree with leaving the test files separate.  Shared code risks being unable to detect when the MI code is broken for one of the MI versions, but not the other; this is a rare case where we want the tests to be a little more brittle than usual.

OTOH, MI2 really broke backward compatibility with MI1 clients.  The differences between
MI1 and MI2 are not that great (grep for mi_version) though.

But more than that, in reality, we stopped supporting MI1 almost 10 years ago:

 http://sourceware.org/ml/gdb-patches/2004-02/msg00352.html

If you look at the current docs, you'll see we ended up with just calling it
deprecated, as a result of that thread.  But the reality is that we really
stopped supporting it.  See, for example that we deleted all the MI1 tests
back then too <http://sourceware.org/ml/gdb-patches/2004-02/msg00295.html>:

"In preparation for the next release - no point testing (i.e., maintaining) what isn't going
to be supported."

    2004-02-13  Andrew Cagney  <cagney@redhat.com>

        * gdb.mi/mi1-basics.exp, gdb.mi/mi1-break.exp: Delete file.
        * gdb.mi/mi1-console.exp, gdb.mi/mi1-disassemble.exp: Delete file.
        * gdb.mi/mi1-eval.exp, gdb.mi/mi1-hack-cli.exp: Delete file.
        * gdb.mi/mi1-pthreads.exp, gdb.mi/mi1-read-memory.exp: Delete file.
        * gdb.mi/mi1-regs.exp, gdb.mi/mi1-return.exp: Delete file.
        * gdb.mi/mi1-simplerun.exp, gdb.mi/mi1-stack.exp: Delete file.
        * gdb.mi/mi1-stepi.exp, gdb.mi/mi1-symbol.exp: Delete file.
        * gdb.mi/mi1-until.exp, gdb.mi/mi1-var-block.exp: Delete file.
        * gdb.mi/mi1-var-child.exp, gdb.mi/mi1-var-cmd.exp: Delete file.
        * gdb.mi/mi1-var-display.exp, gdb.mi/mi1-watch.exp: Delete file.


So my opinion is that we revisit the policy a bit, and backtrack a the
mi-.*exp vs mi2-.*exp idea, get rid of the duplication, and call everything
"MI2", as it is in practice (must be, because that's how we run the tests).
When we really introduce an incompatible change that actually justifies MI3,
_then_ we should revisit the policy of whether to mass copying/rename tests, or
share them, depending on how big the difference between the versions would be,
and therefore depending on the practicality of the different options.

As is, the double testing seems just pointless to me.

-- 
Pedro Alves