Better handle replaced files in Subversion

Review Request #5648 — Created March 20, 2014 and submitted

Information

RBTools
master

Reviewers

A few issues noticed with how SVN client deals with replaced files. Replaced file is a file copied as a part of a directory, then removed and added from another source. E.g.

$ svn cp A B # Copy a directory
$ svn rm B/file1
$ echo > B/file1
$ svn add B/file1 # B/file1 is replaced without history
$ svn rm B/file2
$ svn cp ^/some/other/file B/file2 # B/file2 is replaced with history

First, a new file is typically displayed in one column in RB. However, the diff for B/file1 has two columns, with the left one showing the path to A/file1 - and no file text. Since A/file1 is no longer related to B/file1 after replacement, the right solution would be to just consider B/file1 a new file. To distinguish such files, find_copy_from could check the "Schedule" field in 'svn info' output. As long as a file is still a part of the directory copy (even if it is modified), it will have a "Schedule: normal" line in its svn info.

Second, if a file is replaced with history (e.g. B/file2 above), by default 'svn diff' will produce a patch against the the file copied as a part of directory (i.e. against A/file1 in the example above). Then it's massaged by post-review/rbt to replace the source of the copy (to /some/other/file in the example above). When such diff is displayed by RB, it fails to apply as RB attempts to apply it to /some/other/file while the diff was generated against A/file2.

The solution for that is to use 'svn diff --notice-ancestry', which makes the diff for unrelated files (i.e., replaced files) to be generated as delete + add. In this case, the 'add' part is generated against /some/other/file and RB displays it correctly. However, the 'delete' part needs to be special-cased in handle_renames: for it to apply, we must keep the file names as they were generated by 'svn diff'. Thus, cache both from/to lines for each file and look at the first diff hunk. If it is a complete removal of a file (@@ -1,X +0,0 @@) - keep the from/to lines intact. Otherwise, find the source of the copy and replace it in the diff as it was done before.

Third, the status for the replaced files is displayed as "R +" or "RM +". SVN client only checks for "A +" at this time when it verifies whether the patch being posted has files with history. Replace that check with just checking the column 4 for '+' character.

Tested on 0.5.7, then patch applied to 0.6.0. I would appreciate if you could test it on 0.6.0.

david
  1. Ship It!

  2. 
      
ST
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (d556231). Thanks!
Loading...