821: Updating a diff with a diff + parent diff does not work.

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
chipx86
#1 chipx86 
What version/nightly are you using? Both Review Board and Djblets.
  • +NeedInfo
#2 j**@multan****** (Google Code) (Is this you? Claim this profile.) 
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 ...
#3 j**@multan****** (Google Code) (Is this you? Claim this profile.) 
I forgot to say that we are using Review Board with Mercurial repositories (Mercurial
1.1.2).
#4 j**@multan****** (Google Code) (Is this you? Claim this profile.) 
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.
#5 j**@multan****** (Google Code) (Is this you? Claim this profile.) 
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...
david
#6 david
  • -NeedInfo
    +New
david
#7 david
  • +Component-DiffParser
    +Component-SCMTools
chipx86
#8 chipx86 
The error display part of this bug is fixed on master (ab7ae7d)
david
#9 david 
It actually looks like this is entirely fixed now.
  • -New
    +Fixed