You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2015/11/03 15:53:25 UTC
svn commit: r1712304 - in /subversion/branches/move-tracking-2/subversion:
svnmover/merge3.c svnmover/svnmover.c svnmover/svnmover.h
tests/cmdline/svnmover_tests.py
Author: julianfoad
Date: Tue Nov 3 14:53:25 2015
New Revision: 1712304
URL: http://svn.apache.org/viewvc?rev=1712304&view=rev
Log:
On the 'move-tracking-2' branch: Detect cycle conflicts -- the last of the
four kinds of tree conflict. Add basic tests for tree conflict detection.
* subversion/svnmover/merge3.c
(element_merge3_conflict_create): Deep-copy the input parameters.
(element_merge3_conflict_dup): New.
(cycle_conflict_t,
cycle_conflict_create): New.
(orphan_conflict_create): Deep-copy the input parameter.
(svnmover_display_conflicts): Include details of cycle conflicts. Tweak
the display format of orphan conflicts. Display a summary, after the
details.
(detect_cycles): New.
(branch_merge_subtree_r): Include cycle conflicts. Allocate all conflicts
in the result pool.
(svnmover_branch_merge): Set the 'conflicts' output to null when there are
no conflicts, rather than an empty conflict object.
* subversion/svnmover/svnmover.c
(do_switch,
execute): Update the usage of svnmover_branch_merge().
* subversion/svnmover/svnmover.h
(conflict_storage_t): Add a 'cycle conflicts' member.
(svnmover_branch_merge): Add a result pool parameter. Update doc string.
(svnmover_display_conflicts): Remove the 'prefix' parameter.
* subversion/tests/cmdline/svnmover_tests.py
(merge_detects_clash_conflicts): Delete (replaced by tree_conflict_clash_1).
(tree_conflict_element_1,
tree_conflict_clash_1,
tree_conflict_clash_2,
tree_conflict_cycle_1,
tree_conflict_orphan_1): New tests.
(test_list): Update.
Modified:
subversion/branches/move-tracking-2/subversion/svnmover/merge3.c
subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c
subversion/branches/move-tracking-2/subversion/svnmover/svnmover.h
subversion/branches/move-tracking-2/subversion/tests/cmdline/svnmover_tests.py
Modified: subversion/branches/move-tracking-2/subversion/svnmover/merge3.c
URL: http://svn.apache.org/viewvc/subversion/branches/move-tracking-2/subversion/svnmover/merge3.c?rev=1712304&r1=1712303&r2=1712304&view=diff
==============================================================================
--- subversion/branches/move-tracking-2/subversion/svnmover/merge3.c (original)
+++ subversion/branches/move-tracking-2/subversion/svnmover/merge3.c Tue Nov 3 14:53:25 2015
@@ -179,12 +179,21 @@ element_merge3_conflict_create(svn_eleme
{
element_merge3_conflict_t *c = apr_pcalloc(result_pool, sizeof(*c));
- c->yca = yca;
- c->side1 = side1;
- c->side2 = side2;
+ c->yca = yca ? svn_element_content_dup(yca, result_pool) : NULL;
+ c->side1 = side1 ? svn_element_content_dup(side1, result_pool) : NULL;
+ c->side2 = side2 ? svn_element_content_dup(side2, result_pool) : NULL;
return c;
}
+static element_merge3_conflict_t *
+element_merge3_conflict_dup(element_merge3_conflict_t *old_conflict,
+ apr_pool_t *result_pool)
+{
+ return element_merge3_conflict_create(old_conflict->yca,
+ old_conflict->side1,
+ old_conflict->side2, result_pool);
+}
+
/* A name-clash conflict description.
*/
typedef struct name_clash_conflict_t
@@ -208,6 +217,23 @@ name_clash_conflict_create(int parent_ei
return c;
}
+/* A cycle conflict description.
+ */
+typedef struct cycle_conflict_t
+{
+ /* All EIDs that conflict with each other: hash of (eid -> irrelevant). */
+ apr_hash_t *elements;
+} cycle_conflict_t;
+
+static cycle_conflict_t *
+cycle_conflict_create(apr_pool_t *result_pool)
+{
+ cycle_conflict_t *c = apr_pcalloc(result_pool, sizeof(*c));
+
+ c->elements = apr_hash_make(result_pool);
+ return c;
+}
+
/* An orphan conflict description.
*/
typedef struct orphan_conflict_t
@@ -221,7 +247,7 @@ orphan_conflict_create(svn_element_conte
{
orphan_conflict_t *c = apr_pcalloc(result_pool, sizeof(*c));
- c->element = element;
+ c->element = svn_element_content_dup(element, result_pool);
return c;
}
@@ -247,11 +273,12 @@ brief_eid_and_name_or_nil(svn_element_co
svn_error_t *
svnmover_display_conflicts(conflict_storage_t *conflict_storage,
- const char *prefix,
apr_pool_t *scratch_pool)
{
apr_hash_index_t *hi;
+ svnmover_notify(_("Conflicts:"));
+
for (hi = apr_hash_first(scratch_pool,
conflict_storage->single_element_conflicts);
hi; hi = apr_hash_next(hi))
@@ -259,8 +286,8 @@ svnmover_display_conflicts(conflict_stor
int eid = svn_int_hash_this_key(hi);
element_merge3_conflict_t *c = apr_hash_this_val(hi);
- svnmover_notify("%ssingle-element conflict: e%d: yca=%s, side1=%s, side2=%s",
- prefix, eid,
+ svnmover_notify(" single-element conflict: e%d: yca=%s, side1=%s, side2=%s",
+ eid,
brief_eid_and_name_or_nil(c->yca, scratch_pool),
brief_eid_and_name_or_nil(c->side1, scratch_pool),
brief_eid_and_name_or_nil(c->side2, scratch_pool));
@@ -273,16 +300,35 @@ svnmover_display_conflicts(conflict_stor
name_clash_conflict_t *c = apr_hash_this_val(hi);
apr_hash_index_t *hi2;
- svnmover_notify("%sname-clash conflict: peid %d, name '%s', %d elements",
- prefix,
+ svnmover_notify(" name-clash conflict: peid %d, name '%s', %d elements",
c->parent_eid, c->name, apr_hash_count(c->elements));
for (hi2 = apr_hash_first(scratch_pool, c->elements);
hi2; hi2 = apr_hash_next(hi2))
{
int eid = svn_int_hash_this_key(hi2);
- svnmover_notify("%s element %d", prefix, eid);
+ svnmover_notify(" element %d", eid);
+ }
+ }
+ for (hi = apr_hash_first(scratch_pool,
+ conflict_storage->cycle_conflicts);
+ hi; hi = apr_hash_next(hi))
+ {
+ int eid = svn_int_hash_this_key(hi);
+ cycle_conflict_t *c = apr_hash_this_val(hi);
+ const char *desc = "elements";
+ apr_hash_index_t *hi2;
+
+ for (hi2 = apr_hash_first(scratch_pool, c->elements);
+ hi2; hi2 = apr_hash_next(hi2))
+ {
+ int eid2 = svn_int_hash_this_key(hi2);
+
+ desc = apr_psprintf(scratch_pool, "%s e%d", desc, eid2);
}
+
+ svnmover_notify(" cycle conflict: e%d: %s",
+ eid, desc);
}
for (hi = apr_hash_first(scratch_pool,
conflict_storage->orphan_conflicts);
@@ -291,9 +337,20 @@ svnmover_display_conflicts(conflict_stor
int eid = svn_int_hash_this_key(hi);
orphan_conflict_t *c = apr_hash_this_val(hi);
- svnmover_notify("%sorphan conflict: element %d/%s: peid %d does not exist",
- prefix, eid, c->element->name, c->element->parent_eid);
+ svnmover_notify(" orphan conflict: e%d: %d/%s: parent e%d does not exist",
+ eid, c->element->parent_eid, c->element->name,
+ c->element->parent_eid);
}
+
+ svnmover_notify(_("Summary of conflicts:\n"
+ " %d single-element conflicts\n"
+ " %d name-clash conflicts\n"
+ " %d cycle conflicts\n"
+ " %d orphan conflicts\n"),
+ apr_hash_count(conflict_storage->single_element_conflicts),
+ apr_hash_count(conflict_storage->name_clash_conflicts),
+ apr_hash_count(conflict_storage->cycle_conflicts),
+ apr_hash_count(conflict_storage->orphan_conflicts));
return SVN_NO_ERROR;
}
@@ -651,6 +708,50 @@ detect_clashes(apr_hash_t **clashes_p,
return SVN_NO_ERROR;
}
+/* Return all (eid -> cycle_conflict_t) cycle conflicts in BRANCH.
+
+ * ### This implementation is crude: it finds all cycles, but doesn't
+ * report them minimally. It reports each element that leads to a cycle,
+ * without isolating the minimal cycles nor eliminating duplicates.
+ */
+static svn_error_t *
+detect_cycles(apr_hash_t **cycles_p,
+ svn_branch_state_t *branch,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ apr_hash_t *cycles = apr_hash_make(result_pool);
+ SVN_ITER_T(svn_element_content_t) *pi;
+ const svn_element_tree_t *elements = svn_branch_get_element_tree(branch);
+
+ for (SVN_HASH_ITER(pi, scratch_pool, elements->e_map))
+ {
+ int eid = *(const int *)(pi->key);
+ svn_element_content_t *element = pi->val;
+ cycle_conflict_t *c = cycle_conflict_create(result_pool);
+
+ svn_int_hash_set(c->elements, eid, c);
+
+ /* See if we can trace the parentage of EID back to the branch root
+ without finding a cycle. If we find a cycle, store a conflict. */
+ for (; element && (element->parent_eid != -1);
+ element = svn_int_hash_get(elements->e_map, element->parent_eid))
+ {
+ /* If this parent-eid is already in the path from EID to the root,
+ then we have found a cycle. */
+ if (svn_int_hash_get(c->elements, element->parent_eid))
+ {
+ svn_int_hash_set(cycles, eid, c);
+ break;
+ }
+ svn_int_hash_set(c->elements, element->parent_eid, c);
+ }
+ }
+
+ *cycles_p = cycles;
+ return SVN_NO_ERROR;
+}
+
/* Return all (eid -> orphan_conflict_t) orphan conflicts in BRANCH.
*/
static svn_error_t *
@@ -785,7 +886,8 @@ branch_merge_subtree_r(svn_branch_txn_t
if (conflict)
{
svnmover_notify_v("! e%d <conflict>", eid);
- svn_int_hash_set(e_conflicts, eid, conflict);
+ svn_int_hash_set(e_conflicts, eid,
+ element_merge3_conflict_dup(conflict, result_pool));
}
else if (e_tgt && result)
{
@@ -841,6 +943,9 @@ branch_merge_subtree_r(svn_branch_txn_t
SVN_ERR(detect_clashes(&conflict_storage->name_clash_conflicts,
tgt->branch,
result_pool, scratch_pool));
+ SVN_ERR(detect_cycles(&conflict_storage->cycle_conflicts,
+ tgt->branch,
+ result_pool, scratch_pool));
SVN_ERR(detect_orphans(&conflict_storage->orphan_conflicts,
tgt->branch,
result_pool, scratch_pool));
@@ -858,8 +963,11 @@ svnmover_branch_merge(svn_branch_txn_t *
svn_branch_el_rev_id_t *src,
svn_branch_el_rev_id_t *tgt,
svn_branch_el_rev_id_t *yca,
+ apr_pool_t *result_pool,
apr_pool_t *scratch_pool)
{
+ conflict_storage_t *conflicts;
+
/*SVN_ERR(verify_exists_in_branch(from, scratch_pool));*/
/*SVN_ERR(verify_exists_in_branch(to, scratch_pool));*/
/*SVN_ERR(verify_exists_in_branch(yca, scratch_pool));*/
@@ -868,10 +976,24 @@ svnmover_branch_merge(svn_branch_txn_t *
/*SVN_ERR(verify_not_subbranch_root(yca, scratch_pool));*/
SVN_ERR(branch_merge_subtree_r(edit_txn,
- conflict_storage_p,
+ &conflicts,
src, tgt, yca,
- scratch_pool, scratch_pool));
+ result_pool, scratch_pool));
+ if (conflict_storage_p)
+ {
+ if (apr_hash_count(conflicts->single_element_conflicts)
+ || apr_hash_count(conflicts->name_clash_conflicts)
+ || apr_hash_count(conflicts->cycle_conflicts)
+ || apr_hash_count(conflicts->orphan_conflicts))
+ {
+ *conflict_storage_p = conflicts;
+ }
+ else
+ {
+ *conflict_storage_p = NULL;
+ }
+ }
return SVN_NO_ERROR;
}
Modified: subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c
URL: http://svn.apache.org/viewvc/subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c?rev=1712304&r1=1712303&r2=1712304&view=diff
==============================================================================
--- subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c (original)
+++ subversion/branches/move-tracking-2/subversion/svnmover/svnmover.c Tue Nov 3 14:53:25 2015
@@ -1332,23 +1332,14 @@ do_switch(svnmover_wc_t *wc,
svn_branch_root_eid(wc->working->branch),
SVN_INVALID_REVNUM, scratch_pool);
SVN_ERR(svnmover_branch_merge(wc->edit_txn, &conflicts,
- src, tgt, yca, scratch_pool));
+ src, tgt, yca, scratch_pool, scratch_pool));
- if (apr_hash_count(conflicts->single_element_conflicts)
- || apr_hash_count(conflicts->name_clash_conflicts)
- || apr_hash_count(conflicts->orphan_conflicts))
+ if (conflicts)
{
- SVN_ERR(svnmover_display_conflicts(conflicts, "switch: ",
- scratch_pool));
+ SVN_ERR(svnmover_display_conflicts(conflicts, scratch_pool));
return svn_error_createf(
SVN_ERR_BRANCHING, NULL,
- _("Switch failed because of conflicts: "
- "%d single-element conflicts, "
- "%d name-clash conflicts, "
- "%d orphan conflicts"),
- apr_hash_count(conflicts->single_element_conflicts),
- apr_hash_count(conflicts->name_clash_conflicts),
- apr_hash_count(conflicts->orphan_conflicts));
+ _("Switch failed because of conflicts"));
}
else
{
@@ -2887,23 +2878,14 @@ execute(svnmover_wc_t *wc,
arg[0]->el_rev /*from*/,
arg[1]->el_rev /*to*/,
arg[2]->el_rev /*yca*/,
- iterpool));
+ iterpool, iterpool));
- if (apr_hash_count(conflicts->single_element_conflicts)
- || apr_hash_count(conflicts->name_clash_conflicts)
- || apr_hash_count(conflicts->orphan_conflicts))
+ if (conflicts)
{
- SVN_ERR(svnmover_display_conflicts(conflicts, "merge: ",
- iterpool));
+ SVN_ERR(svnmover_display_conflicts(conflicts, iterpool));
return svn_error_createf(
SVN_ERR_BRANCHING, NULL,
- _("Merge failed because of conflicts: "
- "%d single-element conflicts, "
- "%d name-clash conflicts, "
- "%d orphan conflicts"),
- apr_hash_count(conflicts->single_element_conflicts),
- apr_hash_count(conflicts->name_clash_conflicts),
- apr_hash_count(conflicts->orphan_conflicts));
+ _("Merge failed because of conflicts"));
}
else
{
Modified: subversion/branches/move-tracking-2/subversion/svnmover/svnmover.h
URL: http://svn.apache.org/viewvc/subversion/branches/move-tracking-2/subversion/svnmover/svnmover.h?rev=1712304&r1=1712303&r2=1712304&view=diff
==============================================================================
--- subversion/branches/move-tracking-2/subversion/svnmover/svnmover.h (original)
+++ subversion/branches/move-tracking-2/subversion/svnmover/svnmover.h Tue Nov 3 14:53:25 2015
@@ -88,6 +88,10 @@ typedef struct conflict_storage_t
/* ("%{parent_eid}d/%{name}s" -> name_clash_conflict_t) */
apr_hash_t *name_clash_conflicts;
+ /* Cycle conflicts */
+ /* (eid -> cycle_conflict_t) */
+ apr_hash_t *cycle_conflicts;
+
/* Orphan conflicts */
/* (eid -> orphan_conflict_t) */
apr_hash_t *orphan_conflicts;
@@ -98,7 +102,8 @@ typedef struct conflict_storage_t
* Merge the two sets of changes: YCA -> SRC and YCA -> TGT, applying
* the result to the transaction at TGT.
*
- * If conflicts arise, just fail.
+ * If conflicts arise, return them in *CONFLICT_STORAGE_P; otherwise set
+ * that to null.
*
* SRC, TGT and YCA must be existing and corresponding (same EID) elements.
*
@@ -112,12 +117,12 @@ svnmover_branch_merge(svn_branch_txn_t *
svn_branch_el_rev_id_t *src,
svn_branch_el_rev_id_t *tgt,
svn_branch_el_rev_id_t *yca,
+ apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
/* */
svn_error_t *
svnmover_display_conflicts(conflict_storage_t *conflict_storage,
- const char *prefix,
apr_pool_t *scratch_pool);
Modified: subversion/branches/move-tracking-2/subversion/tests/cmdline/svnmover_tests.py
URL: http://svn.apache.org/viewvc/subversion/branches/move-tracking-2/subversion/tests/cmdline/svnmover_tests.py?rev=1712304&r1=1712303&r2=1712304&view=diff
==============================================================================
--- subversion/branches/move-tracking-2/subversion/tests/cmdline/svnmover_tests.py (original)
+++ subversion/branches/move-tracking-2/subversion/tests/cmdline/svnmover_tests.py Tue Nov 3 14:53:25 2015
@@ -1316,25 +1316,6 @@ def modify_payload_of_branch_root_elemen
test_svnmover2(sbox, '', None,
'put ' + mk_file(sbox, 'f2') + ' f2')
-def merge_detects_clash_conflicts(sbox):
- "merge detects clash conflicts"
- sbox_build_svnmover(sbox)
-
- # Make a trunk and a branch
- test_svnmover2(sbox, '', None,
- 'mkbranch A ' +
- 'branch A B')
-
- # Make new elements at the same path in each branch
- test_svnmover2(sbox, '', None,
- 'mkdir A/D ' +
- 'mkdir B/D')
-
- # Merge: should generate a clash conflict
- xtest_svnmover(sbox.repo_url, 'Merge failed.*clash',
- 'merge A B A@1')
-
-
def merge_swap_abc(sbox):
"merge swaps A and C in A/B/C"
sbox_build_svnmover(sbox)
@@ -1469,6 +1450,78 @@ def move_to_related_branch_2(sbox):
expected_eids,
'branch-into-and-delete X/A/B Y/B')
+def tree_conflict_detect(sbox,
+ initial_state_cmds,
+ side1_cmds,
+ side2_cmds):
+ """Set up an initial state on one branch using INITIAL_STATE_CMDS,
+ branch it to a second branch, make changes on each branch using
+ SIDE1_CMDS and SIDE2_CMDS, merge the first branch to the second,
+ and expect a conflict."""
+ sbox_build_svnmover(sbox)
+
+ # initial state
+ test_svnmover2(sbox, '', None,
+ 'mkbranch trunk')
+ if initial_state_cmds:
+ test_svnmover2(sbox, 'trunk', None,
+ initial_state_cmds)
+ # branching
+ test_svnmover2(sbox, '', None,
+ 'branch trunk br1')
+ # conflicting changes
+ if side1_cmds:
+ test_svnmover2(sbox, 'trunk', None,
+ side1_cmds)
+ if side2_cmds:
+ test_svnmover2(sbox, 'br1', None,
+ side2_cmds)
+ # merge
+ xtest_svnmover(sbox.repo_url, 'E123456: Merge failed because of conflicts',
+ 'merge trunk br1 trunk@2')
+
+# A simple single-element tree conflict
+def tree_conflict_element_1(sbox):
+ "tree_conflict_element_1"
+ tree_conflict_detect(sbox,
+ 'mkdir a',
+ 'mv a b',
+ 'mv a c')
+
+# A simple name-clash tree conflict
+def tree_conflict_clash_1(sbox):
+ "tree_conflict_clash_1"
+ tree_conflict_detect(sbox,
+ 'mkdir a '
+ 'mkdir b',
+ 'mv a c',
+ 'mv b c')
+
+# A simple name-clash tree conflict
+def tree_conflict_clash_2(sbox):
+ "tree_conflict_clash_2"
+ tree_conflict_detect(sbox,
+ None,
+ 'mkdir c',
+ 'mkdir c')
+
+# A simple cycle tree conflict
+def tree_conflict_cycle_1(sbox):
+ "tree_conflict_cycle_1"
+ tree_conflict_detect(sbox,
+ 'mkdir a '
+ 'mkdir b',
+ 'mv a b/a',
+ 'mv b a/b')
+
+# A simple orphan tree conflict
+def tree_conflict_orphan_1(sbox):
+ "tree_conflict_orphan_1"
+ tree_conflict_detect(sbox,
+ 'mkdir orphan-parent',
+ 'mkdir orphan-parent/orphan',
+ 'rm orphan-parent')
+
######################################################################
test_list = [ None,
@@ -1489,9 +1542,13 @@ test_list = [ None,
branch_to_subbranch_of_self,
merge_from_subbranch_to_subtree,
modify_payload_of_branch_root_element,
- merge_detects_clash_conflicts,
merge_swap_abc,
move_to_related_branch_2,
+ tree_conflict_element_1,
+ tree_conflict_clash_1,
+ tree_conflict_clash_2,
+ tree_conflict_cycle_1,
+ tree_conflict_orphan_1,
]
if __name__ == '__main__':