You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2013/05/09 15:09:42 UTC

svn commit: r1480616 - /subversion/trunk/subversion/libsvn_wc/conflicts.c

Author: stsp
Date: Thu May  9 13:09:42 2013
New Revision: 1480616

URL: http://svn.apache.org/r1480616
Log:
Simplify some conflict resolution code in libsvn_wc.

Remove the resolve_conflict_on_node() helper function, which was a thin
wrapper around three other functions for resolving text, prop, and tree
conflicts. Which of the other functions was called could be controlled
by the caller with a set of boolean parameters. Because every caller was
setting only one of these boolean parameters, the case where multiple
wrapped functions are called never occurred. So the abstraction provided
by resolve_conflict_on_node() is not necessary.

Adjust all callers of resolve_conflict_on_node() to call the appropriate
text/prop/tree function directly. This will also make it easier in the future
to add specific functionality to resolution of text, prop, or tree conflicts,
without having to pass data through the resolve_conflict_on_node() wrapper.

No functional change.

* subversion/libsvn_wc/conflicts.c
  (resolve_text_conflict_on_node,
   resolve_prop_conflict_on_node,
   resolve_tree_conflict_on_node): Merge code from resolve_conflict_on_node()
    into these functions, and adjust their interfaces so that former callers
    of resolve_conflict_on_node() can easily make use of these functions.
  (resolve_conflict_on_node): Remove.
  (svn_wc__mark_resolved_text_conflict,
   svn_wc__mark_resolved_prop_conflicts,
   conflict_status_walker): Switch callers from resolve_conflict_on_node()
    to the more specific functions, as appropriate.

Modified:
    subversion/trunk/subversion/libsvn_wc/conflicts.c

Modified: subversion/trunk/subversion/libsvn_wc/conflicts.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/conflicts.c?rev=1480616&r1=1480615&r2=1480616&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/conflicts.c (original)
+++ subversion/trunk/subversion/libsvn_wc/conflicts.c Thu May  9 13:09:42 2013
@@ -2352,14 +2352,11 @@ remove_artifact_file_if_exists(svn_skel_
 }
 
 /*
- * Resolve the text conflict found in DB/LOCAL_ABSPATH/CONFLICTS
- * according to CONFLICT_CHOICE.  (Don't mark it as resolved.)
+ * Resolve the text conflict found in DB/LOCAL_ABSPATH according
+ * to CONFLICT_CHOICE.
  *
- * If there were any marker files recorded and present on disk, append to
- * *WORK_ITEMS work items to remove them, and set *REMOVED_REJECT_FILES
- * to TRUE.  Otherwise, don't change *REMOVED_REJECT_FILES.
- *
- * It is an error if there is no text conflict.
+ * It is not an error if there is no text conflict. If a text conflict
+ * existed and was resolved, set *DID_RESOLVE to TRUE, else set it to FALSE.
  *
  * Note: When there are no conflict markers to remove there is no existing
  * text conflict; just a database containing old information, which we should
@@ -2368,13 +2365,12 @@ remove_artifact_file_if_exists(svn_skel_
  * Subversion 1.0.
  */
 static svn_error_t *
