1235: More intelligent handling of rename operations.

josh.h*******@gmai***** (Google Code) (Is this you? Claim this profile.)
Jan. 18, 2014
1089 
board.org/


What version are you running?
1.0

What's the URL of the page this enhancement relates to, if any?
Any review request.

Describe the enhancement and the motivation for it.
Most SCM systems have some concept of a rename operation:
'svn mv'
'p4 integrate/p4 delete'
'git mv'
These operations generally end up being shown as a deleted file/added file
by ReviewBoard (and most code review systems in general).  It would be very
slick if there was sufficient intelligence to detect this situation and
only show differences between the files rather than the add/delete.

What operating system are you using? What browser?
N/A

Please provide any additional information below.
david
#1 david
david
#2 david
  • +Component-DiffViewer
#3 gm***@cryp***** (Google Code) (Is this you? Claim this profile.) 
Note that for the git world a good starting point is 'git diff --full-index -M master'. As well as mentioning renames, it actually gives the diff between the before- and after-move paths. (Currently in v1.5, you can't upload such a diff into reviewboard: it sees a diff between "a/old/path" and "b/new/path", and complains that "old/path (revision )" doesn't exist in the repository.)

I currently have a 600kB diff in review that reduces to 100kB with -M. It involves moving some 40 files, each with small differences as well as the move (primarily the class name).

Resolving this may also link to <a href="http://code.google.com/p/reviewboard/issues/detail?id=1066">issue 1066</a>.
#4 Carl.va*******@gmai***** (Google Code) (Is this you? Claim this profile.) 
Mercurial can also provide rename information in the patch when doing "hg diff -g" to give git style diffs.
david
#5 david 
We actually support showing renames with mercurial, now.
#6 Carl.va*******@gmai***** (Google Code) (Is this you? Claim this profile.) 
Can you elaborate?
Uploading a git-style patch generated by 'hg diff -g' causes exceptions when trying to view the diff.
Uploading a standard diff doesn't get renames picked up.
I'm running reviewboard "1.6 RC2"
#7 shu****@gmai***** (Google Code) (Is this you? Claim this profile.) 
there is still an issue with pure file rename in mercurial
http://code.google.com/p/reviewboard/issues/detail?id=2184
#8 Carl.va*******@gmai***** (Google Code) (Is this you? Claim this profile.) 
The review I am trying has both rename and modifications so I don't think its the same issue. I see that thread mentions using 'hg export -g', which I wasn't doing.

Note: the export in that thread appears to be for a single commit. We typically do reviews on a branch that is going to merge, having multiple commits.
Thus I tried for example: 'hg export -g tip:50324 > merge-review.patch'

Reviewboard won't even allow that resultant patch-file to be uploaded saying file of first change in the patch can't be found.
#9 Carl.va*******@gmai***** (Google Code) (Is this you? Claim this profile.) 
Ok, so I rediscovered that adding the #NoteID / #Parent Id to the git-diff patch allows reviewboard to get this needed info. Also found a reviewboard mercurial extension that does the same. I'll be making my own simple extension that just does the patch generation.
#10 Carl.va*******@gmai***** (Google Code) (Is this you? Claim this profile.) 
It seems this issue is actually fixed in at least 1.6 RC2.
uploading a patch as: 'git diff --full-index -M' works fine.

The outstanding issue is handling renames without modify, add and delete of empty files. I've comments put in Issue 2184.
#11 Carl.va*******@gmai***** (Google Code) (Is this you? Claim this profile.) 
Here's a sample mecurial extension that generates the required diff format for reviewboard including renames.
It's a quick hack which needs cleanup.
Also supports generating 'parent diff' if a review is based on a yet to be merged previous review.
  • +
    gdiff.py 
    '''Generate diffs with git sytle suitable for reviewboard
    Quck and dirty hack, by Carl van Schaik - Open Kernel Labs.'''
    from mercurial import ui, util, node, patch, diffopts
    from mercurial.i18n import _
    import os
    def gen_diff(ui, repo, n, p):
        '''return a diff for the specified node->parent'''
        diff = ""
        diff += "# Node ID " + node.hex(n.node()) + "\n"
        diff += "# Parent  " + node.hex(p.node()) + "\n"
        opts = patch.diffopts(ui, opts={'git': True})
        for chunk in patch.diff(repo, p.node(), n.node(), opts=opts):
            diff += chunk
        return diff
    def do_gdiff(ui, repo, rev='.', **opts):
        '''Generate a patch with Git syntax but suitable for reviewboard.
        A diff containing changes and rename information is generated. Output is by
        default to stdout, unless a filename is specified for the patch to be
        written to.
        An optional 'parent' patch may be created if an intermediate revision is
        specified. This is useful when a feature is based of anoth
    
 
    

    


    
   
    • 


  


 








 

   

 
 

  

    
     

   
   #12
   gi***@ypc*** (Google Code)



(Is this you? Claim this profile.)


  


  
I try to generate "git diff -M --full-index revA revB" but the patch is not accepted by RB with error:

The file 'path/to/fileB' (r) could not be found in the repository

Where could be the problem? I'll try to add # Node ID/ # Parent at the top of diff file but no success (same result), if I try to prepend before line Similarity index 100% a line index revA..revB 100644 the error is only more concrete:

The file 'path/to/fileB' (revA) could not be found in the repository

I suppose that fileB should be searched in revB not revA.

Thank for any advice (We use RB v1.6.4.1).



 








 

  

   david

  

 
 

  

    
     

   
   #13
   david


  


  
This is generally fixed for most SCMs when using rbtools to generate the diff.



  
    


   
  • 
    
    
    +Fixed
    
   
    •