3067: Parent diffs with mercurial are still broken
- Fixed
- Review Board
| glin****@dynamicco********** (Google Code) (Is this you? Claim this profile.) | |
| Jan. 18, 2014 |
What version are you running? RB 1.7.13 What's the URL of the page containing the problem? n/a What steps will reproduce the problem? 1. create new local repo with three changeset, see attachment 2. create new empty central repository, linked to RB 3. push up revision 0 to central repository (don't push up r1 and r2) 4. my intention is to use RB to review changes between rev 1 and rev 2: 5. create parent diff: hg diff -r0 -r1>parent.diff 6. create diff to review: hg diff -r1 -r2>target diff 7. create new review in RB, upload parent.diff & target.diff 8. When clicking "Create Review Request" RB responds with error: The file 'f1' (r5f714bed7dcc) could not be found in the repository What is the expected output? What do you see instead? - I expect RB to create a new review, instead it returns a fault. RB starts looking for file 'f1' (r5f714bed7dcc) in the central repository, but it doesn't exist. This is logical because the listed revision is not in th central repository. What operating system are you using? What browser? - tried it from Windows XP and Debian Linux, both give the same result - browser is Iceweasel Please provide any additional information below. - I created the parent and target diff manually to exclude any tool involvement
I'm not sure that revision 0 was properly pushed to the repository. According to this: $ hg diff -c 1 diff -r 5f714bed7dcc -r ec32bb492cac f1 --- a/f1 Thu Aug 22 07:12:46 2013 +1200 +++ b/f1 Thu Aug 22 07:14:14 2013 +1200 @@ -0,0 +1,1 @@ +xxx revision 1 starts with f1 at 5f714bed7dcc and ends at revision ec32bb492cac. I'm not sure how to get hg to tell me what revision the files are at which changes, but it seems to me like 5f714bed7dcc was introduced at revision 0.
I made a mistake when creating a new review request. I used the wrong main repository :-( When I use the correct repository, RB creates the review request and it goes wrong when I want to look at the diff. The diff for f1 is OK, the diff for f2 gives an error: Error: The file 'f2' (rec32bb492cac) could not be found in the repository. See images attached.
-
+ +
-
+ Parent diffs with mercurial are still broken
This bug is extremely aggravating for us. It appears to happen whenever the parent diff does not contain the file that is being changed in the diff to be reviewed. For instance, parent diff made changes to files A, B Uploaded diff makes changes to files B, C B will display correctly, C will throw an error. We figured out how to work around it, by using a parent diff that goes from 0:desireddiff-1; for example: we want to upload revision 5, then we would do: hg export 5 > diff.patch hg diff -r 0:4 > parent.patch (or null:4) and upload those, and that always seems to work. Unfortunately, this means we are unable to use postreview in most cases.
Thanks for that. Two remarks on your work-around: - you can still use postreview. Just use both the --master and --parent options - when using an earlier changeset for the parent, the changeset can become very big. When using changeset zero, it includes the whole change history. Another option is to manually patch the changeset (diff.patch). I you change the changeset id (in the diffs that go wrong) to the id of the last common parent, it works. Reviewboard can find this changeset and perform the diff correctly. This works because the file hasn't changed in the changesets between the actual parent and the last common parent.
For postreview, I'm using the -o option in combination with -g and it is enough to work around this bug. -o will use a parent diff based on the upstream repo -g submits the review in a git format For some reason using the git format fixes the "Diff currently unavailable" bug for me.
I can't seem to figure out a permutation of --master and --parent to get it to work. file added in revision 0, trying to review revision 3. master parent result 0 2 issue present 2 0 abort: The file was not found in the repository (207) 0 0 gives the appearance that all the files in the repo are new However, the recently posted -o -g work-around does appear to work for me as well.
Possible related issue, but it may be a different bug. I attempted to use the -o -g workaround and have encountered another edge case that fails: r4 [draft] : add file A (unrelated in content, but sharing the same name) r3 [draft] : miscelaneous r2 [draft] : remove file A r1 [public] : file A present hg postreview r4 -o -g -i 'foo' File A will show 'diff unavailable' My original workaround (diff -r 0:r3) still worked in this scenario. I have not tested to see if r3 is necessary to the test. Since the -o -g workaround seemed to work because it was using git diffs (and thus likely a different code path), this case should probably be tested with a git repository as well to ensure that it doesn't exist there.
I'm using RB 1.7.12 and I had a slightly different problem with a simpler solution (when using hg-style diffs). The old code allowed the 'origChangesetId' from any file in the parent diff to become the source_rev for every file in the child diff; however it only worked if the parent diff had any files in common with the child. My attached patch fixes this. While the code in 1.7.12 was not 100% correct, this effectively solved the issue. If you try with 1.7.12 and the attached patch, the problem should be solved. I also had no issues when removing files and re-adding them back--this should be handled by the "f.origInfo != PRE_CREATION" check. Perhaps it is a bug with git-style diffs. I have yet to test in RB 1.7.13, but looking at the commit log, it looks like the code changed significantly between 1.7.12 and 1.7.13 with https://reviews.reviewboard.org/r/4121/ (45faed44) -- From what I can tell, the code now will only work in the case that you specify a parent diff with a global "# Parent" comment because it now only checks the global origChangesetId, so I think this change just made the issue more complicated. The good news is that with commits 253ff68 and 5acf49a it is now possible to specify the parent commit ID via the API, so the API parameter might be a nice solution to this problem in versions after 1.7.13. I will try to do some tests on 1.7.13 or a later version. I also have only tested with hg-style diffs, not git-style diffs so those might be different.
-
+
-
+ Component-RBTools
This should now be completely fixed in rbtools master as of commit 5a93bb8. This will ship in rbtools 0.6
-
+ Fixed