769: post-review should warn if there's no "diff" in the user's path

jeff****@gmai***** (Google Code) (Is this you? Claim this profile.)
chipx86
chipx86
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)
david
#1 david
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.
david
#2 david
  • +Confirmed
  • -Priority-Medium
    +Priority-Low
    +Component-Scripts
  • +post-review should warn if there's no "diff" in the user's path
#3 trem*****@gmai***** (Google Code) (Is this you? Claim this profile.)
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.
#5 nme***@gmai***** (Google Code) (Is this you? Claim this profile.)
++ 

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. 

#6 rpo****@gmai***** (Google Code) (Is this you? Claim this profile.)
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.
chipx86
#7 chipx86
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.
chipx86
#8 chipx86
We now give the user some useful instructions.

Fixed in 7dce756
  • -Confirmed
    +Fixed
  • -Component-Scripts
    +Component-RBTools
    +Milestone-RBTools-Release1.0
  • +chipx86