Login | Register
My pages Projects Community openCollabNet

Discussions > dev [DISABLED] > Re: [PATCH] Fix for blame -g incorrect revisions

subversion
Discussion topic

Back to topic list

Re: [PATCH] Fix for blame -g incorrect revisions

Author Lieven Govaerts <svnlgo at mobsol dot be>
Full name Lieven Govaerts <svnlgo at mobsol dot be>
Date 2009-05-13 23:19:58 PDT
Message Hi Alan,

the minimal change to make the current test working is committed in
r37723. Can you test if it fixes all problems for you?

I'll add a new test with some local changes in the file, to simulate
the problems that might necessitate the other changes in your patch.

thanks.

Lieven


On Thu, May 14, 2009 at 12:03 AM, Alan Wood <alan dot wood at clear dot net dot nz> wrote:
> On 13 May 2009 at 19:34, Lieven Govaerts wrote:
>> Hi Alan,
>>
>> I have a few questions on your patch (inline).
>>
>> > @@ -781,7 +781,7 @@
>> >                ​                   sb->data, FALSE, iterpool));
>> >                else
>> >                  SVN_ERR(receiver(rec​eiver_baton, line_no,
>> > SVN_INVALID_REVNUM,
>> > -                   ​              NULL, SVN_INVALID_REVNUM, NULL, NULL,
>> > +                   ​              NULL, merged_rev, merged_rev_props,
>> > merged_path,
>>
>> Why is this chunk needed?
> I can't guarantee that it is needed, but my logic goes like this.
>  if walk->rev can be NULL   // the case we are talking about
>   then it doesn't follow that walk_merged-> rev would also be NULL at the
> same line.
>  So there should be cases where there is merged information but no trunk!
> Anyway that was the logic, but it may be flawed.
> My thought today when having another look at this code:
>   I have found that walk->rev can be NULL from line 706, but there is no
> corresponding case for the merged chain. This is the case with local working
> copy mods which I have never tested.
> Looks like this part of the change is not needed.
>
>>
>> >                ​                   sb->data, TRUE, iterpool));
>> >              }
>> >            if (eof) break;
>> > Index: subversion/svn/blame-cmd.c
>> > ====================​====================​====================​=======
>> > --- subversion/svn/blame​-cmd.c   (revision 37376)
>> > +++ subversion/svn/blame​-cmd.c            (w​orking copy)
>> > @@ -178,8 +178,9 @@
>> >           revision which put the line in its current state, so we use
>> > the
>> >           earliest revision.  If we ever switch to a backward blame
>> > algorithm,
>> >           we may need to adjust this. */
>> > -      if (merged_revision < revision)
>> > -        {
>> > +      if (SVN_IS_VALID_REVNUM​(merged_revision) &&
>> > +                 (!SVN_IS_VALID_REVNUM(revision) || merged_revision <
>> > revision))
>> > +               {
>>
>> Same question here.
> The interface specification ( svn_client.h line 677 ) states that revision
> can be invalid so it shouldn't be used in the comparison without first being
> checked to see if it's valid. The same is also true for merged_revision.
>> The test case you provided - committed in r37719 - doesn't need those
>> two changes to pass.
> It looks like the invalid revs only occur with using blame on a modified
> working copy. I haven't looked into this.
>> Lieven
>>
>> --------------------​--------------------​--------------
>>
>> http://subversion.ti​gris.org/ds/viewMess​age.do?dsForumId=462​&dsMessageId=224​0565
>

« Previous message in topic | 7 of 15 | Next message in topic »

Messages

Show all messages in topic

[PATCH] Fix for blame -g incorrect revisions alwood Alan Wood 2009-04-20 02:33:25 PDT
     Re: [PATCH] Fix for blame -g incorrect revisions alwood Alan Wood 2009-04-23 02:11:41 PDT
         Re: [PATCH] Fix for blame -g incorrect revisions Lieven Govaerts <svnlgo at mobsol dot be> Lieven Govaerts <svnlgo at mobsol dot be> 2009-05-04 03:22:46 PDT
             Re: [PATCH] Fix for blame -g incorrect revisions alwood Alan Wood 2009-05-04 13:18:53 PDT
         Re: [PATCH] Fix for blame -g incorrect revisions Lieven Govaerts <svnlgo at mobsol dot be> Lieven Govaerts <svnlgo at mobsol dot be> 2009-05-13 10:34:52 PDT
             Re: [PATCH] Fix for blame -g incorrect revisions alwood Alan Wood 2009-05-13 15:04:09 PDT
                 Re: [PATCH] Fix for blame -g incorrect revisions Lieven Govaerts <svnlgo at mobsol dot be> Lieven Govaerts <svnlgo at mobsol dot be> 2009-05-13 23:19:58 PDT
                     Re: [PATCH] Fix for blame -g incorrect revisions alwood Alan Wood 2009-05-14 00:56:29 PDT
                     Re: [PATCH] Fix for blame -g incorrect revisions alwood Alan Wood 2009-05-14 01:50:24 PDT
                         Re: [PATCH] Fix for blame -g incorrect revisions gavinbaumanis Gavin Baumanis 2009-05-25 03:18:17 PDT
                             Re: [PATCH] Fix for blame -g incorrect revisions Lieven Govaerts <svnlgo at mobsol dot be> Lieven Govaerts <svnlgo at mobsol dot be> 2009-05-25 07:01:26 PDT
     Re: [PATCH] Fix for blame -g incorrect revisions alwood Alan Wood 2009-04-24 15:17:56 PDT
         Re: [PATCH] Fix for blame -g incorrect revisions gavinbaumanis Gavin Baumanis 2009-05-01 04:34:21 PDT
             RE: Re: [PATCH] Fix for blame -g incorrect revisions webpost at tigris dot org webpost at tigris dot org 2009-05-01 13:56:49 PDT
     RE: [PATCH] Fix for blame -g incorrect revisions webpost at tigris dot org webpost at tigris dot org 2009-04-27 03:21:56 PDT
Messages per page: