On Tue, May 24, 2011 at 12:35 PM, <s...@apache.org> wrote:
> Author: stsp
> Date: Tue May 24 16:35:14 2011
> New Revision: 1127134
>
> URL: http://svn.apache.org/viewvc?rev=1127134&view=rev
> Log:
> As part of issue #3779, "actual-only nodes need regression tests",
> make 'svn add' detect tree conflict victims that do not exist on disk
> and prevent adding new nodes at that path with a meaningful error message.
>
> This implies that if users need to add a new node to resolve the conflict
> they need to mark the conflict as resolved first. I think this is safer
> than allowing accidental additions to take place. Since the node is not
> visible on disk the addition might be a mistake.
>
> * subversion/libsvn_wc/adm_ops.c
> (check_can_add_node): Don't allow adding new items on top of nonexistent
> conflicted nodes.
>
> * subversion/libsvn_client/add.c
> (add): As previous.
>
> * subversion/tests/cmdline/tree_conflict_tests.py
> (actual_only_node_behaviour): Adjust test cases for 'add' and 'mkdir'.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/add.c
> subversion/trunk/subversion/libsvn_wc/adm_ops.c
> subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
>
> Modified: subversion/trunk/subversion/libsvn_client/add.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/add.c?rev=1127134&r1=1127133&r2=1127134&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/add.c (original)
> +++ subversion/trunk/subversion/libsvn_client/add.c Tue May 24 16:35:14 2011
> @@ -537,10 +537,29 @@ add(void *baton, apr_pool_t *result_pool
> else if (kind == svn_node_file)
> err = add_file(b->local_abspath, b->ctx, scratch_pool);
> else if (kind == svn_node_none)
> - return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> - _("'%s' not found"),
> - svn_dirent_local_style(b->local_abspath,
> - scratch_pool));
> + {
> + svn_boolean_t tree_conflicted;
> +
> + /* Provide a meaningful error message if the node does not exist
> + * on disk but is a tree conflict victim. */
> + err = svn_wc_conflicted_p3(NULL, NULL, &tree_conflicted,
> + b->ctx->wc_ctx, b->local_abspath,
> + scratch_pool);
> + if (err)
> + svn_error_clear(err);
> + else if (tree_conflicted)
> + return svn_error_createf(SVN_ERR_WC_FOUND_CONFLICT, NULL,
> + _("'%s' is an existing item in conflict; "
> + "please mark the conflict as resolved "
> + "before adding a new item here"),
> + svn_dirent_local_style(b->local_abspath,
> + scratch_pool));
> + return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> + _("'%s' not found"),
> + svn_dirent_local_style(b->local_abspath,
> + scratch_pool));
> + }
> else
> return svn_error_createf(SVN_ERR_UNSUPPORTED_FEATURE, NULL,
> _("Unsupported node kind for path '%s'"),
>
> Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=1127134&r1=1127133&r2=1127134&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue May 24 16:35:14 2011
> @@ -849,10 +849,12 @@ check_can_add_node(svn_node_kind_t *kind
> adding the new node; if not, return an error. */
> {
> svn_wc__db_status_t status;
> + svn_boolean_t conflicted;
> svn_error_t *err
> = svn_wc__db_read_info(&status, NULL, NULL, NULL, NULL, NULL, NULL,
> NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> - NULL, NULL, NULL, NULL, NULL, NULL, NULL,
> + NULL, NULL, NULL, NULL, NULL, NULL,
> + &conflicted,
> NULL, NULL, NULL, NULL, NULL, NULL,
> db, local_abspath,
> scratch_pool, scratch_pool);
> @@ -870,6 +872,16 @@ check_can_add_node(svn_node_kind_t *kind
> {
> is_wc_root = FALSE;
> exists = TRUE;
> +
> + /* Note that the node may be in conflict even if it does not
> + * exist on disk (certain tree conflict scenarios). */
> + if (conflicted)
> + return svn_error_createf(SVN_ERR_WC_FOUND_CONFLICT, NULL,
> + _("'%s' is an existing item in conflict; "
> + "please mark the conflict as resolved "
> + "before adding a new item here"),
> + svn_dirent_local_style(local_abspath,
> + scratch_pool));
Hi Stefan,
While reviewing some outstanding merge-related issues I noticed that
this change breaks the use-case of incoming replacements on local
deletes. The delete portion of the replacement is handled and a
tree-conflict set by the time the add is done and the above error is
raised.
Here's a quick demonstration starting with a vanilla Greek tree:
### Make a branch:
>svn copy ^^/A ^^/branch -m "Create a branch"
Committed revision 2.
### Replace a directory on the "trunk":
>svn del A\C
D A\C
>svn mkdir A\C
A A\C
>svn ci -m "Replace A/C"
Replacing A\C
Committed revision 3.
### Delete the corresponding directory on the "branch":
>svn del ^^/branch/C -m "Delete branch/C"
Committed revision 4.
>svn up -q
### Now try to merge the replacement onto the deletion; it fails:
>svn merge ^^/A branch
..\..\..\subversion\svn\util.c:913: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:11349: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:11303: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:11303: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:11273: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:9287: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:8870: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:5349: (apr_err=155015)
..\..\..\subversion\libsvn_repos\reporter.c:1430: (apr_err=155015)
..\..\..\subversion\libsvn_client\ra.c:247: (apr_err=155015)
..\..\..\subversion\libsvn_repos\reporter.c:1269: (apr_err=155015)
..\..\..\subversion\libsvn_repos\reporter.c:1205: (apr_err=155015)
..\..\..\subversion\libsvn_repos\reporter.c:920: (apr_err=155015)
..\..\..\subversion\libsvn_delta\cancel.c:120: (apr_err=155015)
..\..\..\subversion\libsvn_delta\cancel.c:120: (apr_err=155015)
..\..\..\subversion\libsvn_client\repos_diff.c:710: (apr_err=155015)
..\..\..\subversion\libsvn_client\merge.c:2234: (apr_err=155015)
..\..\..\subversion\libsvn_wc\adm_ops.c:1069: (apr_err=155015)
..\..\..\subversion\libsvn_wc\adm_ops.c:937: (apr_err=155015)
svn: E155015:
'C:\SVN\src-trunk\Debug\subversion\tests\cmdline\svn-test-work\working_copies\merge_tree_conflict_tests-24\branch\C'
is an existing item in conflict; please mark the conflict as resolved
before adding a new item here
>svn st
? C branch\C
> local delete, incoming delete upon merge
Summary of conflicts:
Tree conflicts: 1
### Prior to r1127134 the merge raised a tree conflict:
trunk@1127133>svn merge ^^/A branch
--- Merging r2 through r4 into 'branch':
C branch\C
--- Recording mergeinfo for merge of r2 through r4 into 'branch':
U branch
Summary of conflicts:
Tree conflicts: 1
trunk@1127133>svn st
M branch
! C branch\C
> local delete, incoming delete upon merge
Summary of conflicts:
Tree conflicts: 1
Not exactly sure how to fix this...I can look at it further tomorrow,
just wanted to get your thoughts if you have time.
Paul
> switch (status)
> {
> case svn_wc__db_status_not_present:
>
> 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=1127134&r1=1127133&r2=1127134&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
> (original)
> +++ subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py Tue May
> 24 16:35:14 2011
> @@ -1134,23 +1134,17 @@ def actual_only_node_behaviour(sbox):
>
> # add
> expected_stdout = None
> - expected_stderr = ".*foo.*not found.*"
> + expected_stderr = ".*foo.*is an existing item in conflict.*"
> run_and_verify_svn(None, expected_stdout, expected_stderr,
> "add", foo_path)
>
> # add (with an existing obstruction of foo)
> - ### this does not error out -- needs review
> svntest.main.file_write(foo_path, "This is an obstruction of foo.\n")
> - expected_stdout = "A.*foo"
> - expected_stderr = []
> + expected_stdout = None
> + expected_stderr = ".*foo.*is an existing item in conflict.*"
> run_and_verify_svn(None, expected_stdout, expected_stderr,
> "add", foo_path)
> - ### for now, ignore the fact that add succeeds, revert the entire
> - ### working copy, and repeat the merge so we can test more commands
> - svntest.main.run_svn(None, "revert", "-R", wc_dir)
> os.remove(foo_path) # remove obstruction
> - svntest.main.run_svn(None, "merge", '-c', '4', A_copy_url,
> - os.path.join(wc_dir, 'A'))
>
> # blame (praise, annotate, ann)
> expected_stdout = None
> @@ -1267,17 +1261,10 @@ def actual_only_node_behaviour(sbox):
> run_and_verify_svn(None, expected_stdout, expected_stderr,
> "mergeinfo", A_copy_url + '/foo', foo_path)
> # mkdir
> - ### this does not error out -- needs review
> expected_stdout = None
> - expected_stderr = []
> + expected_stderr = ".*foo.*is an existing item in conflict.*"
> run_and_verify_svn(None, expected_stdout, expected_stderr,
> "mkdir", foo_path)
> - ### for now, ignore the fact that mkdir succeeds, revert the entire
> - ### working copy, and repeat the merge so we can test more commands
> - svntest.main.run_svn(None, "revert", "-R", wc_dir)
> - os.rmdir(foo_path) # remove obstruction
> - svntest.main.run_svn(None, "merge", '-c', '4', A_copy_url,
> - os.path.join(wc_dir, 'A'))
>
> # move (mv, rename, ren)
> expected_stdout = None
>
>
>
