451: svn switch'd directories not handled correctly by post-review & server
- Fixed
- Review Board
| daniel.d********@gmai***** (Google Code) (Is this you? Claim this profile.) | |
| April 22, 2008 |
happens when I try to use post-review What steps will reproduce the problem? 1. Create an svn checkout of some subtree of the repos, with an empty stub directory in it called e.g. "stub" 2. Ensure there is another folder elsewhere in the repo (e.g. /somewhere) which contains a file "x.txt" 3. svn switch http://localhost/svn/somewhere stub (or similar, depending on the base url of your repository) 4. Modify "stub/x.txt". svn diff would name the file "stub/x.txt". 5. Attempt post-review. What is the expected output? What do you see instead? Expected output: (normal success) Actual output: Error uploading diff: The file was not found in the repository (207) What operating system are you using? What browser? debian etch (client & server) Please provide any additional information below. I'm pretty sure this is because the server is trying to look for the file "stub/x.txt" which does not exist, without realising that "stub" is actually switched to some other part of the repository. It can't know this just from the diff... post-review would need to upload additional information regarding the switching perhaps?
I've only just started using this software, so please forgive me if i have a poor understanding of the code base. From taking a look at it, it seems that this should be a simple fix from the server's point of view if the file names specified in post-review's diff were absolute (not including the server, but including the full path from the absolute root of the tree). This would be a bit more work for post-commit, but from what i can tell, the server would only have to not add the basedir at the front of the filename when checking for existance, (and hopefully in not too many other places). svn info gives the full url of a file in a checkout, so this should be easy to ascertain.
SvnRepositoryInfo doesn't properly handle determining relative paths. The initial version posted in the review (http://reviews.review-board.org/r/351/) was correct, but then it was changed to incorrectly compare the pathdirs and rootdirs lists directly in r3 of the diff. List comparison checks that the lists are the same, which means they are they same length and every element is the same.
This is complicated by the fact that SvnClient always creates absolute paths for the diff, so even when the relative path calculation is fixed in SvnRepositoryInfo the diff is sent with absolute paths and a (now incorrect) basedir specified.