On Wed, 2011-08-24, stsp_at_apache.org wrote:
> Author: stsp
> Date: Wed Aug 24 18:26:19 2011
> New Revision: 1161219
>
> URL: http://svn.apache.org/viewvc?rev=1161219&view=rev
> Log:
> Auto-resolve 'local move vs. incoming edit' conflicts for files during merge.
>
> * subversion/libsvn_client/repos_diff.c
> (close_file): If the file did receive changes during the merge
> and has been moved-away, show the file's new location during
> notification.
Hi Stefan. I've just come across this bit of code and I think we need
to move it to somewhere else. It's inside a diff editor which is used
for general repos-repos diffing as well as for providing the source of a
merge. The merge code gives this editor a (supposed) WC target path
parameter:
svn_client__get_diff_editor(target=target_abspath ...)
and the editor uses that "target" parameter as the prefix on each path
that it sends to the diff callbacks, and on each path that it sends to
the notification callback. But that's the only thing it does with it.
Until now.
When "svn diff" uses the same editor, it passes an empty string as the
"target" parameter because there is no WC path involved in a repos-repos
diff.
I was just cleaning up this editor, stripping out this "target" path
completely, so that it would simply pass relative paths out to the diff
callbacks and to the notification functions. The merge code knows what
WC path this diff relates to, and so can add the prefix itself inside
its callbacks. That will keep the knowledge about the WC completely
separate from the repos-diff editor, which is a good thing.
We are at the point now where we will want to follow a local move and
apply a sub-tree of the incoming diff to the new path in the merge
target WC, not the path that is currently being constructed by prefixing
the supposed "WC target" onto the repository-side relpath. So getting
this info out of the diff editor will help to make everything clearer
for following local moves as well.
Do you follow? Can you help me to find the right way to implement this
notification adjustment in the merge code outside this editor?
- Julian
> * subversion/libsvn_client/merge.c
> (merge_file_changed): If the file being merged into was
> moved away locally, merge incoming changes to the file
> at the new location.
>
> * subversion/tests/cmdline/tree_conflict_tests.py
> (merge_file_mod_onto_not_file): Don't expect a tree conflict
> for locally moved files.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/merge.c
> subversion/trunk/subversion/libsvn_client/repos_diff.c
> subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1161219&r1=1161218&r2=1161219&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Wed Aug 24 18:26:19 2011
> @@ -1460,8 +1460,9 @@ merge_file_changed(svn_wc_notify_state_t
> way svn_wc_merge4() can do the merge. */
> if (wc_kind != svn_node_file || is_deleted)
> {
> - svn_boolean_t moved_away;
> + const char *moved_to_abspath;
> svn_wc_conflict_reason_t reason;
> + svn_error_t *err;
>
> /* Maybe the node is excluded via depth filtering? */
>
> @@ -1491,23 +1492,39 @@ merge_file_changed(svn_wc_notify_state_t
> /* This is use case 4 described in the paper attached to issue
> * #2282. See also notes/tree-conflicts/detection.txt
> */
> - SVN_ERR(check_moved_away(&moved_away, merge_b->ctx->wc_ctx,
> - mine_abspath, scratch_pool));
> - if (moved_away)
> - reason = svn_wc_conflict_reason_moved_away;
> - else if (is_deleted)
> - reason = svn_wc_conflict_reason_deleted;
> + err = svn_wc__node_was_moved_away(&moved_to_abspath, NULL,
> + merge_b->ctx->wc_ctx, mine_abspath,
> + scratch_pool, scratch_pool);
> + if (err)
> + {
> + if (err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
> + svn_error_clear(err);
> + else
> + return svn_error_trace(err);
> + }
> +
> + if (moved_to_abspath)
> + {
> + /* File has been moved away locally -- apply incoming
> + * changes at the new location. */
> + mine_abspath = moved_to_abspath;
> + }
> else
> - reason = svn_wc_conflict_reason_missing;
> - SVN_ERR(tree_conflict(merge_b, mine_abspath, svn_node_file,
> - svn_wc_conflict_action_edit, reason));
> - if (tree_conflicted)
> - *tree_conflicted = TRUE;
> - if (content_state)
> - *content_state = svn_wc_notify_state_missing;
> - if (prop_state)
> - *prop_state = svn_wc_notify_state_missing;
> - return SVN_NO_ERROR;
> + {
> + if (is_deleted)
> + reason = svn_wc_conflict_reason_deleted;
> + else
> + reason = svn_wc_conflict_reason_missing;
> + SVN_ERR(tree_conflict(merge_b, mine_abspath, svn_node_file,
> + svn_wc_conflict_action_edit, reason));
> + if (tree_conflicted)
> + *tree_conflicted = TRUE;
> + if (content_state)
> + *content_state = svn_wc_notify_state_missing;
> + if (prop_state)
> + *prop_state = svn_wc_notify_state_missing;
> + return SVN_NO_ERROR;
> + }
> }
>
> /* ### TODO: Thwart attempts to merge into a path that has
>
> Modified: subversion/trunk/subversion/libsvn_client/repos_diff.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/repos_diff.c?rev=1161219&r1=1161218&r2=1161219&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/repos_diff.c (original)
> +++ subversion/trunk/subversion/libsvn_client/repos_diff.c Wed Aug 24 18:26:19 2011
> @@ -1052,6 +1052,7 @@ close_file(void *file_baton,
> svn_wc_notify_t *notify;
> svn_wc_notify_action_t action;
> svn_node_kind_t kind = svn_node_file;
> + const char *moved_to_abspath = NULL;
>
> /* Find out if a pending delete notification for this path is
> * still around. */
> @@ -1087,9 +1088,30 @@ close_file(void *file_baton,
> else if (b->added)
> action = svn_wc_notify_update_add;
> else
> - action = svn_wc_notify_update_update;
> + {
> + svn_error_t *err;
> +
> + action = svn_wc_notify_update_update;
> +
> + /* If the file was moved-away, use its new path in the
> + * notification.
> + * ### This is redundant. The file_changed() callback should
> + * ### pass the moved-to path back up here. */
> + err = svn_wc__node_was_moved_away(&moved_to_abspath, NULL,
> + eb->wc_ctx, b->wcpath,
> + scratch_pool, scratch_pool);
> + if (err)
> + {
> + if (err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
> + svn_error_clear(err);
> + else
> + return svn_error_trace(err);
> + }
> + }
>
> - notify = svn_wc_create_notify(b->wcpath, action, scratch_pool);
> + notify = svn_wc_create_notify(moved_to_abspath ? moved_to_abspath
> + : b->wcpath,
> + action, scratch_pool);
> notify->kind = kind;
> notify->content_state = content_state;
> notify->prop_state = prop_state;
>
> Modified: subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py?rev=1161219&r1=1161218&r2=1161219&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py Wed Aug 24 18:26:19 2011
> @@ -657,7 +657,7 @@ def merge_file_mod_onto_not_file(sbox):
> "merge file: modify onto not-file"
> sbox2 = sbox.clone_dependent()
> test_tc_merge(sbox, f_mods, br_scen = f_dels + f_moves + f_rpl_d)
> - test_tc_merge(sbox2, f_mods, wc_scen = f_dels + f_moves)
> + test_tc_merge(sbox2, f_mods, wc_scen = f_dels)
> # Note: See UC4 in notes/tree-conflicts/use-cases.txt.
>
> def merge_file_del_onto_not_same(sbox):
>
>
Received on 2011-08-26 18:14:20 CEST