2971: Can't view Mercurial diffs with parent diffs in recent versions
- Fixed
- Review Board
| c.ca*****@gmai***** (Google Code) (Is this you? Claim this profile.) | |
| Aug. 5, 2013 |
What version are you running? 1.7.7.1 (the problem did not exist in 1.7.5) What's the URL of the page containing the problem? http://reviews/r/4/diff/#index_header What steps will reproduce the problem? hg init echo Hello World > file1 echo Lorem Ipsum > file2 hg add file1 file2 hg commit -m "Added files" [Clone this repo to a master location that ReviewBoard can see, and set up a repo pointing at it] echo Line2 > file1 hg commit -m "Modified file1" echo Line2 > file2 hg commit -m "Modified file2" hg postreview [Open the uploaded draft review and click "View Diff"] What is the expected output? What do you see instead? Expected: Diff shown normally. Actual: Under the modified file an error message is shown (see attached screenshot) What operating system are you using? What browser? RB server is on Linux, client on Windows. Tried with Chrome and Firefox.
Forgot to say - the repro steps use the latest version of the hgreviewboard extension: https://bitbucket.org/ccaughie/hgreviewboard.
I've investigated further; the issue was introduced by this changeset here: https://github.com/reviewboard/reviewboard/commit/562a8b4a21a932c590551e3164d0679f46f75445 I've got it working again in our environment by not passing the limit_to argument to _process_files at line 108 of diffviewer/forms.py. I believe the problem is that with Mercurial this prevents the source_revision field of the FileDiff from being set correctly for any files that are present in the main diff but not in the parent diff. I'm guessing this change was made with git in mind. Git diffs are a little different from Mercurial diffs (even Mercurial "git-style" diffs) in that (afaik) they record the original revision ID *of every file in the diff*. That means that if the parent diff doesn't mention a file, the original version of that file can be found without even looking at the parent diff. Mercurial diffs (and revision numbers for that matter) are different. First remember that in Mercurial there are no file revisions, only whole-repo revisions. A particular revision of a file is identified by the changeset that introduced the new file revision. Therefore a Mercurial diff only identifies the repo revision it is based on, not the revisions of each file within it. In the case of a diff/parent_diff pair, the source revision of the main diff will be the destination revision of the parent diff, which by definition doesn't exist in the repository that ReviewBoard can see. So, for parent diffs to work with Mercurial, the source revision ID of all files in the diff must be set to the source revision of the parent diff, regardless of whether or not they appear in the parent diff. I'm not sure of the best way to fix this. It seems like the code to handle the differences between Git and Mercurial should be in the various SCMTool subclasses, but I think that means SCMTool will have to gain methods to process parent diffs. I'll mull this over and propose a fix, in the mean time if you have any suggestions I'd be glad to hear them.
This problem does also affect Git though - see http://demo.reviewboard.org/r/12105/diff/#index_header
This doesn't appear to be fixed - I'm still seeing this problem in 1.7.13.
This particular issue was fixed (with a patch from the reporter), but there's another, similar issue open at https://code.google.com/p/reviewboard/issues/detail?id=3067