821: Updating a diff with a diff + parent diff does not work.
- Fixed
- Review Board
d**@comps***** (Google Code) (Is this you? Claim this profile.) | |
Feb. 13, 2012 |
What steps will reproduce the problem? 1. Make a review request with just a diff. 2. Try to update the diff and a parent diff. 3. Press Upload. 4. Nothing happens, no error, the upload diff box just stays there and the review request is not changed at all. What is the expected output? What do you see instead? Expected output: The upload diff box going away and thre review request being updated with the new diff. Instead: The upload diff box statys up and nothing happens when the upload button is pressed. The review request is left unchanged. Note: Update diff works fine when no parent diff is given. What operating system are you using? What browser? Server: Gentoo Linux with apache. Client: Windows Vista and Firefox 3.0.5
I can reproduce the same problem here too. We are using ReviewBoard-1.0alpha2 and Djblets-0.5alpha2. The bug is rather strange, since nothing happens on the client side and in the log server, I can see that the HTTP error code is still 200. However, _sometimes_, it works. I can't find what are differences in both cases :( We have this error for some times now. It was already there with 0.9.x ...
I forgot to say that we are using Review Board with Mercurial repositories (Mercurial 1.1.2).
Actually, using Wireshark, I can see that the server send the following reply: {"stat": "fail", "err": {"msg": "The file was not found in the repository", "code": 207}, "file": "buildout.cfg", "revision": "8a713b1f46b3"} In fact, we are using Mercurial Queue (aka. mq) to manage our patches in our repositories. If we have several patches in a queue (two patches in my case), the diff header for the first one contains the hash number of the latest commit in my Mercurial repository, which is fine. This patch is well understood by ReviewBoard. However, for the second one, as it depends on the first one, the diff header contains the hash number of the *first* patch, which doesn't exist in the repository (and which is subject to change I guess, it I update the corresponding patch). In the scenario which triggers this bug, the patch I want to update is the second one, and I want to set the first one as the parent patch. In the captured response above, the revision correspond to the revision of the first patch. And obviously, the buildout.cfg file exists in the repository (since the very first commit). I hope this can clarify the bug report. Don't hesitate to ask if you need more informations.
I trace down the problem into reviewbord.diffviewer.forms, in UploadDiffForm._process_files(). AFAIK, _process_files is called with check_existance=True, and so, the method checks that the each file of the patch exists in the revision specified in the diff file. However, the revision refers to the first patch, which is not yet commited into the repository. Since we are using Mercurial, the method get_file() of the HgTool backend raises an error: FileNotFoundError: The file 'a' (ra3347c67b46f) could not be found in the repository: unknown revision 'a3347c67b46f' ... which leads to the problem reported here. However, I'm not sure what the best solution is...