769: post-review should warn if there's no "diff" in the user's path
- Fixed
- Review Board
| jeff****@gmai***** (Google Code) (Is this you? Claim this profile.) | |
 |
|
| March 29, 2010 | |
| 1378, 1530 |
I'm using version 0.8 of post-review.py on WinXP.
The following lines gave us an issue:
return (self.do_diff('svn diff --diff-cmd=diff %s' % ' '.join(files)),
return self.do_diff('svn diff --diff-cmd=diff -r %s' % revision_range)
Maybe there's a good reason for using --diff-cmd=diff, but I don't know what
it is. This looks for a program called diff in your path instead of using
svn's diff ability. One developer didn't have cygwin installed and post-
review failed.
We fixed it with the following:
return (self.do_diff('svn diff %s' % ' '.join(files)),
return self.do_diff('svn diff -r %s' % revision_range)
svn diff doesn't give us a usable diff if there are moved or copied files. We can probably look for "diff" in the path and show a useful error message, though.
-
+ Confirmed -
- Priority-Medium + Priority-Low + Component-Scripts -
+ post-review should warn if there's no "diff" in the user's path
I think (if it doesn't already) the documentation needs to state that Windows installations need an external diff tool to be available in order for post-review to work.
++ Its not a given that every system will have a diff binary in their path. On our team a couple of us edit in windows over smb shares, but the development environment is on linux. Thus no cygwin, nor any other reason there would be a diff.exe in my path on my windows laptop. and when you hit this error, what happened in our cases is that the developers in question abandoned reviewboard entirely.
This needs to be made a configuration option. The default diff util in my default path is a crappy old version that is not fully-featured enough to work with svn and reviewboard.
The plan after the RBTools 0.2 release is to bundle a custom diff tool that we can use instead of GNU diff for this.
We now give the user some useful instructions. Fixed in 7dce756
-
- Confirmed + Fixed -
- Component-Scripts + Component-RBTools + Milestone-RBTools-Release1.0 -
+ chipx86