You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2013/02/06 19:22:19 UTC
svn commit: r1443112 - in /subversion/trunk/subversion:
libsvn_client/merge.c tests/cmdline/merge_tree_conflict_tests.py
tests/cmdline/tree_conflict_tests.py
Author: rhuijben
Date: Wed Feb 6 18:22:19 2013
New Revision: 1443112
URL: http://svn.apache.org/viewvc?rev=1443112&view=rev
Log:
Resolve long standing open parts of issues #3150 'tree conflicts with
directories as victims' and issue #2282 by making the merge of a directory
delete validate if the delete target matches what the merge expects.
This was one of the major reasons to refactor the diff interface more than
absolutely necessary.
These issues might be completed by this patch, but the descriptions are
to vague to call them closed at this time.
* subversion/libsvn_client/merge.c
(dir_delete_baton_t): Add struct.
(mark_dir_edited,
mark_file_edited): If handling a parent-delete, just note the edited state
in the delete baton instead of notifying.
(merge_file_opened): If handling a parent-delete and we already detected an
edit: skip further processing on this node.
(merge_file_deleted): Store comparision in parent delete baton when
available. Just call the wc delete if we don't use a single feature of the
libsvn_client wrapper.
(merge_dir_opened): Remove the global skip of descendants of deletes, but
still apply it if we find a reason to skip the current node. Continue or
start comparison mode on non force-delete deletes.
(verify_touched_by_del_check): New function.
(merge_dir_deleted): Compare properties to see if we are different than
expected to be deleted. Store result in baton.
For the root: If no differences were collected, verify if we walked
all existing descendants (including not-ignored local files) and only
if so delete the directory. Otherwise mark tree conflict.
* subversion/tests/cmdline/merge_tree_conflict_tests.py
(tree_conflicts_on_merge_local_ci_5_2): Remove XFail.
Update expected status.
* subversion/tests/cmdline/tree_conflict_tests.py
(merge_dir_del_onto_not_same): Remove XFail.
Modified:
subversion/trunk/subversion/libsvn_client/merge.c
subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py
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=1443112&r1=1443111&r2=1443112&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Wed Feb 6 18:22:19 2013
@@ -1354,6 +1354,21 @@ check_moved_here(svn_boolean_t *moved_he
#define CONFLICT_REASON_SKIP ((svn_wc_conflict_reason_t)-3)
#define CONFLICT_REASON_SKIP_WC ((svn_wc_conflict_reason_t)-4)
+/* Baton used for testing trees for being editted while performing tree
+ conflict detection for incoming deletes */
+struct dir_delete_baton_t
+{
+ /* Reference to dir baton of directory that is the root of the deletion */
+ struct merge_dir_baton_t *del_root;
+
+ /* Boolean indicating that some edit is found. Allows avoiding more work */
+ svn_boolean_t found_edit;
+
+ /* A list of paths that are compared. Kept up to date until FOUND_EDIT is
+ set to TRUE */
+ apr_hash_t *compared_abspaths;
+};
+
/* Baton for the merge_dir_*() functions. Initialized in merge_dir_opened() */
struct merge_dir_baton_t
{
@@ -1411,6 +1426,11 @@ struct merge_dir_baton_t
notification (which may be a replaced notification if the node is not just
deleted) */
apr_hash_t *pending_deletes;
+
+ /* If not NULL, a reference to the information of the delete test that is
+ currently in progress. Allocated in the root-directory baton, referenced
+ from all descendants */
+ struct dir_delete_baton_t *delete_state;
};
/* Baton for the merge_dir_*() functions. Initialized in merge_file_opened() */
@@ -1691,8 +1711,14 @@ mark_dir_edited(merge_cmd_baton_t *merge
if (! db->shadowed)
return SVN_NO_ERROR; /* Easy out */
- if (db->tree_conflict_reason == CONFLICT_REASON_SKIP
- || db->tree_conflict_reason == CONFLICT_REASON_SKIP_WC)
+ if (db->parent_baton
+ && db->parent_baton->delete_state
+ && db->tree_conflict_reason != CONFLICT_REASON_NONE)
+ {
+ db->parent_baton->delete_state->found_edit = TRUE;
+ }
+ else if (db->tree_conflict_reason == CONFLICT_REASON_SKIP
+ || db->tree_conflict_reason == CONFLICT_REASON_SKIP_WC)
{
/* open_directory() decided not to flag a tree conflict, but
for clarity we produce a skip for this node that
@@ -1798,8 +1824,14 @@ mark_file_edited(merge_cmd_baton_t *merg
if (! fb->shadowed)
return SVN_NO_ERROR; /* Easy out */
- if (fb->tree_conflict_reason == CONFLICT_REASON_SKIP
- || fb->tree_conflict_reason == CONFLICT_REASON_SKIP_WC)
+ if (fb->parent_baton
+ && fb->parent_baton->delete_state
+ && fb->tree_conflict_reason != CONFLICT_REASON_NONE)
+ {
+ fb->parent_baton->delete_state->found_edit = TRUE;
+ }
+ else if (fb->tree_conflict_reason == CONFLICT_REASON_SKIP
+ || fb->tree_conflict_reason == CONFLICT_REASON_SKIP_WC)
{
/* open_directory() decided not to flag a tree conflict, but
for clarity we produce a skip for this node that
@@ -1998,7 +2030,13 @@ merge_file_opened(void **new_file_baton,
return SVN_NO_ERROR; /* Already set a tree conflict */
}
- /* TODO: Start comparison mode to verify for delete tree conflict */
+ /* Comparison mode to verify for delete tree conflicts? */
+ if (pdb && pdb->delete_state
+ && pdb->delete_state->found_edit)
+ {
+ /* Earlier nodes found a conflict. Done. */
+ *skip = TRUE;
+ }
}
}
else
@@ -2509,21 +2547,45 @@ merge_file_deleted(const char *relpath,
}
/* If the files are identical, attempt deletion */
- SVN_ERR(files_same_p(&same, left_file, left_props,
- local_abspath, merge_b->ctx->wc_ctx,
- scratch_pool));
- if (same || merge_b->force_delete || merge_b->record_only /* ### why? */)
- {
- /* Passing NULL for the notify_func and notify_baton because
- repos_diff.c:delete_entry() will do it for us. */
- SVN_ERR(svn_client__wc_delete(local_abspath, TRUE,
- merge_b->dry_run, FALSE, NULL, NULL,
- merge_b->ctx, scratch_pool));
+ if (merge_b->force_delete)
+ same = TRUE;
+ else
+ SVN_ERR(files_same_p(&same, left_file, left_props,
+ local_abspath, merge_b->ctx->wc_ctx,
+ scratch_pool));
+
+ if (fb->parent_baton
+ && fb->parent_baton->delete_state)
+ {
+ if (same)
+ {
+ /* Note that we checked this file */
+ store_path(fb->parent_baton->delete_state->compared_abspaths,
+ local_abspath);
+ }
+ else
+ {
+ /* We found some modification. Parent should raise a tree conflict */
+ fb->parent_baton->delete_state->found_edit = TRUE;
+ }
+
+ return SVN_NO_ERROR;
+ }
+ else if (same)
+ {
+ if (!merge_b->dry_run)
+ SVN_ERR(svn_wc_delete4(merge_b->ctx->wc_ctx, local_abspath,
+ FALSE /* keep_local */, FALSE /* unversioned */,
+ merge_b->ctx->cancel_func,
+ merge_b->ctx->cancel_baton,
+ NULL, NULL /* no notify */,
+ scratch_pool));
/* Record that we might have deleted mergeinfo */
alloc_and_store_path(&merge_b->paths_with_deleted_mergeinfo,
local_abspath, merge_b->pool);
+ /* And notify the deletion */
SVN_ERR(record_update_delete(merge_b, fb->parent_baton, local_abspath,
svn_node_file, scratch_pool));
}
@@ -2586,9 +2648,6 @@ merge_dir_opened(void **new_dir_baton,
*new_dir_baton = db;
- if (! right_source)
- *skip_children = TRUE; /* ### Compatibility with walk_children = FALSE */
-
if (pdb)
{
db->parent_baton = pdb;
@@ -2637,6 +2696,14 @@ merge_dir_opened(void **new_dir_baton,
db->tree_conflict_reason = CONFLICT_REASON_SKIP;
db->skip_reason = obstr_state;
+
+ if (! right_source)
+ {
+ *skip = *skip_children = TRUE;
+ SVN_ERR(mark_dir_edited(merge_b, db, local_abspath,
+ scratch_pool));
+ }
+
return SVN_NO_ERROR;
}
@@ -2712,10 +2779,32 @@ merge_dir_opened(void **new_dir_baton,
if (db->shadowed)
{
+ *skip_children = TRUE;
return SVN_NO_ERROR; /* Already set a tree conflict */
}
- /* TODO: Start comparison mode to verify for delete tree conflict */
+ db->delete_state = (pdb != NULL) ? pdb->delete_state : NULL;
+
+ if (db->delete_state && db->delete_state->found_edit)
+ {
+ /* A sibling found a conflict. Done. */
+ *skip = TRUE;
+ *skip_children = TRUE;
+ }
+ else if (merge_b->force_delete)
+ {
+ /* No comparison necessary */
+ *skip_children = TRUE;
+ }
+ else if (! db->delete_state)
+ {
+ /* Start descendant comparison */
+ db->delete_state = apr_pcalloc(db->pool,
+ sizeof(*db->delete_state));
+
+ db->delete_state->del_root = db;
+ db->delete_state->compared_abspaths = apr_hash_make(db->pool);
+ }
}
}
else
@@ -3064,6 +3153,31 @@ merge_dir_added(const char *relpath,
return SVN_NO_ERROR;
}
+/* Helper for merge_dir_deleted. Implement svn_wc_status_func4_t */
+static svn_error_t *
+verify_touched_by_del_check(void *baton,
+ const char *local_abspath,
+ const svn_wc_status3_t *status,
+ apr_pool_t *scratch_pool)
+{
+ struct dir_delete_baton_t *delb = baton;
+
+ if (contains_path(delb->compared_abspaths, local_abspath))
+ return SVN_NO_ERROR;
+
+ switch (status->node_status)
+ {
+ case svn_wc_status_deleted:
+ case svn_wc_status_ignored:
+ case svn_wc_status_none:
+ return SVN_NO_ERROR;
+
+ default:
+ delb->found_edit = TRUE;
+ return svn_error_create(SVN_ERR_CEASE_INVOCATION, NULL, NULL);
+ }
+}
+
/* An svn_diff_tree_processor_t function.
*
* Called after merge_dir_opened() when a node existed only in the left source.
@@ -3071,9 +3185,6 @@ merge_dir_added(const char *relpath,
* After the merge_dir_opened() but before the call to this merge_dir_deleted()
* function all descendants that existed in left_source will have been deleted.
*
- * ### By default the diff drivers report all descendants of a delete,
- * ### but this feature is currently disabled by a SKIP on merge_dir_opened()
- *
* If this node is replaced, an _opened() followed by a matching _add() will
* be invoked after this function.
*/
@@ -3089,7 +3200,9 @@ merge_dir_deleted(const char *relpath,
struct merge_dir_baton_t *db = dir_baton;
const char *local_abspath = svn_dirent_join(merge_b->target->abspath,
relpath, scratch_pool);
- svn_error_t *err;
+ struct dir_delete_baton_t *delb;
+ svn_boolean_t same;
+ apr_hash_t *working_props;
SVN_ERR(handle_pending_notifications(merge_b, db, scratch_pool));
SVN_ERR(mark_dir_edited(merge_b, db, local_abspath, scratch_pool));
@@ -3113,23 +3226,96 @@ merge_dir_deleted(const char *relpath,
return SVN_NO_ERROR;
}
- /* ### TODO: Before deleting, we should ensure that this dir
- tree is equal to the one we're being asked to delete.
- If not, mark this directory as a tree conflict victim,
- because this could be use case 5 as described in
- notes/tree-conflicts/detection.txt.
- */
+ SVN_ERR(svn_wc_prop_list2(&working_props,
+ merge_b->ctx->wc_ctx, local_abspath,
+ scratch_pool, scratch_pool));
- /* Passing NULL for the notify_func and notify_baton because
- repos_diff.c:delete_entry() will do it for us. */
- err = svn_client__wc_delete(local_abspath, merge_b->force_delete,
- merge_b->dry_run, FALSE,
- NULL, NULL,
- merge_b->ctx, scratch_pool);
- if (err)
+ if (merge_b->force_delete)
+ same = TRUE;
+ else
{
- svn_error_clear(err);
+ /* Compare the properties */
+ SVN_ERR(properties_same_p(&same, left_props, working_props,
+ scratch_pool));
+ }
+
+ delb = db->delete_state;
+ assert(delb != NULL);
+ if (! same)
+ {
+ delb->found_edit = TRUE;
+ }
+ else
+ {
+ store_path(delb->compared_abspaths, local_abspath);
+ }
+
+ if (delb->del_root != db)
+ return SVN_NO_ERROR;
+
+ if (delb->found_edit)
+ same = FALSE;
+ else if (merge_b->force_delete)
+ same = TRUE;
+ else
+ {
+ apr_array_header_t *ignores;
+ svn_error_t *err;
+ same = TRUE;
+
+ SVN_ERR(svn_wc_get_default_ignores(&ignores, merge_b->ctx->config,
+ scratch_pool));
+
+ /* None of the descendants was modified, but maybe there are
+ descendants we haven't walked?
+
+ Note that we aren't interested in changes, as we already verified
+ changes in the paths touched by the merge. And the existance of
+ other paths is enough to mark the directory edited */
+ err = svn_wc_walk_status(merge_b->ctx->wc_ctx, local_abspath,
+ svn_depth_infinity, TRUE /* get-all */,
+ FALSE /* no-ignore */,
+ TRUE /* ignore-text-mods */, ignores,
+ verify_touched_by_del_check, delb,
+ merge_b->ctx->cancel_func,
+ merge_b->ctx->cancel_baton,
+ scratch_pool);
+
+ if (err)
+ {
+ if (err->apr_err != SVN_ERR_CEASE_INVOCATION)
+ return svn_error_trace(err);
+
+ svn_error_clear(err);
+ }
+
+ same = ! delb->found_edit;
+ }
+
+ if (same && !merge_b->dry_run)
+ {
+ svn_error_t *err;
+
+ err = svn_wc_delete4(merge_b->ctx->wc_ctx, local_abspath,
+ FALSE /* keep_local */, FALSE /* unversioned */,
+ merge_b->ctx->cancel_func,
+ merge_b->ctx->cancel_baton,
+ NULL, NULL /* no notify */,
+ scratch_pool);
+
+ if (err)
+ {
+ if (err->apr_err != SVN_ERR_WC_LEFT_LOCAL_MOD)
+ return svn_error_trace(err);
+
+ svn_error_clear(err);
+ same = FALSE;
+ }
+ }
+
+ if (! same)
+ {
/* If the attempt to delete an existing directory failed,
* the directory has local modifications (e.g. locally added
* files, or property changes). Flag a tree conflict. */
@@ -3143,8 +3329,13 @@ merge_dir_deleted(const char *relpath,
else
{
/* Record that we might have deleted mergeinfo */
- alloc_and_store_path(&merge_b->paths_with_deleted_mergeinfo,
- local_abspath, merge_b->pool);
+ if (working_props
+ && apr_hash_get(working_props, SVN_PROP_MERGEINFO,
+ APR_HASH_KEY_STRING))
+ {
+ alloc_and_store_path(&merge_b->paths_with_deleted_mergeinfo,
+ local_abspath, merge_b->pool);
+ }
SVN_ERR(record_update_delete(merge_b, db->parent_baton, local_abspath,
svn_node_dir, scratch_pool));
Modified: subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py?rev=1443112&r1=1443111&r2=1443112&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tree_conflict_tests.py Wed Feb 6 18:22:19 2013
@@ -941,7 +941,6 @@ def tree_conflicts_on_merge_local_ci_5_1
expected_skip) ], True)
#----------------------------------------------------------------------
-@XFail()
@Issue(2282)
def tree_conflicts_on_merge_local_ci_5_2(sbox):
"tree conflicts 5.2: leaf del, tree del"
@@ -957,15 +956,15 @@ def tree_conflicts_on_merge_local_ci_5_2
'D' : Item(status=' ', wc_rev='3'),
'F' : Item(status=' ', wc_rev='3'),
'DD' : Item(status=' ', wc_rev='3'),
- 'DD/D1' : Item(status='! ', treeconflict='C'),
+ 'DD/D1' : Item(status=' ', wc_rev='3', treeconflict='C'),
'DF' : Item(status=' ', wc_rev='3'),
- 'DF/D1' : Item(status='! ', treeconflict='C'),
+ 'DF/D1' : Item(status=' ', wc_rev='3', treeconflict='C'),
'DDD' : Item(status=' ', wc_rev='3'),
- 'DDD/D1' : Item(status='! ', treeconflict='C'),
- 'DDD/D1/D2' : Item(status='D ', wc_rev='3'),
+ 'DDD/D1' : Item(status=' ', wc_rev='3', treeconflict='C'),
+ 'DDD/D1/D2' : Item(status=' ', wc_rev='3'),
'DDF' : Item(status=' ', wc_rev='3'),
- 'DDF/D1' : Item(status='! ', treeconflict='C'),
- 'DDF/D1/D2' : Item(status='D ', wc_rev='3'),
+ 'DDF/D1' : Item(status=' ', wc_rev='3', treeconflict='C'),
+ 'DDF/D1/D2' : Item(status=' ', wc_rev='3'),
'D/D1' : Item(status='! ', treeconflict='C'),
'F/alpha' : Item(status='! ', treeconflict='C'),
})
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=1443112&r1=1443111&r2=1443112&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py Wed Feb 6 18:22:19 2013
@@ -689,7 +689,6 @@ def merge_dir_mod_onto_not_dir(sbox):
test_tc_merge(sbox2, d_mods, wc_scen = d_dels + d_moves)
# Test for issue #3150 'tree conflicts with directories as victims'.
-@XFail()
@Issue(3150)
def merge_dir_del_onto_not_same(sbox):
"merge dir: del/rpl/mv onto not-same"