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 alwood
Full name Alan Wood
Date 2009-05-13 15:04:09 PDT
Message 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 (working 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
Attachments

« Previous message in topic | 6 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: