You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by sv...@apache.org on 2015/03/19 19:23:34 UTC
svn commit: r1667830 - in /subversion/branches/1.9.x: ./ STATUS
subversion/libsvn_wc/conflicts.c
subversion/tests/libsvn_wc/conflict-data-test.c
subversion/tests/libsvn_wc/utils.c subversion/tests/libsvn_wc/utils.h
Author: svn-role
Date: Thu Mar 19 18:23:34 2015
New Revision: 1667830
URL: http://svn.apache.org/r1667830
Log:
Merge the r1663338 group from trunk:
* r1663338, r1663347, r1663374
Properly record resolving of individual property conflicts.
Justification:
Fixes a few implementation bugs in the resolver code and exposes the
api for more users than those that pass a callback function.
Notes:
Bert says it changes a public API and so needs to go into 1.9.0: see
http://colabti.org/irclogger/irclogger_log/svn-dev?date=2015-03-10#l93
.
rhuijben: While the new feature of allowing the resolving of individual
properties is not a release blocker to me, the known buggy behavior of
the current code that may cause unuexpected loss of properties and
conflict data is.
.
The part of the patch that enables the new feature is very small
compared to the part that fixes the issue, and the regression
test would be at least tree times harder to write and review without
enabling the resolving of individual property conflicts.
.
Note that not being able to resolve individual property conflicts
on update/switch/merge is a regression against 1.7.x, where we didn't
switch to resolving after the update completed yet.
.
Votes:
+1: rhuijben, philip
+1: brane (But there's no API change here, no public headers are
modified by the backport merge.)
Modified:
subversion/branches/1.9.x/ (props changed)
subversion/branches/1.9.x/STATUS
subversion/branches/1.9.x/subversion/libsvn_wc/conflicts.c
subversion/branches/1.9.x/subversion/tests/libsvn_wc/conflict-data-test.c
subversion/branches/1.9.x/subversion/tests/libsvn_wc/utils.c
subversion/branches/1.9.x/subversion/tests/libsvn_wc/utils.h
Propchange: subversion/branches/1.9.x/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Mar 19 18:23:34 2015
@@ -89,4 +89,4 @@
/subversion/branches/verify-at-commit:1462039-1462408
/subversion/branches/verify-keep-going:1439280-1546110
/subversion/branches/wc-collate-path:1402685-1480384
-/subversion/trunk:1660545-1660547,1660549-1662901,1663003,1663450,1663697,1663706,1663749,1664078,1664080,1664084-1664085,1664187,1664191,1664200,1664344,1664476,1664480-1664481,1664483,1664507,1664520-1664521,1664523,1664526-1664527,1664531-1664532,1664588,1664927,1665164,1665611-1665612,1665845,1665850,1665852,1665886
+/subversion/trunk:1660545-1660547,1660549-1662901,1663003,1663338,1663347,1663374,1663450,1663697,1663706,1663749,1664078,1664080,1664084-1664085,1664187,1664191,1664200,1664344,1664476,1664480-1664481,1664483,1664507,1664520-1664521,1664523,1664526-1664527,1664531-1664532,1664588,1664927,1665164,1665611-1665612,1665845,1665850,1665852,1665886
Modified: subversion/branches/1.9.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.9.x/STATUS?rev=1667830&r1=1667829&r2=1667830&view=diff
==============================================================================
--- subversion/branches/1.9.x/STATUS (original)
+++ subversion/branches/1.9.x/STATUS Thu Mar 19 18:23:34 2015
@@ -178,34 +178,6 @@ Veto-blocked changes:
Approved changes:
=================
- * r1663338, r1663347, r1663374
- Properly record resolving of individual property conflicts.
- Justification:
- Fixes a few implementation bugs in the resolver code and exposes the
- api for more users than those that pass a callback function.
- Notes:
- Bert says it changes a public API and so needs to go into 1.9.0: see
- http://colabti.org/irclogger/irclogger_log/svn-dev?date=2015-03-10#l93
- .
- rhuijben: While the new feature of allowing the resolving of individual
- properties is not a release blocker to me, the known buggy behavior of
- the current code that may cause unuexpected loss of properties and
- conflict data is.
- .
- The part of the patch that enables the new feature is very small
- compared to the part that fixes the issue, and the regression
- test would be at least tree times harder to write and review without
- enabling the resolving of individual property conflicts.
- .
- Note that not being able to resolve individual property conflicts
- on update/switch/merge is a regression against 1.7.x, where we didn't
- switch to resolving after the update completed yet.
- .
- Votes:
- +1: rhuijben, philip
- +1: brane (But there's no API change here, no public headers are
- modified by the backport merge.)
-
* r1666270, r1666272
Resolve a segfault when an update introduces a conflict on the update root
Justification:
Modified: subversion/branches/1.9.x/subversion/libsvn_wc/conflicts.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.9.x/subversion/libsvn_wc/conflicts.c?rev=1667830&r1=1667829&r2=1667830&view=diff
==============================================================================
--- subversion/branches/1.9.x/subversion/libsvn_wc/conflicts.c (original)
+++ subversion/branches/1.9.x/subversion/libsvn_wc/conflicts.c Thu Mar 19 18:23:34 2015
@@ -2338,7 +2338,7 @@ static svn_error_t *
resolve_prop_conflict_on_node(svn_boolean_t *did_resolve,
svn_wc__db_t *db,
const char *local_abspath,
- const svn_skel_t *conflicts,
+ svn_skel_t *conflicts,
const char *conflicted_propname,
svn_wc_conflict_choice_t conflict_choice,
const char *merged_file,
@@ -2357,6 +2357,8 @@ resolve_prop_conflict_on_node(svn_boolea
svn_skel_t *work_items = NULL;
svn_wc_operation_t operation;
svn_boolean_t prop_conflicted;
+ apr_hash_t *actual_props;
+ svn_boolean_t resolved_all, resolved_all_prop;
*did_resolve = FALSE;
@@ -2372,12 +2374,35 @@ resolve_prop_conflict_on_node(svn_boolea
db, local_abspath, conflicts,
scratch_pool, scratch_pool));
+ if (!conflicted_props)
+ {
+ /* We have a pre 1.8 property conflict. Just mark it resolved */
+
+ SVN_ERR(remove_artifact_file_if_exists(&work_items, did_resolve,
+ db, local_abspath, prop_reject_file,
+ scratch_pool, scratch_pool));
+ SVN_ERR(svn_wc__db_op_mark_resolved(db, local_abspath, FALSE, TRUE, FALSE,
+ work_items, scratch_pool));
+ SVN_ERR(svn_wc__wq_run(db, local_abspath, cancel_func, cancel_baton,
+ scratch_pool));
+ return SVN_NO_ERROR;
+ }
+
+ if (conflicted_propname[0] != '\0'
+ && !svn_hash_gets(conflicted_props, conflicted_propname))
+ {
+ return SVN_NO_ERROR; /* This property is not conflicted! */
+ }
+
if (operation == svn_wc_operation_merge)
SVN_ERR(svn_wc__db_read_pristine_props(&old_props, db, local_abspath,
scratch_pool, scratch_pool));
else
old_props = their_old_props;
+ SVN_ERR(svn_wc__db_read_props(&actual_props, db, local_abspath,
+ scratch_pool, scratch_pool));
+
/* We currently handle *_conflict as *_full as this argument is currently
always applied for all conflicts on a node at the same time. Giving
an error would break some tests that assumed that this would just
@@ -2405,11 +2430,7 @@ resolve_prop_conflict_on_node(svn_boolea
case svn_wc_conflict_choose_merged:
if ((merged_file || merged_value) && conflicted_propname[0] != '\0')
{
- apr_hash_t *actual_props;
-
- SVN_ERR(svn_wc__db_read_props(&actual_props, db, local_abspath,
- scratch_pool, scratch_pool));
- resolve_from = actual_props;
+ resolve_from = apr_hash_copy(scratch_pool, actual_props);
if (!merged_value)
{
@@ -2433,15 +2454,26 @@ resolve_prop_conflict_on_node(svn_boolea
_("Invalid 'conflict_result' argument"));
}
- if (conflicted_props && apr_hash_count(conflicted_props) && resolve_from)
+
+ if (resolve_from)
{
apr_hash_index_t *hi;
- apr_hash_t *actual_props;
+ apr_hash_t *apply_on_props;
- SVN_ERR(svn_wc__db_read_props(&actual_props, db, local_abspath,
- scratch_pool, scratch_pool));
+ if (conflicted_propname[0] == '\0')
+ {
+ /* Apply to all conflicted properties */
+ apply_on_props = conflicted_props;
+ }
+ else
+ {
+ /* Apply to a single property */
+ apply_on_props = apr_hash_make(scratch_pool);
+ svn_hash_sets(apply_on_props, conflicted_propname, "");
+ }
- for (hi = apr_hash_first(scratch_pool, conflicted_props);
+ /* Apply the selected changes */
+ for (hi = apr_hash_first(scratch_pool, apply_on_props);
hi;
hi = apr_hash_next(hi))
{
@@ -2452,28 +2484,67 @@ resolve_prop_conflict_on_node(svn_boolea
svn_hash_sets(actual_props, propname, new_value);
}
- SVN_ERR(svn_wc__db_op_set_props(db, local_abspath, actual_props,
- FALSE, NULL, NULL,
- scratch_pool));
}
+ /*else the user accepted the properties as-is */
- /* Legacy behavior: Only report property conflicts as resolved when the
- property reject file exists
+ /* This function handles conflicted_propname "" as resolving
+ all property conflicts... Just what we need here */
+ SVN_ERR(svn_wc__conflict_skel_resolve(&resolved_all, conflicts,
+ db, local_abspath,
+ FALSE, conflicted_propname,
+ FALSE,
+ scratch_pool, scratch_pool));
- If not the UI shows the conflict as already resolved
- (and in this case we just remove the in-db conflict) */
+ if (!resolved_all)
+ {
+ /* Are there still property conflicts left? (or only...) */
+ SVN_ERR(svn_wc__conflict_read_info(NULL, NULL, NULL, &prop_conflicted,
+ NULL, db, local_abspath, conflicts,
+ scratch_pool, scratch_pool));
- {
- svn_skel_t *work_item;
+ resolved_all_prop = (! prop_conflicted);
+ }
+ else
+ {
+ resolved_all_prop = TRUE;
+ conflicts = NULL;
+ }
- SVN_ERR(remove_artifact_file_if_exists(&work_item, did_resolve,
- db, local_abspath, prop_reject_file,
- scratch_pool, scratch_pool));
- work_items = svn_wc__wq_merge(work_items, work_item, scratch_pool);
- }
+ if (resolved_all_prop)
+ {
+ /* Legacy behavior: Only report property conflicts as resolved when the
+ property reject file exists
+
+ If not the UI shows the conflict as already resolved
+ (and in this case we just remove the in-db conflict) */
+ SVN_ERR(remove_artifact_file_if_exists(&work_items, did_resolve,
+ db, local_abspath,
+ prop_reject_file,
+ scratch_pool, scratch_pool));
+ }
+ else
+ {
+ /* Create a new prej file, based on the remaining conflicts */
+ SVN_ERR(svn_wc__wq_build_prej_install(&work_items,
+ db, local_abspath,
+ scratch_pool, scratch_pool));
+ *did_resolve = TRUE; /* We resolved a property conflict */
+ }
+
+ /* This installs the updated conflict skel */
+ SVN_ERR(svn_wc__db_op_set_props(db, local_abspath, actual_props,
+ FALSE, conflicts, work_items,
+ scratch_pool));
+
+ if (resolved_all)
+ {
+ /* Remove the whole conflict. Should probably be integrated
+ into the op_set_props() call */
+ SVN_ERR(svn_wc__db_op_mark_resolved(db, local_abspath,
+ FALSE, TRUE, FALSE,
+ NULL, scratch_pool));
+ }
- SVN_ERR(svn_wc__db_op_mark_resolved(db, local_abspath, FALSE, TRUE, FALSE,
- work_items, scratch_pool));
SVN_ERR(svn_wc__wq_run(db, local_abspath, cancel_func, cancel_baton,
scratch_pool));
@@ -3020,13 +3091,6 @@ svn_wc__resolve_conflicts(svn_wc_context
apr_pool_t *iterpool = NULL;
svn_error_t *err;
- /* ### the underlying code does NOT support resolving individual
- ### properties. bail out if the caller tries it. */
- if (resolve_prop != NULL && *resolve_prop != '\0')
- return svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL,
- U_("Resolving a single property is not (yet) "
- "supported."));
-
/* ### Just a versioned check? */
/* Conflicted is set to allow invoking on actual only nodes */
SVN_ERR(svn_wc__db_read_info(NULL, &kind, NULL, NULL, NULL, NULL, NULL,
Modified: subversion/branches/1.9.x/subversion/tests/libsvn_wc/conflict-data-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.9.x/subversion/tests/libsvn_wc/conflict-data-test.c?rev=1667830&r1=1667829&r2=1667830&view=diff
==============================================================================
--- subversion/branches/1.9.x/subversion/tests/libsvn_wc/conflict-data-test.c (original)
+++ subversion/branches/1.9.x/subversion/tests/libsvn_wc/conflict-data-test.c Thu Mar 19 18:23:34 2015
@@ -809,6 +809,108 @@ test_prop_conflicts(const svn_test_opts_
return SVN_NO_ERROR;
}
+static svn_error_t *
+test_prop_conflict_resolving(const svn_test_opts_t *opts,
+ apr_pool_t *pool)
+{
+ svn_test__sandbox_t b;
+ svn_skel_t *conflict;
+ const char *A_abspath;
+ const char *marker_abspath;
+ apr_hash_t *conflicted_props;
+ apr_hash_t *props;
+ const char *value;
+
+ SVN_ERR(svn_test__sandbox_create(&b, "test_prop_resolving", opts, pool));
+ SVN_ERR(sbox_wc_mkdir(&b, "A"));
+
+ SVN_ERR(sbox_wc_propset(&b, "prop-1", "r1", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-2", "r1", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-3", "r1", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-4", "r1", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-5", "r1", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-6", "r1", "A"));
+
+ SVN_ERR(sbox_wc_commit(&b, ""));
+ SVN_ERR(sbox_wc_propset(&b, "prop-1", "r2", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-2", "r2", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-3", "r2", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-4", NULL, "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-5", NULL, "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-7", "r2", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-8", "r2", "A"));
+ SVN_ERR(sbox_wc_commit(&b, ""));
+
+ SVN_ERR(sbox_wc_propset(&b, "prop-1", "mod", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-2", "mod", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-3", "mod", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-4", "mod", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-5", "mod", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-6", "mod", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-7", "mod", "A"));
+ SVN_ERR(sbox_wc_propset(&b, "prop-8", "mod", "A"));
+
+ SVN_ERR(sbox_wc_update(&b, "", 1));
+
+ A_abspath = sbox_wc_path(&b, "A");
+ SVN_ERR(svn_wc__db_read_conflict(&conflict, NULL,
+ b.wc_ctx->db, A_abspath,
+ pool, pool));
+
+ /* We have tree conflicts... */
+ SVN_TEST_ASSERT(conflict != NULL);
+
+ SVN_ERR(svn_wc__conflict_read_prop_conflict(&marker_abspath,
+ NULL, NULL, NULL,
+ &conflicted_props,
+ b.wc_ctx->db, A_abspath,
+ conflict,
+ pool, pool));
+
+ SVN_TEST_ASSERT(conflicted_props != NULL);
+ /* All properties but r6 are conflicted */
+ SVN_TEST_ASSERT(apr_hash_count(conflicted_props) == 7);
+ SVN_TEST_ASSERT(! svn_hash_gets(conflicted_props, "prop-6"));
+
+ /* Let's resolve a few conflicts */
+ SVN_ERR(sbox_wc_resolve_prop(&b, "A", "prop-1",
+ svn_wc_conflict_choose_mine_conflict));
+ SVN_ERR(sbox_wc_resolve_prop(&b, "A", "prop-2",
+ svn_wc_conflict_choose_theirs_conflict));
+ SVN_ERR(sbox_wc_resolve_prop(&b, "A", "prop-3",
+ svn_wc_conflict_choose_merged));
+
+ SVN_ERR(svn_wc__db_read_conflict(&conflict, NULL,
+ b.wc_ctx->db, A_abspath,
+ pool, pool));
+
+ /* We have tree conflicts... */
+ SVN_TEST_ASSERT(conflict != NULL);
+
+ SVN_ERR(svn_wc__conflict_read_prop_conflict(&marker_abspath,
+ NULL, NULL, NULL,
+ &conflicted_props,
+ b.wc_ctx->db, A_abspath,
+ conflict,
+ pool, pool));
+
+ SVN_TEST_ASSERT(conflicted_props != NULL);
+ SVN_TEST_ASSERT(apr_hash_count(conflicted_props) == 4);
+
+ SVN_ERR(svn_wc__db_read_props(&props, b.wc_ctx->db, A_abspath,
+ pool, pool));
+
+ value = svn_prop_get_value(props, "prop-1");
+ SVN_TEST_STRING_ASSERT(value, "mod");
+ value = svn_prop_get_value(props, "prop-2");
+ SVN_TEST_STRING_ASSERT(value, "r1");
+ value = svn_prop_get_value(props, "prop-3");
+ SVN_TEST_STRING_ASSERT(value, "mod");
+
+ return SVN_NO_ERROR;
+}
+
+
/* The test table. */
static int max_threads = 1;
@@ -830,6 +932,8 @@ static struct svn_test_descriptor_t test
"read and write a tree conflict"),
SVN_TEST_OPTS_PASS(test_prop_conflicts,
"test prop conflicts"),
+ SVN_TEST_OPTS_PASS(test_prop_conflict_resolving,
+ "test property conflict resolving"),
SVN_TEST_NULL
};
Modified: subversion/branches/1.9.x/subversion/tests/libsvn_wc/utils.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.9.x/subversion/tests/libsvn_wc/utils.c?rev=1667830&r1=1667829&r2=1667830&view=diff
==============================================================================
--- subversion/branches/1.9.x/subversion/tests/libsvn_wc/utils.c (original)
+++ subversion/branches/1.9.x/subversion/tests/libsvn_wc/utils.c Thu Mar 19 18:23:34 2015
@@ -599,6 +599,35 @@ sbox_wc_resolve(svn_test__sandbox_t *b,
}
svn_error_t *
+sbox_wc_resolve_prop(svn_test__sandbox_t *b, const char *path,
+ const char *propname,
+ svn_wc_conflict_choice_t conflict_choice)
+{
+ const char *lock_abspath;
+ svn_error_t *err;
+
+ SVN_ERR(svn_wc__acquire_write_lock_for_resolve(&lock_abspath, b->wc_ctx,
+ sbox_wc_path(b, path),
+ b->pool, b->pool));
+ err = svn_wc__resolve_conflicts(b->wc_ctx, sbox_wc_path(b, path),
+ svn_depth_empty,
+ FALSE,
+ propname,
+ FALSE,
+ conflict_choice,
+ NULL, NULL, /* conflict func */
+ NULL, NULL, /* cancellation */
+ NULL, NULL, /* notification */
+ b->pool);
+
+ err = svn_error_compose_create(err, svn_wc__release_write_lock(b->wc_ctx,
+ lock_abspath,
+ b->pool));
+ return err;
+}
+
+
+svn_error_t *
sbox_wc_move(svn_test__sandbox_t *b, const char *src, const char *dst)
{
svn_client_ctx_t *ctx;
Modified: subversion/branches/1.9.x/subversion/tests/libsvn_wc/utils.h
URL: http://svn.apache.org/viewvc/subversion/branches/1.9.x/subversion/tests/libsvn_wc/utils.h?rev=1667830&r1=1667829&r2=1667830&view=diff
==============================================================================
--- subversion/branches/1.9.x/subversion/tests/libsvn_wc/utils.h (original)
+++ subversion/branches/1.9.x/subversion/tests/libsvn_wc/utils.h Thu Mar 19 18:23:34 2015
@@ -166,6 +166,12 @@ sbox_wc_resolve(svn_test__sandbox_t *b,
/* */
svn_error_t *
+sbox_wc_resolve_prop(svn_test__sandbox_t *b, const char *path,
+ const char *propname,
+ svn_wc_conflict_choice_t conflict_choice);
+
+/* */
+svn_error_t *
sbox_wc_move(svn_test__sandbox_t *b, const char *src, const char *dst);
/* Set property NAME to VALUE on PATH. If VALUE=NULL, delete the property. */