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 2015/01/15 13:17:00 UTC
svn commit: r1652073 - in /subversion/trunk/subversion:
libsvn_wc/wc_db_update_move.c tests/cmdline/update_tests.py
tests/libsvn_wc/op-depth-test.c
Author: rhuijben
Date: Thu Jan 15 12:16:59 2015
New Revision: 1652073
URL: http://svn.apache.org/r1652073
Log:
Following up on r1561425, and r1652047 fix some working copy lock tests in the
move update logic that were applied too strict.
* subversion/libsvn_wc/wc_db_update_move.c
(update_moved_away_node,
replace_moved_layer): Verify write lock here.
(bump_mark_tree_conflict): Instead of here. Update documentation.
(svn_wc__db_bump_moved_away): Avoid returning an error in a case where we
shouldn't be involved anyway.
* subversion/tests/cmdline/update_tests.py
(update_moved_away): Remove XFail marker.
* subversion/tests/libsvn_wc/op-depth-test.c
(test_funcs): Remove XFail from update_within_move, move_update_subtree and
update_with_tree_conflict that only failed because we verified a lock that
isn't really required.
Modified:
subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
subversion/trunk/subversion/tests/cmdline/update_tests.py
subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c
Modified: subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c?rev=1652073&r1=1652072&r2=1652073&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db_update_move.c Thu Jan 15 12:16:59 2015
@@ -1504,6 +1504,9 @@ update_moved_away_node(update_move_baton
apr_array_header_t *src_children, *dst_children;
int dst_op_depth = relpath_depth(move_root_dst_relpath);
+ SVN_ERR(verify_write_lock(wcroot, src_relpath, scratch_pool));
+ SVN_ERR(verify_write_lock(wcroot, dst_relpath, scratch_pool));
+
SVN_ERR(get_info(&src_props, &src_checksum, &src_children, &src_kind,
src_relpath, src_op_depth,
wcroot, scratch_pool, scratch_pool));
@@ -1653,6 +1656,9 @@ replace_moved_layer(const char *src_relp
svn_boolean_t have_row;
int dst_op_depth = relpath_depth(dst_relpath);
+ SVN_ERR(verify_write_lock(wcroot, src_relpath, scratch_pool));
+ SVN_ERR(verify_write_lock(wcroot, dst_relpath, scratch_pool));
+
/* Replace entire subtree at one op-depth. */
SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
STMT_SELECT_LOCAL_RELPATH_OP_DEPTH));
@@ -2046,40 +2052,8 @@ bump_mark_tree_conflict(svn_wc__db_wcroo
svn_wc_conflict_version_t *old_version;
svn_wc_conflict_version_t *new_version;
- /* Verify precondition*/
- SVN_ERR(verify_write_lock(wcroot, move_src_op_root_relpath, scratch_pool));
- {
- svn_boolean_t locked;
-
- SVN_ERR(svn_wc__db_wclock_owns_lock_internal(&locked, wcroot,
- move_dst_op_root_relpath,
- FALSE, scratch_pool));
-
- if (!locked)
- {
- /* The user is updating something that is moved away, but the location
- that it is moved to is outside the lock scope of the operation
- itself.
-
- The only thing we can do is mark a tree conflict, to allow
- updating the target in a followup 'resolve' operation, as updating
- the target itself is not allowed. (Taking a lock is the responsibility
- of the caller!) */
-
- /* ### WIP: Return same error as before */
- return svn_error_createf(SVN_ERR_WC_NOT_LOCKED, NULL,
- _("Can't bump move from '%s' to '%s' as the "
- "target isn't locked"),
- path_for_error_message(
- wcroot,
- move_src_op_root_relpath,
- scratch_pool),
- path_for_error_message(
- wcroot,
- move_dst_op_root_relpath,
- scratch_pool));
- }
- }
+ /* Verify precondition: We are allowed to set a tree conflict here. */
+ SVN_ERR(verify_write_lock(wcroot, move_src_root_relpath, scratch_pool));
/* Read new (post-update) information from the new move source BASE node. */
SVN_ERR(svn_wc__db_base_get_info_internal(NULL, &new_kind, &new_rev,
@@ -2091,7 +2065,14 @@ bump_mark_tree_conflict(svn_wc__db_wcroo
SVN_ERR(svn_wc__db_fetch_repos_info(&repos_root_url, &repos_uuid,
wcroot->sdb, repos_id, scratch_pool));
- /* Read old (pre-update) information from the move destination node. */
+ /* Read old (pre-update) information from the move destination node.
+
+ This potentially touches nodes that aren't locked by us, but that is not
+ a problem because we have a SQLite write lock here, and all sqlite
+ operations that affect move stability use a sqlite lock as well.
+ (And affecting the move itself requires a write lock on the node that
+ we do own the lock for: the move source)
+ */
SVN_ERR(svn_wc__db_depth_get_info(NULL, &old_kind, &old_rev,
&old_repos_relpath, NULL, NULL, NULL,
NULL, NULL, NULL, NULL, NULL, NULL,
@@ -2099,6 +2080,21 @@ bump_mark_tree_conflict(svn_wc__db_wcroo
relpath_depth(move_dst_op_root_relpath),
scratch_pool, scratch_pool));
+ if (strcmp(move_src_root_relpath, move_src_op_root_relpath))
+ {
+ /* We have information for the op-root, but need it for the node that
+ we are putting the tree conflict on. Luckily we know that we have
+ a clean BASE */
+
+ const char *rpath = svn_relpath_skip_ancestor(move_src_op_root_relpath,
+ move_src_root_relpath);
+
+ old_repos_relpath = svn_relpath_join(old_repos_relpath, rpath,
+ scratch_pool);
+ new_repos_relpath = svn_relpath_join(new_repos_relpath, rpath,
+ scratch_pool);
+ }
+
old_version = svn_wc_conflict_version_create2(
repos_root_url, repos_uuid, old_repos_relpath, old_rev,
old_kind, scratch_pool);
@@ -2365,10 +2361,26 @@ svn_wc__db_bump_moved_away(svn_wc__db_wc
{
if (strcmp(move_src_root_relpath, local_relpath))
{
- SVN_ERR(bump_mark_tree_conflict(wcroot, move_src_root_relpath,
- move_src_op_root_relpath,
- move_dst_op_root_relpath,
- db, scratch_pool));
+ /* An ancestor of the path that was updated is moved away.
+
+ If we have a lock on that ancestor, we can mark a tree
+ conflict on it, if we don't we ignore this case. A future
+ update of the ancestor will handle this. */
+ svn_boolean_t locked;
+
+ SVN_ERR(svn_wc__db_wclock_owns_lock_internal(
+ &locked, wcroot,
+ move_src_root_relpath,
+ FALSE, scratch_pool));
+
+ if (locked)
+ {
+ SVN_ERR(bump_mark_tree_conflict(wcroot,
+ move_src_root_relpath,
+ move_src_op_root_relpath,
+ move_dst_op_root_relpath,
+ db, scratch_pool));
+ }
return SVN_NO_ERROR;
}
}
Modified: subversion/trunk/subversion/tests/cmdline/update_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/update_tests.py?rev=1652073&r1=1652072&r2=1652073&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/update_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/update_tests.py Thu Jan 15 12:16:59 2015
@@ -6517,7 +6517,6 @@ def windows_update_backslash(sbox):
expected_status = svntest.actions.get_virginal_state(wc_dir, 2)
svntest.actions.run_and_verify_status(wc_dir, expected_status)
-@XFail() # Tries to modify unlocked part of working copy; found via r1561425
def update_moved_away(sbox):
"update subtree of moved away"
Modified: subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c?rev=1652073&r1=1652072&r2=1652073&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c Thu Jan 15 12:16:59 2015
@@ -9975,7 +9975,7 @@ static struct svn_test_descriptor_t test
"move_replace"),
SVN_TEST_OPTS_PASS(layered_moved_to,
"layered_moved_to"),
- SVN_TEST_OPTS_XFAIL(update_within_move,
+ SVN_TEST_OPTS_PASS(update_within_move,
"update_within_move"),
SVN_TEST_OPTS_PASS(commit_moved_descendant,
"commit_moved_descendant"),
@@ -9997,7 +9997,7 @@ static struct svn_test_descriptor_t test
"new_basemove"),
SVN_TEST_OPTS_PASS(move_back,
"move_back (issue 4302)"),
- SVN_TEST_OPTS_XFAIL(move_update_subtree,
+ SVN_TEST_OPTS_PASS(move_update_subtree,
"move_update_subtree (issue 4232)"),
SVN_TEST_OPTS_PASS(move_parent_into_child,
"move_parent_into_child (issue 4333)"),
@@ -10007,7 +10007,7 @@ static struct svn_test_descriptor_t test
"move retract (issue 4336)"),
SVN_TEST_OPTS_PASS(move_delete_file_externals,
"move/delete file externals (issue 4293)"),
- SVN_TEST_OPTS_XFAIL(update_with_tree_conflict,
+ SVN_TEST_OPTS_PASS(update_with_tree_conflict,
"update with tree conflict (issue 4347)"),
SVN_TEST_OPTS_PASS(move_update_parent_replace,
"move update with replaced parent (issue 4388)"),