Author: kotkov
Date: Tue Oct 18 15:41:58 2016
New Revision: 1765466
URL: http://svn.apache.org/viewvc?rev=1765466&view=rev
Log:
In the tree conflict resolver, fix merging text content in the "remote move"
vs "local edit" scenario.
The issue was that if the conflict happened during merge, we were
using mismatched right and target files when calling svn_wc_merge5().
For instance, if we're merging from trunk to branch, we would want
- "left" file to have the ancestor's content
- "right" file to have the content from trunk
- "target" file to have the content from branch
Since we were first doing a merge on the added part of the move, and then
doing the metadata-only move, the "target" file (that arrived during merge)
was actually having content from trunk, and the "right" file had content
from branch.
Fix this by putting aside the target's file content, and moving the file
*before* doing a three-way merge. Ideally, this should be performed on
the libsvn_wc layer, and should be using a workqueue, but this is something
left for future work.
* subversion/libsvn_client/conflicts.c
(resolve_incoming_move_file_text_merge): Check the conflict operation
at the beginning of this function. If the conflict happened as a result
of the merge, set aside the move target file, perform the move, and then
do a proper three-way merge.
* subversion/tests/libsvn_client/conflicts-test.c
(test_merge_incoming_move_file_text_merge_conflict): Remove XFAIL comment.
(test_funcs): The test_merge_incoming_move_file_text_merge_conflict() test
now passes.
Modified:
subversion/trunk/subversion/libsvn_client/conflicts.c
subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c
Modified: subversion/trunk/subversion/libsvn_client/conflicts.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/conflicts.c?rev=1765466&r1=1765465&r2=1765466&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/conflicts.c (original)
+++ subversion/trunk/subversion/libsvn_client/conflicts.c Tue Oct 18 15:41:58
2016
@@ -6744,6 +6744,7 @@ resolve_incoming_move_file_text_merge(sv
struct conflict_tree_incoming_delete_details *details;
apr_array_header_t *possible_moved_to_abspaths;
const char *moved_to_abspath;
+ const char *incoming_abspath;
local_abspath = svn_client_conflict_get_local_abspath(conflict);
operation = svn_client_conflict_get_operation(conflict);
@@ -6755,6 +6756,12 @@ resolve_incoming_move_file_text_merge(sv
"to be fetched from the repository first."),
svn_dirent_local_style(local_abspath,
scratch_pool));
+ if (operation == svn_wc_operation_none)
+ return svn_error_createf(SVN_ERR_WC_CORRUPT, NULL,
+ _("Invalid operation code '%d' recorded for "
+ "conflict at '%s'"), operation,
+ svn_dirent_local_style(local_abspath,
+ scratch_pool));
option_id = svn_client_conflict_option_get_id(option);
SVN_ERR_ASSERT(option_id ==
@@ -6835,6 +6842,47 @@ resolve_incoming_move_file_text_merge(sv
if (err)
goto unlock_wc;
+ if (operation == svn_wc_operation_update ||
+ operation == svn_wc_operation_switch)
+ {
+ incoming_abspath = local_abspath;
+ }
+ else if (operation == svn_wc_operation_merge)
+ {
+ /* Set aside the current move target file. This is required to apply
+ * the move, and only then perform a three-way text merge between
+ * the ancestor's file, our working file (which we would move to
+ * the destination), and the file that we have set aside, which
+ * contains the incoming fulltext. */
+ err = svn_io_open_uniquely_named(NULL, &incoming_abspath,
+ svn_dirent_dirname(moved_to_abspath,
+ scratch_pool),
+ svn_dirent_basename(moved_to_abspath,
+ scratch_pool),
+ ".tmp",
+ svn_io_file_del_on_pool_cleanup,
+ scratch_pool, scratch_pool);
+ if (err)
+ goto unlock_wc;
+
+ err = svn_io_file_rename2(moved_to_abspath, incoming_abspath,
+ FALSE, scratch_pool);
+ if (err)
+ goto unlock_wc;
+
+ /* Apply the incoming move. */
+ err = svn_wc__move2(ctx->wc_ctx, local_abspath, moved_to_abspath,
+ FALSE, /* ordinary (not meta-data only) move */
+ FALSE, /* mixed-revisions don't apply to files */
+ NULL, NULL, /* don't allow user to cancel here */
+ NULL, NULL, /* no extra notification */
+ scratch_pool);
+ if (err)
+ goto unlock_wc;
+ }
+ else
+ SVN_ERR_MALFUNCTION();
+
/* Perform the file merge.
*
* ### Need to fix what we pass as "right_abspath" here, as svn_wc_merge5()
@@ -6847,7 +6895,7 @@ resolve_incoming_move_file_text_merge(sv
*/
err = svn_wc_merge5(&merge_content_outcome, &merge_props_outcome,
ctx->wc_ctx, ancestor_abspath,
- local_abspath, moved_to_abspath,
+ incoming_abspath, moved_to_abspath,
NULL, NULL, NULL, /* labels */
NULL, NULL, /* conflict versions */
FALSE, /* dry run */
@@ -6877,12 +6925,10 @@ resolve_incoming_move_file_text_merge(sv
ctx->notify_func2(ctx->notify_baton2, notify, scratch_pool);
}
- /* The merge is done. Local edits are now at the moved-to location. */
if (operation == svn_wc_operation_update ||
operation == svn_wc_operation_switch)
{
- /* The move operation is part of our natural history.
- * Delete the tree conflict victim (clears the tree conflict marker). */
+ /* Delete the tree conflict victim (clears the tree conflict marker). */
err = svn_wc_delete4(ctx->wc_ctx, local_abspath, FALSE, FALSE,
NULL, NULL, /* don't allow user to cancel here */
NULL, NULL, /* no extra notification */
@@ -6890,30 +6936,6 @@ resolve_incoming_move_file_text_merge(sv
if (err)
goto unlock_wc;
}
- else if (operation == svn_wc_operation_merge)
- {
- /* The move operation is not part of natural history. We must replicate
- * this move in our history. Record a move in the working copy. */
- err = svn_wc__move2(ctx->wc_ctx, local_abspath, moved_to_abspath,
- TRUE, /* this is a meta-data only move */
- FALSE, /* mixed-revisions don't apply to files */
- NULL, NULL, /* don't allow user to cancel here */
- NULL, NULL, /* no extra notification */
- scratch_pool);
- if (err)
- goto unlock_wc;
-
- /* Delete the original file from disk. */
- err = svn_io_remove_file2(local_abspath, FALSE, scratch_pool);
- if (err)
- goto unlock_wc;
- }
- else
- return svn_error_createf(SVN_ERR_WC_CORRUPT, NULL,
- _("Invalid operation code '%d' recorded for "
- "conflict at '%s'"), operation,
- svn_dirent_local_style(local_abspath,
- scratch_pool));
if (ctx->notify_func2)
{
Modified: subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c?rev=1765466&r1=1765465&r2=1765466&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_client/conflicts-test.c Tue Oct 18
15:41:58 2016
@@ -2703,10 +2703,6 @@ test_merge_incoming_move_file_text_merge
SVN_ERR(svn_stringbuf_from_file2(&buf, incoming_old_abspath, pool));
SVN_TEST_STRING_ASSERT(buf->data, "Initial content.\n");
- /* XFAIL: We merge from trunk (/A) to branch (/A1). The working version
- * should say "New branch content", and the incoming version should
- * say "New trunk content", not vice versa.
- */
SVN_ERR(svn_stringbuf_from_file2(&buf, working_abspath, pool));
SVN_TEST_STRING_ASSERT(buf->data, "New branch content.\n");
@@ -3753,8 +3749,8 @@ static struct svn_test_descriptor_t test
"merge incoming delete vs local delete"),
SVN_TEST_OPTS_PASS(test_merge_file_prop,
"merge file property"),
- SVN_TEST_OPTS_XFAIL(test_merge_incoming_move_file_text_merge_conflict,
- "merge incoming move file merge with text conflict"),
+ SVN_TEST_OPTS_PASS(test_merge_incoming_move_file_text_merge_conflict,
+ "merge incoming move file merge with text conflict"),
SVN_TEST_OPTS_PASS(test_merge_incoming_edit_file_moved_away,
"merge incoming edit for a moved-away working file"),
SVN_TEST_OPTS_PASS(test_merge_incoming_chained_move_local_edit,
