Hi Lieven,
On 14 May 2009 at 8:19, Lieven Govaerts wrote:
> 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?
For me to do the real test I need to apply the patch to 1.6.x and rebuild TortoiseSVN.
I will let you know how I get on.
> 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.
I have convinced myself that merged_revision can not be invalid
now so the existing code will always work. So making a test to show
that a change is required should not be possible
However, this is only because the invalid revision numbers are all less than 0.
Which ever way you look at it my patch is wrong here. The best that
I can see at the moment would be
- if (merged_revision < revision)
- {
+ /* if revision is invalid that means local mods so don't blame a merge */
+ if (SVN_IS_VALID_REVNUM(revision) && merged_revision < revision)
+ {
The invalid revision numbers only seem to enter the system when
there are local mods and I'm not convinced that using an invalid
revision number is the best way of reporting that back. I'm not putting
my hand up to change the client interface here though.
>
> thanks.
>
> Lieven
>
> >> > 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.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2240565
> >
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2252466
Received on 2009-05-14 09:56:46 CEST