RB 2.0.17 - Interdiff fails to highlight deleted code

[Paul Fee]

Hi all,

I'm using ReviewBoard 2.0.17 and see unexpected behaviour in the diff viewer.

Steps to reproduce:

1. Change a few files (I'm using SVN)
2. rbt post <changelist>
3. Publish review
4. Delete a group of lines from one of the files already changed.
5. rbt post -r <ID> <changelist>
6. Publish review
7. Review entire diff: http://reviewboard/r/9581/diff/2/ - Result: PASS
8. Review first interdiff: http://reviewboard/r/9581/diff/1/ - Result: PASS
9. Review second interdiff: http://reviewboard/r/9581/diff/1-2/ - Result: FAIL

In the second interdiff, RB states that the file contains only whitespace changes, this is incorrect as lines have been deleted.

Expanding the entire file, the contain on the left and right diff panels is correct, I can see that the deleted lines have gone, however they're not highlighted in red as expected.

I don't think this is related to caching as the following steps had no effect, the second interdiff consistently shows the same result.

* systemctl restart memcached
* systemctl restart httpd
* View second interdiff with different browsers (Firefox and Chromium), both show same results, hence not a browser cache issue.

I'm running ReviewBoard on CentOS7 using EPEL packages.

Let me know if you need more information to help recreate or fix this bug.

Thanks,
Paul

[Christian Hammond]

Hi Paul,

We recently fixed a bug like this, but perhaps there's another issue somewhere.

In order to diagnose this, I'll need to have original copies of the affected files, as well as both diffs. I won't be able to diagnose without those, unfortunately.

Christian 

--
Christian Hammond - chri...@beanbaginc.com
Review Board - https://www.reviewboard.org
Beanbag, Inc. - https://www.beanbaginc.com

-----Original Message-----
From: Paul Fee <paul....@gmail.com>
Reply: revie...@googlegroups.com <revie...@googlegroups.com>>
Date: June 30, 2015 at 4:02:10 AM
To: revie...@googlegroups.com <revie...@googlegroups.com>>
Subject:  RB 2.0.17 - Interdiff fails to highlight deleted code

> Hi all,
>
> I'm using ReviewBoard 2.0.17 and see unexpected behaviour in the diff
> viewer.
>
> Steps to reproduce:
>
> 1. Change a few files (I'm using SVN)
> 2. rbt post

> 3. Publish review
> 4. Delete a group of lines from one of the files already changed.
> 5. rbt post -r

> 6. Publish review
> 7. Review entire diff: http://reviewboard/r/9581/diff/2/ - Result: PASS
> 8. Review first interdiff: http://reviewboard/r/9581/diff/1/ - Result: PASS
> 9. Review second interdiff: http://reviewboard/r/9581/diff/1-2/ - Result:
> FAIL
>
> In the second interdiff, RB states that the file contains only whitespace
> changes, this is incorrect as lines have been deleted.
>
> Expanding the entire file, the contain on the left and right diff panels is
> correct, I can see that the deleted lines have gone, however they're not
> highlighted in red as expected.
>
> I don't think this is related to caching as the following steps had no
> effect, the second interdiff consistently shows the same result.
>
> * systemctl restart memcached
> * systemctl restart httpd
> * View second interdiff with different browsers (Firefox and Chromium),
> both show same results, hence not a browser cache issue.
>
> I'm running ReviewBoard on CentOS7 using EPEL packages.
>
> Let me know if you need more information to help recreate or fix this bug.
>
> Thanks,
> Paul
>

> --
> Supercharge your Review Board with Power Pack: https://www.reviewboard.org/powerpack/
> Want us to host Review Board for you? Check out RBCommons: https://rbcommons.com/
> Happy user? Let us know! https://www.reviewboard.org/users/
> ---
> You received this message because you are subscribed to the Google Groups "reviewboard"
> group.
> To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[Peter Hodgson]

Hi Chris,

I just came across this same bug in 2.5.12 exactly as described by Paul so I'm presuming it wasn't fixed.

What files do you need to debug this? I'll see if I can reproduce this without distributing our actual codebase.

Thanks,

Peter

[Peter Hodgson]

Hi Chris,

I've now replicated this with a very simple example.

The original file is attached as LICENSE.

I added a char and committed the change reflected in rb40.patch and created the review request (that file is created via the download patch link)

I then committed a change removing the line starting with the word LIABILITY and updated the review request, the full diff of which is the file rb40_v2.patch (again created via the download patch link).

To define the issue again:

orig - v2 Shows the correct diff, showing both the char addition and line removal

orig - v1 Shows the correct diff, showing just the line change

v1 - v2    Is nonsense, the diff claims "This file contains only whitespace changes" and shows the last 5 lines of the rev1 file vs the last four of the rev2 file. It does not highlight the missing lines.

I understand this is an old thread, if I don't hear back in a couple of days I'll raise it afresh.

Many thanks,

Peter

[Christian Hammond]

Thanks, Peter. There's still bugs here, and I've spent a lot of time reworking the algorithms to fix these. I'm not ready to ship any of that code yet, though. Needs further tweaking and testing. Having test cases like these really help with that, and I'll add the files to my regression tests.

Christian

To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Christian Hammond

President/CEO of Beanbag

Makers of Review Board

[Peter Hodgson]

Excellent, that sounds good Christian,

Thanks for the reply,

Peter

[Will Jacobs]

Hey Christian - 

Thanks as usual for the response! Just a quick question - how long has this bug been around? I recently upgraded from 1.6.14 to 2.5.12, and am seeing this as well, but I'm trying to understand if it was also a bug in 1.6.14 which we never noticed or never exhibited.

Thanks,

-Will

[Christian Hammond]

Hey Will,

It's a bug in 2.5.x and later 2.0.x releases. It wasn't present in 1.6.x or 1.7.x.

Christian

To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Will Jacobs]

Awesome, thanks again!

[Sebastian Unger]

We've just come across this (or something very similar) in 2.5.13.1.

• Is this expected (i.e. you know the bug is still there) or something new (i.e. you fixed the bug and this is something else)?
  
• Is there a bug tracker issue somewhere that I can subscribe to to get updates regarding this?
• Are the circumstances under which this happens known so I can inform my users what to look out for?
• Is there any additional testing or debugging I can help with to get this resolved?

Cheers,

Seb

[sebun...@gmail.com]

This is still an issue in 5.0.4.

[Christian Hammond]

Hi Sebastian,

Hmm, that's unfortunate. We have recently found an edge case ourselves involving the new algorithm, but need to collect more samples. Would it be possible to collect some information from you privately about the diff and its contents? We're happy to sign an NDA.

Christian

You received this message because you are subscribed to the Google Groups "Review Board Community" group.

To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/reviewboard/b4c15a02-2d7f-47b0-99d1-e49aa4c1dd9an%40googlegroups.com.