From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25653 invoked by alias); 11 Sep 2012 15:15:51 -0000 Received: (qmail 25624 invoked by uid 22791); 11 Sep 2012 15:15:49 -0000 X-SWARE-Spam-Status: No, hits=-7.7 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 11 Sep 2012 15:15:34 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q8BFFW5K009758 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 11 Sep 2012 11:15:32 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q8BFFUQG016426; Tue, 11 Sep 2012 11:15:31 -0400 Message-ID: <504F5592.6000102@redhat.com> Date: Tue, 11 Sep 2012 15:15:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120828 Thunderbird/15.0 MIME-Version: 1.0 To: Stan Shebs CC: gdb-patches@sourceware.org Subject: Re: [RFC] Merge mi-cli.exp and mi2-cli.exp References: <1346419770-5718-1-git-send-email-yao@codesourcery.com> <50469CDA.1030406@earthlink.net> In-Reply-To: <50469CDA.1030406@earthlink.net> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-09/txt/msg00155.txt.bz2 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 : "In preparation for the next release - no point testing (i.e., maintaining) what isn't going to be supported." 2004-02-13 Andrew Cagney * 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