2256: post-review should use -c option when calling svn diff with single rev

seamus*******@gmai***** (Google Code) (Is this you? Claim this profile.)
Jan. 18, 2014
2249 
What version are you running?
1.6 RC2

What's the URL of the page containing the problem?
post-review 

What steps will reproduce the problem?
1.Point post-review to SVN repo
2.Run post-review with single --revision-range option, e.g.
    post-review --revision-range=157778 --debug 

What is the expected output? What do you see instead?

Expected debug output: 
----------------------
>>> RBTools 0.3.3
>>> Home = /home/<snip>
>>> svn info
>>> diff --version
>>> repository info: Path: http://<snip>/repo, Base path: /<snip>/trunk, Supports changesets: False
>>> HTTP GETting api/
>>> HTTP GETting http://<snip>/reviews/api/info/
>>> Using the new web API
>>> svn diff --diff-cmd=diff -c 157778

^CTraceback (most recent call last):
<snip>
KeyboardInterrupt 

Actual debug output:
--------------------
>>> RBTools 0.3.3
>>> Home = /home/<snip>
>>> svn info
>>> diff --version
>>> repository info: Path: http://<snip>/repo, Base path: /<snip>/trunk, Supports changesets: False
>>> HTTP GETting api/
>>> HTTP GETting http://<snip>/reviews/api/info/
>>> Using the new web API
>>> svn diff --diff-cmd=diff -r 157778

^CTraceback (most recent call last):
<snip>
KeyboardInterrupt 

What operating system are you using? What browser?
Ubuntu 9.10

Please provide any additional information below.

The documentation states "If you only need to post a single revision, you can type:
$ post-review --revision-range=REVISION"

However, it uses the -r option with svn diff, which is equivalent to 0:157778, and begins fetching EVERY change made on the current path, up to rev 157778 in
this example. It should use the -c option with svn diff.
#1 curtis.********@gmai***** (Google Code) (Is this you? Claim this profile.) 
Is this as simple as changing the flag "-r" to "-c"?

Line 150 of /rbtools/rbtools/clients/svn.py

        # Otherwise, perform the revision range diff using a working copy
        else:
            return (self.do_diff(["svn", "diff", "--diff-cmd=diff", "-r",
                                  revision_range],
                                 repository_info), None)
#2 seamus*******@mformat******* (Google Code) (Is this you? Claim this profile.) 
Not quite, you need to support both cases, where single revision is listed and where a range is listed, so something like:

        # Otherwise, perform the revision range diff using a working copy
        else:

            # use -c if single rev supplied, else -r if range supplied
            revisions = revision_range.split(':')
            if len(revisions) == 1:
                return (self.do_diff(["svn", "diff", "--diff-cmd=diff", "-c",
                                  revision_range],
                                 repository_info), None)
            else:
                return (self.do_diff(["svn", "diff", "--diff-cmd=diff", "-r",
                                  revision_range],
                                 repository_info), None)
david
#3 david
david
#4 david
  • +Started
  • +Component-RBTools
  • +david
david
#5 david 
Actually, this is probably suitable for a starter bug for a student.
  • -Started
    +Confirmed
  • +EasyFix
  • -david
    +-**@gmai***** (Google Code)
david
#6 david 
This is fixed in rbtools master (will ship in 0.6)
  • -Confirmed
    +Fixed