Summary

Mercurial: add support for copy/rename handling via git-style diffs

Review Request #1937 — Created Nov. 21, 2010 and submitted

Information

Owner

ccaughie

Repository

Review Board

Branch

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Mercurial: add support for copy/rename handling via git-style diffs

This adds support for the git-extended diff format used by Mercurial. This
includes information about copies and renames, allowing the ReviewBoard diff
view to show content diffs even if a file has been renamed by the changeset
being reviewed.
N.B. The uploaded diff files must include the "Node ID" and "Parent" headers as
generated by "hg export". This is the only way ReviewBoard can know the
original and new changeset IDs.

Testing Done

Posted a large number of git-style diffs to our production RB server (which has this patch applied) over the course of several months.
Some of these have included renamed files, many include added and deleted files.
Verified that in all cases the renames are shown correctly (with the patch in review request #1944 also applied) and everything else works as before.
Regular Mercurial diffs have also been tested extensively with this patch applied (many of our developers have not updated their configuration to use git-style diffs), with no observed change in behaviour.

Change Summary:

Turns out I had underestimated the complexity of git diffs. This second diff handles more of the possible cases. Tested with adding files, deleting files, binary files.

Diff:

Revision 2 (+70 -12)

Show changes

reviewboard/scmtools/hg.py

Sorry for the delay on this; we've been pretty swamped with stuff. I'm not entirely sure how to review this, since the Hg implementation is entirely 3rd-party contributed. I'd feel a lot better if you had the testing done section filled out, and even better if there were unit tests for this.

ccaughie

March 21, 2011, 5:18 p.m.

Sorry - I've updated the testing done section now. The advantage of the long delay is that the patch has now had several months of testing!
I'll try to find time to write some unit tests - I agree those would be a good thing.

david

March 21, 2011, 5:36 p.m.

If it's been in production for months, that makes me much more willing to take the change. If you do find the time for unit tests, I'd love to have them as a separate change, but I'm going to push this now.

david

March 21, 2011, 5:39 p.m.

Pushed to master as 044afe1. The next release that will have this change will be 1.6 beta 2. Thanks!

Testing Done:: +
Posted a large number of git-style diffs to our production RB server (which has this patch applied) over the course of several months.
+ Some of these have included renamed files, many include added and deleted files.
+ Verified that in all cases the renames are shown correctly (with the patch in review request #1944 also applied) and everything else works as before.

Testing Done:: Posted a large number of git-style diffs to our production RB server (which has this patch applied) over the course of several months.
Some of these have included renamed files, many include added and deleted files.
~ Verified that in all cases the renames are shown correctly (with the patch in review request #1944 also applied) and everything else works as before.
~ Verified that in all cases the renames are shown correctly (with the patch in review request #1944 also applied) and everything else works as before.
+ Regular Mercurial diffs have also been tested extensively with this patch applied (many of our developers have not updated their configuration to use git-style diffs), with no observed change in behaviour.

	+	Posted a large number of git-style diffs to our production RB server (which has this patch applied) over the course of several months.
	+	Some of these have included renamed files, many include added and deleted files.
	+	Verified that in all cases the renames are shown correctly (with the patch in review request #1944 also applied) and everything else works as before.