3433: CVS file path stored incorrectly in database
- Fixed
- Review Board
| nick.m*******@rockwellc********** (Google Code) (Is this you? Claim this profile.) | |
 |
|
| June 26, 2014 |
What version are you running? Reviewboard 2.0.2 RBTools 0.6 What steps will reproduce the problem? 1. Use rbt to post a new review with diffs from a file already committed in CVS. 2. Look at the diffs in your outgoing review request in the web browser. What is the expected output? What do you see instead? I expect to see the diff between the two files (as shown with a simple CVS diff), but instead I get the following error: There was an error displaying this diff. The file 'home/cvsroot/tiger/code/include/support/Attic/PlatformSpecific.h' (r1.2) could not be found in the repository This may be a bug in the software, a temporary outage, or an issue with the format of your diff. Please try again, and if you still have trouble, contact support. What operating system are you using? What browser? Ubuntu 14.04 on the client Ubuntu 12.04 on the server Google Chrome browser. Please provide any additional information below. The issue appears to be that the path to the file is missing a leading '/' when stored in diffset entry in the database (i.e. /home/cvsroot/...) . When reviewboard gets the file list from the database and attempts to post the diff, it can't find the original file, so it searches the CVS attic for the file instead (and also can't find it). We had been using reviewboard 1.6.5 recently with CVS and it seemed to work fine. I figured that I would post the issue here before I start digging into the code too much myself.
hi, i got the same issue. It's good I checked it's already reported. Here is a dirty workaround to unblock you while waiting for the fix:
def get_file(self, path, revision=HEAD):
logging.debug("get_file: ___path=[%s]", path)
if not path:
raise FileNotFoundError(path, revision)
#this is the fix:
if not path.startswith("/"):
path = '/' + path
#end of fix
return self.client.cat_file(path, revision)
the file is cvs.py in /usr/local/lib/python2.7/dist-packages/ReviewBoard-2.0.2-py2.7.egg/reviewboard/scmtools (see exact path on your server)
Thanks! Actually have the exact same hack for now and it seems to be doing the trick.
So the CVS server and RB are on the same box, with the repository configured as a local path?
-
+ NeedInfo
hi, in my case they are on separate servers. RBT used to work very good and I was able to see created review requests and their diffs. When I updated to 2.0.2 (from 1.6) it stopped working for new requests (no diff displayed and error instead) but the old requests were still OK. I noticed that the RB server tries to checkout the file every time and the path used has missing '/' at the beginning.
We do normalize paths now to work around issues elsewhere. Seems like CVS broke along with that. Would you be able to supply a formal patch + unit test for review? We can get a fix into the following 2.0.x.
Hi, to be honest I am new in contributing but I can try. Do you think the 'fix' in https://code.google.com/p/reviewboard/issues/detail?id=3433#c1 is OK to be proposed for patch? As for unit test I am not sure what to do :(
I think the fix is reasonable. You'd need a development environment to confirm the unit test suite and to write a new test. It'd take a familiarity with Python. If you're not comfortable, maybe someone else who's paying attention can give it a shot. I'd be happy to, but I have a full week.
hi, just I did two screenshot before and after the change proposed for the same review request. Btw there is a detailed error stack. If needed, please, let me know to paste it here.
-
+ 
+ 
-
- NeedInfo + PendingReview -
+ Component-SCMTools -
+ david