[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] diff base->repos failed to trace a moved file

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2006-03-13 00:27:23 CET

A base->repos diff of a moved file gave the wrong output.
(Subversion failed to notice the move and gave an all-lines-deleted diff.)

A reproduction recipe is:

   echo "Old" > file
   svn add file
   svn ci -m ""
   svn mv file file-new
   echo "New" > file-new
   svn ci -m ""
   svn diff -r BASE:1 file-new

(where 1 is the revision created by the initial commit)

This results in the following output:

Index: file-new
===================================================================
--- file-new (working copy)
+++ file-new (revision 1)
@@ -1 +0,0 @@
-New

The first problem fixed by this patch is that it didn't notice that it had to
trace "file-new" through history from the implied peg at WC back to revision
number 1, and so it thought the file didn't exist at r1. Fixing svn_cl__diff()
to request a "pegged" diff for a non-local end-rev as well as for a non-local
start-rev, the result is:

Index: file-new
===================================================================
--- file-new (.../file) (working copy)
+++ file-new (.../file-new) (revision 1)
@@ -1 +1 @@
-New
+Old

which is better: the diff is correct except that the parenthetical file names
are the wrong way around. This bug is fixed in
libsvn_client/diff.c:diff_repos_wc(), which was failing to observe its
"reverse" flag, and the result is then correct:

Index: file-new
===================================================================
--- file-new (.../file-new) (working copy)
+++ file-new (.../file) (revision 1)
@@ -1 +1 @@
-New
+Old

Any comments before I commit this?

- Julian

Fix a bug whereby a base->repos diff of a moved file would give wrong output.
(Subversion failed to notice the move and gave an all-lines-deleted diff.)

* subversion/libsvn_client/diff.c
  (diff_repos_wc): Swap the two paths if we're diffing in reverse.

* subversion/svn/diff-cmd.c
  (svn_cl__diff): Do a "pegged" diff if either revision is non-local, not just
    if the start revision is non-local.

* subversion/tests/cmdline/diff_tests.py
  (diff_base_repos_moved): New test for these fixes.
  (test_list): Add the new test.

Index: subversion/libsvn_client/diff.c
===================================================================
--- subversion/libsvn_client/diff.c (revision 18821)
+++ subversion/libsvn_client/diff.c (working copy)
@@ -2132,8 +2132,16 @@
                                           revision1, &end,
                                           ctx, pool));
 
- callback_baton->orig_path_1 = url1;
- callback_baton->orig_path_2 = svn_path_join(anchor_url, target, pool);
+ if (!reverse)
+ {
+ callback_baton->orig_path_1 = url1;
+ callback_baton->orig_path_2 = svn_path_join(anchor_url, target, pool);
+ }
+ else
+ {
+ callback_baton->orig_path_1 = svn_path_join(anchor_url, target, pool);
+ callback_baton->orig_path_2 = url1;
+ }
     }
   
   /* Establish RA session to path2's anchor */
Index: subversion/svn/diff-cmd.c
===================================================================
--- subversion/svn/diff-cmd.c (revision 18821)
+++ subversion/svn/diff-cmd.c (working copy)
@@ -219,9 +219,11 @@
           ? svn_opt_revision_working : svn_opt_revision_head;
 
       /* Determine if we need to do pegged diffs. */
- if (opt_state->start_revision.kind != svn_opt_revision_base
- && opt_state->start_revision.kind != svn_opt_revision_working)
- pegged_diff = TRUE;
+ if ((opt_state->start_revision.kind != svn_opt_revision_base
+ && opt_state->start_revision.kind != svn_opt_revision_working)
+ || (opt_state->end_revision.kind != svn_opt_revision_base
+ && opt_state->end_revision.kind != svn_opt_revision_working))
+ pegged_diff = TRUE;
 
     }
 
Index: subversion/tests/cmdline/diff_tests.py
===================================================================
--- subversion/tests/cmdline/diff_tests.py (revision 18821)
+++ subversion/tests/cmdline/diff_tests.py (working copy)
@@ -2359,6 +2359,44 @@
     os.chdir(current_dir)
 
 
+#----------------------------------------------------------------------
+# A base->repos diff of a moved file used to output an all-lines-deleted diff
+def diff_base_repos_moved(sbox):
+ "base->repos diff of moved file"
+
+ sbox.build()
+
+ current_dir = os.getcwd()
+ os.chdir(sbox.wc_dir)
+ try:
+ oldfile = 'iota'
+ newfile = 'kappa'
+
+ # Move, modify and commit a file
+ svntest.main.run_svn(None, 'mv', oldfile, newfile)
+ open(newfile, 'w').write("new content\n")
+ svntest.actions.run_and_verify_svn(None, None, [], 'ci', '-m', '')
+
+ # Check that a base->repos diff shows deleted and added lines.
+ # It's not clear whether we expect a file-change diff or
+ # a file-delete plus file-add. The former is currently produced if we
+ # explicitly request a diff of the file itself, and the latter if we
+ # request a tree diff which just happens to contain the file.
+ out, err = svntest.actions.run_and_verify_svn(None, SVNAnyOutput, [],
+ 'diff', '-rBASE:1', newfile)
+ if check_diff_output(out, newfile, 'M'):
+ raise svntest.Failure
+
+ # Diff should recognise that the item's name has changed, and mention both
+ # the current and the old name in parentheses, in the right order.
+ if (out[2][:3] != '---' or out[2].find('kappa)') == -1 or
+ out[3][:3] != '+++' or out[3].find('iota)') == -1):
+ raise svntest.Failure
+
+ finally:
+ os.chdir(current_dir)
+
+
 ########################################################################
 #Run the tests
 
@@ -2399,6 +2437,7 @@
               diff_repos_wc_add_with_props,
               diff_nonrecursive_checkout_deleted_dir,
               diff_repos_working_added_dir,
+ diff_base_repos_moved,
               ]
 
 if __name__ == '__main__':

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Mar 13 00:27:44 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.