2824: post-review does not detect renames

matthew********@kitwa****** (Google Code) (Is this you? Claim this profile.)
Dec. 13, 2012 
What version are you running?
RBTools 0.4.2

Not sure how to easily reproduce this... for git repositories, post-review uploads diffs with deleted and created files when renames are non-trivial (i.e. 'git diff' with no options does not show a rename). I think post-review should be passing '-C -C' to 'git diff' to do better rename detection.
chipx86
#1 chipx86 
This requires rename support server-side, and that's only in the upcoming Review Board 1.7 release.
  • +NotABug
#2 matthew********@kitwa****** (Google Code) (Is this you? Claim this profile.) 
Ah, I thought I'd read somewhere it was in 1.6.x also. I guess this means post-review should have a way of knowing the server version, since not everyone will upgrade immediately. Is there a good way to do that? Alternatively, could post-review read e.g. a git configure option whether or not to use rename detection?

I hit this on a diff that was primarily a refactoring, that would have been very hard to review without rename detection... I had to locally modify RBTools to get a reasonable diff.
#3 matthew********@kitwa****** (Google Code) (Is this you? Claim this profile.) 
Also, I have discovered that copy support seems to not exist in 1.7 rc1 either? (So I guess for now it would be -M, not -C -C...)
chipx86
#4 chipx86 
post-review checks a list of server-side capabilities to know whether or not it can do move support, and factors that in. Right now, only Git supports any of those. We never added any of this to 1.6.x though.

There's no copy support, though. That would be a good thing to add, but we should have another feature request for that so it's easier to track outside of a bug report.

Hopefully the move stuff is working correctly in 1.7rc1 though?
#5 matthew********@kitwa****** (Google Code) (Is this you? Claim this profile.) 
The server support seems to work. (I didn't poke it hard... more taking for granted that it does as I haven't seen it *not* work. I have had diffs with renames on it, though, and haven't noticed any issues.)

The problem is that post-review (RBTools 0.4.2) is not generating diffs with rename tracking.
#6 matthew********@kitwa****** (Google Code) (Is this you? Claim this profile.) 
I'm thinking something like the attached patch is what is needed, except it isn't immediately obvious how to propogate the server version to where it is needed.
  • +
    0001-support-renames-in-git.patch 
    From cda0a4afac38104e184167d578952f005c3c881d Mon Sep 17 00:00:00 2001
    From: Matthew Woehlke <matthew.woehlke@kitware.com>
    Date: Sat, 15 Dec 2012 12:21:25 -0500
    Subject: [PATCH] support renames in git
    Modify git.py to generate diffs with rename tracking, if supported by
    the server.
    Fixes http://code.google.com/p/reviewboard/issues/detail?id=2824.
    ---
     rbtools/clients/git.py | 8 ++++++--
     1 file changed, 6 insertions(+), 2 deletions(-)
    diff --git a/rbtools/clients/git.py b/rbtools/clients/git.py
    index 50abf37..7edf87d 100644
    --- a/rbtools/clients/git.py
    +++ b/rbtools/clients/git.py
    @@ -277,8 +277,12 @@ class GitClient(SCMClient):
                                      split_lines=True)
                 return self.make_svn_diff(ancestor, diff_lines)
             elif self.type == "git":
    -            return execute([self.git, "diff", "--no-color", "--full-index",
    -                            "--no-ext-diff", "--ignore-submodules", rev_range])
    +            if parse_version(server.rb_version) >= par
    
 
    

    


    
   
    •