subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From svn-r...@apache.org
Subject 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
Date Thu, 19 Mar 2015 18:23:34 GMT
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. */



Mime
View raw message