Return-Path: X-Original-To: apmail-subversion-dev-archive@minotaur.apache.org Delivered-To: apmail-subversion-dev-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 2ED92C64A for ; Thu, 13 Jun 2013 16:11:21 +0000 (UTC) Received: (qmail 51398 invoked by uid 500); 13 Jun 2013 16:11:20 -0000 Delivered-To: apmail-subversion-dev-archive@subversion.apache.org Received: (qmail 51098 invoked by uid 500); 13 Jun 2013 16:11:18 -0000 Mailing-List: contact dev-help@subversion.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list dev@subversion.apache.org Received: (qmail 51076 invoked by uid 99); 13 Jun 2013 16:11:17 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 13 Jun 2013 16:11:17 +0000 X-ASF-Spam-Status: No, hits=1.5 required=5.0 tests=HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (nike.apache.org: domain of doug.robinson@wandisco.com designates 209.85.212.50 as permitted sender) Received: from [209.85.212.50] (HELO mail-vb0-f50.google.com) (209.85.212.50) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 13 Jun 2013 16:11:10 +0000 Received: by mail-vb0-f50.google.com with SMTP id w16so7169760vbb.9 for ; Thu, 13 Jun 2013 09:10:49 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:x-gm-message-state; bh=jKsz07WC8+bcqZSUenK3Pp+0UqgdMNdCTN2ncB9/zTY=; b=Eft2N4esNz+/v7rLibmepNNOWGMwANd87so+ohUGqkKWKgSBPYFDekmp3TpTI/ba+q p8rt1QFu4vjJ8LWjjRQEn9Flna7lW+IiDS6/UTFfyVmLPHYbeQ1KxQbJFi2s4qg0cdzO PJSlX3cL8fUlpLJobqc7e30twDc7RYfH36dLlgDeIo+mMYg4ujaLenxjgIgCA6LJhsL8 YJso/Nz7KEBHBT/EEqPCpUqlK4uHYvi1BZ7fON3mbaaylOWQtIlO3DEahjZ3i5mcynLM UstzfpXD7KMSd6+wATDTq0D/yi0oZT0AjH+070RhvnqkIfr3W4x1Uixk677RWhSVb1Uu j9Ew== MIME-Version: 1.0 X-Received: by 10.52.28.171 with SMTP id c11mr566026vdh.18.1371139849523; Thu, 13 Jun 2013 09:10:49 -0700 (PDT) Received: by 10.58.7.228 with HTTP; Thu, 13 Jun 2013 09:10:49 -0700 (PDT) In-Reply-To: <20130613083546.GA2773@tarsus.local2> References: <20130613083546.GA2773@tarsus.local2> Date: Thu, 13 Jun 2013 12:10:49 -0400 Message-ID: Subject: Re: Kidney blame's behaviour and edge cases From: Doug Robinson To: Daniel Shahaf Cc: dev@subversion.apache.org Content-Type: multipart/alternative; boundary=20cf30780ae0ab2de804df0b5f42 X-Gm-Message-State: ALoCoQmExGvzR2zM3X55jLK9dyl6Zc8nHkbkqn/3Y36s4FbSl11ot5QCQLSwHDVIbXInd7fkYv0VUYi+shBUQdZ3uyqliPgOmIpeMUF7uSlMDeZzolFCW7k= X-Virus-Checked: Checked by ClamAV on apache.org --20cf30780ae0ab2de804df0b5f42 Content-Type: text/plain; charset=US-ASCII Daniel: I think that simply enabling M wrote: > Definition: "kidney blame" == "blame -r N:M" with M an error (raised by svn_client_blame5()). > > By and large, it should do exactly what 'blame -r M:N' does: walk the > revisions from "before-the-colon revision" to "after-the-colon revision" > and then print the contents of the "after the colon" revision, with each > line annotated by the revnum of the "rightmost" (that is: nearest to the > revnum on the right of the colon) revision within the range that added > that line. In other words, if > (iota@4, kappa@1), (iota@3, kappa@2), (iota@2, kappa@3), (iota@1, kappa@4) > are pairs of byte-for-byte-identical files, then 'blame -r 1:4 kappa' > and 'blame -r 4:1 iota' will output the same thing. > > Currently, the non-kidney blame checks one revision before the start --- > that is, 'blame -r M:N' actually calls svn_ra_get_file_revs2(start=M-1, > end=N). The purpose of this is to differentiate "lines added in rN" > from "lines present since before rN". That makes 'blame -r M:N' > correspond to 'diff -r M-1:N' or 'diff -c M:N', in that it also shows > changes made _by_ rM, not only _since_ rM. > > That behaviour cannot easily replicated for the -r N:M case since it's > not possible to cheaply find the "next interesting revision" after rN, > but in any case I think it shouldn't be: 'blame -r M:N' should not have > automagically decremented M --- instead, if the user had wanted to see > what lines were added in rM, as opposed to before it, the user should > have specified M-1 as the start revision. That way, 'blame -r M:N' > would be consistent with 'diff -r M:N'. > > So I suggest that blame -r N:M not try to find the "next change after > rN" (that's the analogous thing to what "decrement M" is in the '-r M:N' > case). That means -r M:N and -r N:M are assymetrical, and I propose we > fix that by making -r M:N not decrement M --- which we can probably only > do in 2.0. > > --- > > Another issue: what should 'blame -r 3:3' do? Currently it is allowed, > and prints '-' for lines added before r3 and '3' for lines added in r3. > I am not sure whether that is intentional / by design, or just an > accident of the fact that whoever added the 'end_rev < start_rev' check > should have used the broader condition 'end_rev <= start_rev' instead. > (See r7438 <-> http://svn.apache.org/r847512). > > It seems to me it should ideally print '3' for every line, and the user > should pass '-r 2:3' if he wants to distinguish "added in r3" from > "added before r3". It would be easy to preserve the current behaviour, > though, of printing '-' rather than '2' (where '2' here is the youngest > change to that line, for lines added before r3). > > --- > > Cheers, > > Daniel > -- Douglas B. Robinson | *Senior Product Manager* WANdisco // *Non-Stop Data* t. 925-396-1125 e. doug.robinson@wandisco.com -- THIS MESSAGE AND ANY ATTACHMENTS ARE CONFIDENTIAL, PROPRIETARY, AND MAY BE PRIVILEGED. If this message was misdirected, WANdisco, Inc. and its subsidiaries, ("WANdisco") does not waive any confidentiality or privilege. If you are not the intended recipient, please notify us immediately and destroy the message without disclosing its contents to anyone. Any distribution, use or copying of this e-mail or the information it contains by other than an intended recipient is unauthorized. The views and opinions expressed in this e-mail message are the author's own and may not reflect the views and opinions of WANdisco, unless the author is authorized by WANdisco to express such views or opinions on its behalf. All email sent to or from this address is subject to electronic storage and review by WANdisco. Although WANdisco operates anti-virus programs, it does not accept responsibility for any damage whatsoever caused by viruses being passed. --20cf30780ae0ab2de804df0b5f42 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
Daniel:

I think that simply enabling M<N (where it = is now an error) will create the situation where the user makes a mistake, = gets something they don't expect and tries to interpret it based on the= ir desire - leading to confusion. =A0I believe M<N should still be an er= ror. =A0A new option (--reverse ?) should be required to make it clear that= the user wants the reverse blame walk.

=
Thank you.

Doug


On Thu, Jun 13, 2013 at 4:35 AM, Daniel Shahaf <daniels= h@elego.de> wrote:
Definition: "kidney blame" =3D=3D = "blame -r N:M" with M<N. =A0Currently it is
an error (raised by svn_client_blame5()).

By and large, it should do exactly what 'blame -r M:N' does: walk t= he
revisions from "before-the-colon revision" to "after-the-col= on revision"
and then print the contents of the "after the colon" revision, wi= th each
line annotated by the revnum of the "rightmost" (that is: nearest= to the
revnum on the right of the colon) revision within the range that added
that line. =A0In other words, if
(iota@4, kappa@1), (iota@3, kappa@2), (iota@2, kappa@3), (iota@1, kappa@4)<= br> are pairs of byte-for-byte-identical files, then 'blame -r 1:4 kappa= 9;
and 'blame -r 4:1 iota' will output the same thing.

Currently, the non-kidney blame checks one revision before the start --- that is, 'blame -r M:N' actually calls svn_ra_get_file_revs2(start= =3DM-1,
end=3DN). =A0The purpose of this is to differentiate "lines added in r= N"
from "lines present since before rN". =A0That makes 'blame -r= M:N'
correspond to 'diff -r M-1:N' or 'diff -c M:N', in that it = also shows
changes made _by_ rM, not only _since_ rM.

That behaviour cannot easily replicated for the -r N:M case since it's<= br> not possible to cheaply find the "next interesting revision" afte= r rN,
but in any case I think it shouldn't be: 'blame -r M:N' should = not have
automagically decremented M --- instead, if the user had wanted to see
what lines were added in rM, as opposed to before it, the user should
have specified M-1 as the start revision. =A0That way, 'blame -r M:N= 9;
would be consistent with 'diff -r M:N'.

So I suggest that blame -r N:M not try to find the "next change after<= br> rN" (that's the analogous thing to what "decrement M" is= in the '-r M:N'
case). =A0That means -r M:N and -r N:M are assymetrical, and I propose we fix that by making -r M:N not decrement M --- which we can probably only do in 2.0.

---

Another issue: what should 'blame -r 3:3' do? =A0Currently it is al= lowed,
and prints '-' for lines added before r3 and '3' for lines = added in r3.
I am not sure whether that is intentional / by design, or just an
accident of the fact that whoever added the 'end_rev < start_rev'= ; check
should have used the broader condition 'end_rev <=3D start_rev' = instead.
(See r7438 <-> http://svn.apache.org/r847512).

It seems to me it should ideally print '3' for every line, and the = user
should pass '-r 2:3' if he wants to distinguish "added in r3&q= uot; from
"added before r3". =A0It would be easy to preserve the current be= haviour,
though, of printing '-' rather than '2' (where '2' = here is the youngest
change to that line, for lines added before r3).

---

Cheers,

Daniel



--
Douglas B. Robinson=A0|=A0Senior Product Manager

WANdisco=A0//=A0Non-Stop Data
=

THIS MESS= AGE AND ANY ATTACHMENTS ARE CONFIDENTIAL, PROPRIETARY, AND MAY BE PRIVILEGE= D. =A0If this message was misdirected, WANdisco, Inc. and its subsidiaries,= ("WANdisco") does not waive any confidentiality or privilege. = =A0If you are not the intended recipient, please notify us immediately and = destroy the message without disclosing its contents to anyone. =A0Any distr= ibution, use or copying of this e-mail or the information it contains by ot= her than an intended recipient is unauthorized. =A0The views and opinions e= xpressed in this e-mail message are the author's own and may not reflec= t the views and opinions of WANdisco, unless the author is authorized by WA= Ndisco to express such views or opinions on its behalf. =A0All email sent t= o or from this address is subject to electronic storage and review by WANdi= sco. =A0Although WANdisco operates anti-virus programs, it does not accept = responsibility for any damage whatsoever caused by viruses being passed.

=
--20cf30780ae0ab2de804df0b5f42--