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