-resolve_text_conflict_on_node(svn_boolean_t *removed_reject_files,
-                              svn_skel_t **work_items,
+resolve_text_conflict_on_node(svn_boolean_t *did_resolve,
                               svn_wc__db_t *db,
                               const char *local_abspath,
-                              svn_wc_operation_t operation,
-                              svn_skel_t *conflicts,
                               svn_wc_conflict_choice_t conflict_choice,
+                              svn_cancel_func_t cancel_func,
+                              void *cancel_baton,
                               apr_pool_t *scratch_pool)
 {
   const char *conflict_old = NULL;
@@ -2382,6 +2378,23 @@ resolve_text_conflict_on_node(svn_boolea
   const char *conflict_working = NULL;
   const char *auto_resolve_src;
   svn_skel_t *work_item;
+  svn_skel_t *work_items = NULL;
+  svn_skel_t *conflicts;
+  svn_wc_operation_t operation;
+  svn_boolean_t text_conflicted;
+
+  *did_resolve = FALSE;
+
+  SVN_ERR(svn_wc__db_read_conflict(&conflicts, db, local_abspath,
+                                   scratch_pool, scratch_pool));
+  if (!conflicts)
+    return SVN_NO_ERROR;
+
+  SVN_ERR(svn_wc__conflict_read_info(&operation, NULL, &text_conflicted,
+                                     NULL, NULL, db, local_abspath, conflicts,
+                                     scratch_pool, scratch_pool));
+  if (!text_conflicted)
+    return SVN_NO_ERROR;
 
   SVN_ERR(svn_wc__conflict_read_text_conflict(&conflict_working,
                                               &conflict_old,
@@ -2459,12 +2472,12 @@ resolve_text_conflict_on_node(svn_boolea
       SVN_ERR(svn_wc__wq_build_file_copy_translated(
                 &work_item, db, local_abspath,
                 auto_resolve_src, local_abspath, scratch_pool, scratch_pool));
-      *work_items = svn_wc__wq_merge(*work_items, work_item, scratch_pool);
+      work_items = svn_wc__wq_merge(work_items, work_item, scratch_pool);
 
       SVN_ERR(svn_wc__wq_build_sync_file_flags(&work_item, db,
                                                local_abspath,
                                                scratch_pool, scratch_pool));
-      *work_items = svn_wc__wq_merge(*work_items, work_item, scratch_pool);
+      work_items = svn_wc__wq_merge(work_items, work_item, scratch_pool);
     }
 
   /* Legacy behavior: Only report text conflicts as resolved when at least
@@ -2473,33 +2486,36 @@ resolve_text_conflict_on_node(svn_boolea
      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_item, removed_reject_files,
+  SVN_ERR(remove_artifact_file_if_exists(&work_item, did_resolve,
                                          db, local_abspath, conflict_old,
                                          scratch_pool, scratch_pool));
-  *work_items = svn_wc__wq_merge(*work_items, work_item, scratch_pool);
+  work_items = svn_wc__wq_merge(work_items, work_item, scratch_pool);
 
-  SVN_ERR(remove_artifact_file_if_exists(&work_item, removed_reject_files,
+  SVN_ERR(remove_artifact_file_if_exists(&work_item, did_resolve,
                                          db, local_abspath, conflict_new,
                                          scratch_pool, scratch_pool));
-  *work_items = svn_wc__wq_merge(*work_items, work_item, scratch_pool);
+  work_items = svn_wc__wq_merge(work_items, work_item, scratch_pool);
 
-  SVN_ERR(remove_artifact_file_if_exists(&work_item, removed_reject_files,
+  SVN_ERR(remove_artifact_file_if_exists(&work_item, did_resolve,
                                          db, local_abspath, conflict_working,
                                          scratch_pool, scratch_pool));
-  *work_items = svn_wc__wq_merge(*work_items, work_item, scratch_pool);
+  work_items = svn_wc__wq_merge(work_items, work_item, scratch_pool);
+
+  SVN_ERR(svn_wc__db_op_mark_resolved(db, local_abspath,
+                                      TRUE, FALSE, FALSE,
+                                      work_items, scratch_pool));
+  SVN_ERR(svn_wc__wq_run(db, local_abspath, cancel_func, cancel_baton,
+                         scratch_pool));
 
   return SVN_NO_ERROR;
 }
 
 /*
- * Resolve the property conflicts found in DB/LOCAL_ABSPATH/CONFLICTS
- * according to CONFLICT_CHOICE.  (Don't mark it as resolved.)
- *
- * If there was a reject file recorded and present on disk, append to
- * *WORK_ITEMS a work item to remove it, and set *REMOVED_REJECT_FILE
- * to TRUE.  Otherwise, don't change *REMOVED_REJECT_FILE.
+ * Resolve the property conflicts found in DB/LOCAL_ABSPATH according
+ * to CONFLICT_CHOICE.
  *
- * It is an error if there is no prop conflict.
+ * It is not an error if there is no prop conflict. If a prop conflict
+ * existed and was resolved, set *DID_RESOLVE to TRUE, else set it to FALSE.
  *
  * Note: When there are no conflict markers on-disk to remove there is
  * no existing text conflict (unless we are still in the process of
@@ -2530,13 +2546,12 @@ resolve_text_conflict_on_node(svn_boolea
  *
  */
 static svn_error_t *
-resolve_prop_conflict_on_node(svn_boolean_t *removed_reject_file,
-                              svn_skel_t **work_items,
+resolve_prop_conflict_on_node(svn_boolean_t *did_resolve,
                               svn_wc__db_t *db,
                               const char *local_abspath,
-                              svn_wc_operation_t operation,
-                              svn_skel_t *conflicts,
                               svn_wc_conflict_choice_t conflict_choice,
+                              svn_cancel_func_t cancel_func,
+                              void *cancel_baton,
                               apr_pool_t *scratch_pool)
 {
   const char *prop_reject_file;
@@ -2546,6 +2561,24 @@ resolve_prop_conflict_on_node(svn_boolea
   apr_hash_t *conflicted_props;
   apr_hash_t *old_props;
   apr_hash_t *resolve_from = NULL;
+  svn_skel_t *work_items = NULL;
+  svn_skel_t *conflicts;
+  svn_wc_operation_t operation;
+  svn_boolean_t prop_conflicted;
+
+  *did_resolve = FALSE;
+
+  SVN_ERR(svn_wc__db_read_conflict(&conflicts, db, local_abspath,
+                                   scratch_pool, scratch_pool));
+
+  if (!conflicts)
+    return SVN_NO_ERROR;
+
+  SVN_ERR(svn_wc__conflict_read_info(&operation, NULL, NULL, &prop_conflicted,
+                                     NULL, db, local_abspath, conflicts,
+                                     scratch_pool, scratch_pool));
+  if (!prop_conflicted)
+    return SVN_NO_ERROR;
 
   SVN_ERR(svn_wc__conflict_read_prop_conflict(&prop_reject_file,
                                               &mine_props, &their_old_props,
@@ -2624,29 +2657,33 @@ resolve_prop_conflict_on_node(svn_boolea
   {
     svn_skel_t *work_item;
 
-    SVN_ERR(remove_artifact_file_if_exists(&work_item, removed_reject_file,
+    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);
+    work_items = svn_wc__wq_merge(work_items, work_item, 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;
 }
 
 /*
- * Resolve the tree conflict found in DB/LOCAL_ABSPATH/CONFLICTS
- * according to CONFLICT_CHOICE.  (Don't mark it as resolved.)
+ * Resolve the tree conflict found in DB/LOCAL_ABSPATH according to
+ * CONFLICT_CHOICE.
  *
- * ### ... append to *WORK_ITEMS work items to ...?
+ * It is not an error if there is no tree conflict. If a tree conflict
+ * existed and was resolved, set *DID_RESOLVE to TRUE, else set it to FALSE.
  *
- * It is an error if there is no tree conflict.
+ * It is not an error if there is no tree conflict.
  */
 static svn_error_t *
-resolve_tree_conflict_on_node(svn_skel_t **work_items,
+resolve_tree_conflict_on_node(svn_boolean_t *did_resolve,
                               svn_wc__db_t *db,
                               const char *local_abspath,
-                              svn_wc_operation_t operation,
-                              svn_skel_t *conflicts,
                               svn_wc_conflict_choice_t conflict_choice,
                               svn_wc_notify_func2_t notify_func,
                               void *notify_baton,
@@ -2656,7 +2693,22 @@ resolve_tree_conflict_on_node(svn_skel_t
 {
   svn_wc_conflict_reason_t reason;
   svn_wc_conflict_action_t action;
-  svn_boolean_t did_resolve = FALSE;
+  svn_skel_t *conflicts;
+  svn_wc_operation_t operation;
+  svn_boolean_t tree_conflicted;
+
+  *did_resolve = FALSE;
+
+  SVN_ERR(svn_wc__db_read_conflict(&conflicts, db, local_abspath,
+                                   scratch_pool, scratch_pool));
+  if (!conflicts)
+    return SVN_NO_ERROR;
+
+  SVN_ERR(svn_wc__conflict_read_info(&operation, NULL, NULL, NULL,
+                                     &tree_conflicted, db, local_abspath,
+                                     conflicts, scratch_pool, scratch_pool));
+  if (!tree_conflicted)
+    return SVN_NO_ERROR;
 
   SVN_ERR(svn_wc__conflict_read_tree_conflict(&reason, &action, NULL,
                                               db, local_abspath,
@@ -2676,7 +2728,7 @@ resolve_tree_conflict_on_node(svn_skel_t
               SVN_ERR(svn_wc__db_resolve_break_moved_away_children(
                         db, local_abspath, notify_func, notify_baton,
                         scratch_pool));
-              did_resolve = TRUE;
+              *did_resolve = TRUE;
             }
           else if (conflict_choice == svn_wc_conflict_choose_mine_conflict)
             {
@@ -2687,7 +2739,7 @@ resolve_tree_conflict_on_node(svn_skel_t
               SVN_ERR(svn_wc__db_resolve_delete_raise_moved_away(
                         db, local_abspath, notify_func, notify_baton,
                         scratch_pool));
-              did_resolve = TRUE;
+              *did_resolve = TRUE;
             }
           else
             return svn_error_createf(SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE,
@@ -2712,7 +2764,7 @@ resolve_tree_conflict_on_node(svn_skel_t
                         notify_func, notify_baton,
                         cancel_func, cancel_baton,
                         scratch_pool));
-              did_resolve = TRUE;
+              *did_resolve = TRUE;
             }
           else if (conflict_choice == svn_wc_conflict_choose_merged)
             {
@@ -2727,7 +2779,7 @@ resolve_tree_conflict_on_node(svn_skel_t
                                                           notify_func,
                                                           notify_baton,
                                                           scratch_pool));
-              did_resolve = TRUE;
+              *did_resolve = TRUE;
             }
           else
             return svn_error_createf(SVN_ERR_WC_CONFLICT_RESOLVER_FAILURE,
@@ -2740,7 +2792,7 @@ resolve_tree_conflict_on_node(svn_skel_t
         }
     }
 
-  if (! did_resolve && conflict_choice != svn_wc_conflict_choose_merged)
+  if (! *did_resolve && conflict_choice != svn_wc_conflict_choose_merged)
     {
       /* For other tree conflicts, there is no way to pick
        * theirs-full or mine-full, etc. Throw an error if the
@@ -2754,110 +2806,13 @@ resolve_tree_conflict_on_node(svn_skel_t
                                                       scratch_pool));
     }
 
+  SVN_ERR(svn_wc__db_op_mark_resolved(db, local_abspath, FALSE, FALSE, TRUE,
+                                      NULL, scratch_pool));
+  SVN_ERR(svn_wc__wq_run(db, local_abspath, cancel_func, cancel_baton,
+                         scratch_pool));
   return SVN_NO_ERROR;
 }
 
-/* Conflict resolution involves removing the conflict files, if they exist,
-   and clearing the conflict filenames from the entry.  The latter needs to
-   be done whether or not the conflict files exist.
-
-   ### This func combines *resolving* and *marking as resolved* -- seems poor.
-
-   LOCAL_ABSPATH in DB is the path to the item to be resolved.
-   RESOLVE_TEXT, RESOLVE_PROPS and RESOLVE_TREE are TRUE iff text, property
-   and tree conflicts respectively are to be resolved.
-
-   If this call marks any conflict as resolved, set *DID_RESOLVE to true,
-   else to false.
-   If asked to resolve a text or prop conflict, only set *DID_RESOLVE
-   to true if a conflict marker file was present, because if no marker
-   file was present then the conflict is considered to be marked as
-   resolved already.
-   ### If asked to resolve a tree conflict, always set *DID_RESOLVE to true.
-       This would make sense if 'resolve_tree' is only requested when
-       there is in fact a tree conflict to be resolved, but, for
-       consistency with text & prop conflicts, the code should probably
-       say "if (resolve_tree && tree_conflicted) *did_resolve = TRUE".
-
-   See svn_wc_resolved_conflict5() for how CONFLICT_CHOICE behaves.
-*/
-static svn_error_t *
-resolve_conflict_on_node(svn_boolean_t *did_resolve,
-                         svn_wc__db_t *db,
-                         const char *local_abspath,
-                         svn_boolean_t resolve_text,
-                         svn_boolean_t resolve_props,
-                         svn_boolean_t resolve_tree,
-                         svn_wc_conflict_choice_t conflict_choice,
-                         svn_wc_notify_func2_t notify_func,
-                         void *notify_baton,
-                         svn_cancel_func_t cancel_func,
-                         void *cancel_baton,
-                         apr_pool_t *scratch_pool)
-{
-  svn_skel_t *conflicts;
-  svn_wc_operation_t operation;
-  svn_boolean_t text_conflicted;
-  svn_boolean_t prop_conflicted;
-  svn_boolean_t tree_conflicted;
-  svn_skel_t *work_items = NULL;
-
-  *did_resolve = FALSE;
-
-  SVN_ERR(svn_wc__db_read_conflict(&conflicts, db, local_abspath,
-                                   scratch_pool, scratch_pool));
-
-  if (!conflicts)
-    return SVN_NO_ERROR;
-
-  SVN_ERR(svn_wc__conflict_read_info(&operation, NULL, &text_conflicted,
-                                     &prop_conflicted, &tree_conflicted,
-                                     db, local_abspath, conflicts,
-                                     scratch_pool, scratch_pool));
-
-  if (resolve_text && text_conflicted)
-    SVN_ERR(resolve_text_conflict_on_node(did_resolve, &work_items,
-                                          db, local_abspath,
-                                          operation, conflicts,
-                                          conflict_choice,
-                                          scratch_pool));
-
-  if (resolve_props && prop_conflicted)
-    SVN_ERR(resolve_prop_conflict_on_node(did_resolve, &work_items,
-                                          db, local_abspath,
-                                          operation, conflicts,
-                                          conflict_choice,
-                                          scratch_pool));
-
-  if (resolve_tree)
-    {
-      SVN_ERR(resolve_tree_conflict_on_node(&work_items,
-                                            db, local_abspath,
-                                            operation, conflicts,
-                                            conflict_choice,
-                                            notify_func, notify_baton,
-                                            cancel_func, cancel_baton,
-                                            scratch_pool));
-      *did_resolve = TRUE;
-    }
-
-  if (resolve_text || resolve_props || resolve_tree)
-    {
-      SVN_ERR(svn_wc__db_op_mark_resolved(db, local_abspath,
-                                          resolve_text, resolve_props,
-                                          resolve_tree, work_items,
-                                          scratch_pool));
-
-      /* Run the work queue to remove conflict marker files. */
-      SVN_ERR(svn_wc__wq_run(db, local_abspath,
-                             cancel_func, cancel_baton,
-                             scratch_pool));
-    }
-
-  return SVN_NO_ERROR;
-}
-
-
 svn_error_t *
 svn_wc__mark_resolved_text_conflict(svn_wc__db_t *db,
                                     const char *local_abspath,
@@ -2865,15 +2820,11 @@ svn_wc__mark_resolved_text_conflict(svn_
 {
   svn_boolean_t ignored_result;
 
-  return svn_error_trace(resolve_conflict_on_node(
+  return svn_error_trace(resolve_text_conflict_on_node(
                            &ignored_result,
                            db, local_abspath,
-                           TRUE /* resolve_text */,
-                           FALSE /* resolve_props */,
-                           FALSE /* resolve_tree */,
                            svn_wc_conflict_choose_merged,
-                           NULL, NULL, /* notify_func */
-                           NULL, NULL, /* cancel_func */
+                           NULL, NULL,
                            scratch_pool));
 }
 
@@ -2884,15 +2835,11 @@ svn_wc__mark_resolved_prop_conflicts(svn
 {
   svn_boolean_t ignored_result;
 
-  return svn_error_trace(resolve_conflict_on_node(
+  return svn_error_trace(resolve_prop_conflict_on_node(
                            &ignored_result,
                            db, local_abspath,
-                           FALSE /* resolve_text */,
-                           TRUE /* resolve_props */,
-                           FALSE /* resolve_tree */,
                            svn_wc_conflict_choose_merged,
-                           NULL, NULL, /* notify_func */
-                           NULL, NULL, /* cancel_func */
+                           NULL, NULL,
                            scratch_pool));
 }
 
@@ -2974,18 +2921,15 @@ conflict_status_walker(void *baton,
           case svn_wc_conflict_kind_tree:
             if (!cswb->resolve_tree)
               break;
-            SVN_ERR(resolve_conflict_on_node(&did_resolve,
-                                             db,
-                                             local_abspath,
-                                             FALSE /* resolve_text */,
-                                             FALSE /* resolve_props */,
-                                             TRUE /* resolve_tree */,
-                                             my_choice,
-                                             cswb->notify_func,
-                                             cswb->notify_baton,
-                                             cswb->cancel_func,
-                                             cswb->cancel_baton,
-                                             iterpool));
+            SVN_ERR(resolve_tree_conflict_on_node(&did_resolve,
+                                                  db,
+                                                  local_abspath,
+                                                  my_choice,
+                                                  cswb->notify_func,
+                                                  cswb->notify_baton,
+                                                  cswb->cancel_func,
+                                                  cswb->cancel_baton,
+                                                  iterpool));
 
             resolved = TRUE;
             break;
@@ -2994,18 +2938,13 @@ conflict_status_walker(void *baton,
             if (!cswb->resolve_text)
               break;
 
-            SVN_ERR(resolve_conflict_on_node(&did_resolve,
-                                             db,
-                                             local_abspath,
-                                             TRUE /* resolve_text */,
-                                             FALSE /* resolve_props */,
-                                             FALSE /* resolve_tree */,
-                                             my_choice,
-                                             cswb->notify_func,
-                                             cswb->notify_baton,
-                                             cswb->cancel_func,
-                                             cswb->cancel_baton,
-                                             iterpool));
+            SVN_ERR(resolve_text_conflict_on_node(&did_resolve,
+                                                  db,
+                                                  local_abspath,
+                                                  my_choice,
+                                                  cswb->cancel_func,
+                                                  cswb->cancel_baton,
+                                                  iterpool));
 
             if (did_resolve)
               resolved = TRUE;
@@ -3015,8 +2954,8 @@ conflict_status_walker(void *baton,
             if (!cswb->resolve_prop)
               break;
 
-            /* ### this is bogus. resolve_conflict_on_node() does not handle
-               ### individual property resolution.  */
+            /* ### this is bogus. resolve_prop_conflict_on_node() does not
+             * ### handle individual property resolution.  */
             if (*cswb->resolve_prop != '\0' &&
                 strcmp(cswb->resolve_prop, cd->property_name) != 0)
               {
@@ -3025,18 +2964,13 @@ conflict_status_walker(void *baton,
 
 
             /* We don't have property name handling here yet :( */
-            SVN_ERR(resolve_conflict_on_node(&did_resolve,
-                                             db,
-                                             local_abspath,
-                                             FALSE /* resolve_text */,
-                                             TRUE /* resolve_props */,
-                                             FALSE /* resolve_tree */,
-                                             my_choice,
-                                             cswb->notify_func,
-                                             cswb->notify_baton,
-                                             cswb->cancel_func,
-                                             cswb->cancel_baton,
-                                             iterpool));
+            SVN_ERR(resolve_prop_conflict_on_node(&did_resolve,
+                                                  db,
+                                                  local_abspath,
+                                                  my_choice,
+                                                  cswb->cancel_func,
+                                                  cswb->cancel_baton,
+                                                  iterpool));
 
             if (did_resolve)
               resolved = TRUE;