svn-3486

Version:

1.7.0

Bug Link:

http://subversion.tigris.org/issues/show_bug.cgi?id=3486

Patch Link:

source code patch: http://svn.apache.org/viewvc?diff_format=h&view=revision&revision=1058269

reproduction: http://svn.haxx.se/dev/archive-2010-11/att-0290/treeconflict_1.txt

Symptom:

‘svn merge’ can fail with the error message, “svn: Attempt to add tree conflict that already exists” and quit, without proceed to process other files. The correct behavior should be even one file operation failed, it should still continue to process all the remaining files.

 

When two users of one repositories modify the branch and merge back to the trunk, it might fail due to the tree conflict. The example is to merge the file of branch, which has been deleted in the trunk.

How it is diagnosed:

source code analysis

 

The error message routine:

 

/* in ‘svn_wc__add_tree_conflict’ function, the error happens due to the tree confliction.

In this case, we don’t have to stop the execution, we can just keep going and there should be no problem.

*/

svn_error_t *

svn_wc__add_tree_conflict(…)

{

  …

  /* Re-adding an existing tree conflict victim is an error. */

  SVN_ERR(svn_wc__db_op_read_tree_conflict(&existing_conflict, wc_ctx->db,

                                           conflict->local_abspath,

                                           scratch_pool, scratch_pool));

  /* the error message is printed below. When ‘existing_conflict’ is not NULL,

      then there is a conflict! This ‘existing_conflict’ is from the return value

      of function svn_wc__db_op_read_tree_conflict. So it is return value check

      However, this function is not a libc/system call and eventually won’t

      end up in a libc/sys-call. So it is their own library.  */

  if (existing_conflict != NULL)

        return svn_error_createf(SVN_ERR_WC_CORRUPT, NULL,

                             _("Attempt to add tree conflict that already "

                               "exists at '%s'"),

                             svn_dirent_local_style(conflict->local_abspath,

                                                    scratch_pool));

 

  …

}

 

 

Root Cause:

Brief:

We should not abort the entire merge due to the mismatch of revision ranges. We always need to check for an existing tree conflict before attempting to record a new one. If existing, we should do nothing.

Detail:

/* in the function ‘tree_conflict’, ‘svn_wc__add_tree_conflict’ is called and returns error. To prevent the error message, we need to check whether there’s a conflict or not before calling ‘svn_wc__add_tree_conflict’.  */

static svn_error_t*

tree_conflict(merge_cmd_baton_t *merge_b,

                  const char *victim_abspath,

                  svn_node_kind_t node_kind,

                  svn_wc_conflict_action_t action,

                  svn_wc_conflict_reason_t reason)

{

  svn_wc_conflict_description2_t *conflict;

 

  if (merge_b->record_only || merge_b->dry_run)

        return SVN_NO_ERROR;

 

  SVN_ERR(make_tree_conflict(&conflict, merge_b, victim_abspath,

                                 node_kind, action, reason));

 

  return svn_error_return(

    svn_wc__add_tree_conflict(merge_b->ctx->wc_ctx, conflict, merge_b->pool));

}

 

--- subversion/trunk/subversion/libsvn_client/merge.c           2011/01/12 18:36:57              1058268

+++ subversion/trunk/subversion/libsvn_client/merge.c        2011/01/12 18:40:24              1058269

@@ -529,7 +529,9 @@ make_tree_conflict(svn_wc_conflict_descr

 }

 

 /* Record a tree conflict in the WC, unless this is a dry run or a record-

- * only merge.

+ * only merge, or if a tree conflict is already flagged for the VICTIM_PATH.

+ * (The latter can happen if a merge-tracking-aware merge is doing multiple

+ * editor drives because of a gap in the range of eligible revisions.)

  *

  * The tree conflict, with its victim specified by VICTIM_PATH, is

  * assumed to have happened during a merge using merge baton MERGE_B.

@@ -546,16 +548,25 @@ tree_conflict(merge_cmd_baton_t *merge_b

               svn_wc_conflict_action_t action,

               svn_wc_conflict_reason_t reason)

 {

  …

 

/* first, check if there’s a conflict or not. If not, call ‘svn_wc__add_tree_conflict’ */

+  SVN_ERR(svn_wc__get_tree_conflict(&existing_conflict, merge_b->ctx->wc_ctx,

+                                    victim_abspath, merge_b->pool,

+                                    merge_b->pool));

+  if (existing_conflict == NULL)

+        {

+          /* There is no existing tree conflict so it is safe to add one. */

   SVN_ERR(make_tree_conflict(&conflict, merge_b, victim_abspath,

                              node_kind, action, reason));

+      SVN_ERR(svn_wc__add_tree_conflict(merge_b->ctx->wc_ctx, conflict,

+                                        merge_b->pool));

+        }

 

-  return svn_error_return(

-    svn_wc__add_tree_conflict(merge_b->ctx->wc_ctx, conflict, merge_b->pool));

+  return SVN_NO_ERROR;

 }

 

 /* Similar to tree_conflict(), but if this is an "add" action and there

 

Failure symptom category

refuse valid input, early termination

Is there any log message?

Yes

How can we automatically insert the log message?

frequent logging patterns of return value checked.