3887: Perforce diff submission is broken in 2.0.17

erik*****@gmai***** (Google Code) (Is this you? Claim this profile.)
June 20, 2015
3889, 3903
What version are you running?
ReviewBoard 2.0.17 and RBTools 0.7.2 and 0.7.4

What's the URL of the page containing the problem?


What steps will reproduce the problem?
1. Attempt to post any change via perforce


What is the expected output? What do you see instead?
I expect the post to succeed. Instead, I get a 500 error in the client:

Generating diff for pending changeset 179888
ERROR: Error validating diff

HTTP 500


And the error in the server is:
[...]
 File "/usr/local/lib/python2.7/dist-packages/ReviewBoard-2.0.17-py2.7.egg/reviewboard/webapi/resources/validate_diff.py", line 144, in create
   save=False)

 File "/usr/local/lib/python2.7/dist-packages/ReviewBoard-2.0.17-py2.7.egg/reviewboard/diffviewer/managers.py", line 156, in create_from_upload
   save=save)

 File "/usr/local/lib/python2.7/dist-packages/ReviewBoard-2.0.17-py2.7.egg/reviewboard/diffviewer/managers.py", line 180, in create_from_data
   check_existence=(not parent_diff_file_contents)))

 File "/usr/local/lib/python2.7/dist-packages/ReviewBoard-2.0.17-py2.7.egg/reviewboard/diffviewer/managers.py", line 287, in _process_files
   copied=f.copied)

 File "/usr/local/lib/python2.7/dist-packages/ReviewBoard-2.0.17-py2.7.egg/reviewboard/scmtools/perforce.py", line 332, in parse_diff_revision
   filename, revision = revision_str.rsplit('#', 1)

ValueError: need more than 1 value to unpack


What operating system are you using? What browser?
N/A

Please provide any additional information below.

Looking at revision_str, it doesn't contain any '#' character and is actually of the form: "2015-06-16 11:39:35" when called with f.newFile and f.newInfo. When called with f.origFile and f.origInfo, parse_diff_revision() would work as expected but we bomb out before getting there.
david
#1 david
How are you creating your diff? Having a diff that contains a timestamp rather than a file revision isn't going to work, and isn't a regression.
  • +NeedInfo
david
#2 david
Also, what version of perforce on the client and the server?
#3 erik*****@gmai***** (Google Code) (Is this you? Claim this profile.)
I'm using `rbt post'. I tried two versions, and 0.7.2 was confirmed to work with the old 2.0.15 Server install before I upgraded. Upgrading to 2.0.17 was the only change which is why I believe it is a regression in RBServer. I feel https://github.com/reviewboard/reviewboard/commit/8c15f47ab2f29b91e3fc4ef4b62df813543bc750 is the regression because in the past, we never called parse_diff_revision() with f.newInfo, only f.origInfo which has the expected syntax.

Perforce versions are 2015.1 client and server: 2014.2
david
#4 david
Thanks for the info!
  • -NeedInfo
    +New
brennie
#6 brennie
A fix for this has landed on release-2.0.x as 6cc72eb8c3ee7a219b5d7656aa791b2feb976223. This will ship in Review Board 2.0.18.
  • -New
    +Fixed
brennie
#7 brennie
It is actually fixed in cb9a791e722c161d7e06f1add407e180aa025586. I copied the wrong revision.