You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by pb...@apache.org on 2010/06/04 20:23:02 UTC

svn commit: r951517 - in /subversion/trunk/subversion: libsvn_wc/props.c tests/cmdline/prop_tests.py

Author: pburba
Date: Fri Jun  4 18:23:02 2010
New Revision: 951517

URL: http://svn.apache.org/viewvc?rev=951517&view=rev
Log:
Fix some property merge conflict bugs.

1) Incoming delete on a local add of a different value is now a
  conflict.  Previously it was a clean merge and the prop was
  deleted.

2) Incoming delete on a local delete where the incoming base value
  differs from the local value is now a conflict.  Previously
  this caused a segfault.

* subversion/libsvn_wc/props.c

 (generate_conflict_message): Handle incoming delete on local add and
  incoming delete on local delete of a different prop value.  Consistently
  use a trailing ',' after the first line of each prej conflict message.

 (apply_single_prop_delete): Stop considering an incoming delete on a local
  add as a merge.

* subversion/tests/cmdline/prop_tests.py

 (prop_reject_grind): Start testing incoming delete on local delete of
  different prop value.  Verify the resulting *.prej file.

Review by: gstein

Modified:
    subversion/trunk/subversion/libsvn_wc/props.c
    subversion/trunk/subversion/tests/cmdline/prop_tests.py

Modified: subversion/trunk/subversion/libsvn_wc/props.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=951517&r1=951516&r2=951517&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/props.c (original)
+++ subversion/trunk/subversion/libsvn_wc/props.c Fri Jun  4 18:23:02 2010
@@ -852,34 +852,51 @@ generate_conflict_message(const char *pr
       /* Attempting to delete the value INCOMING_BASE.  */
       SVN_ERR_ASSERT_NO_RETURN(incoming_base != NULL);
 
+      /* Are we trying to delete a local addition? */
+      if (original == NULL && mine != NULL)
+        return svn_string_createf(result_pool,
+                                  _("Trying to delete property '%s' with "
+                                    "value '%s',\nbut property has been "
+                                    "locally added with value '%s'."),
+                                  propname, incoming_base->data,
+                                  mine->data);
+
       /* A conflict can only occur if we originally had the property;
          otherwise, we would have merged the property-delete into the
          non-existent property.  */
       SVN_ERR_ASSERT_NO_RETURN(original != NULL);
 
-      if (mine && svn_string_compare(original, incoming_base))
+      if (svn_string_compare(original, incoming_base))
+        {
+          if (mine)
+            /* We were trying to delete the correct property, but an edit
+               caused the conflict.  */
+            return svn_string_createf(result_pool,
+                                      _("Trying to delete property '%s' with "
+                                        "value '%s',\nbut it has been modified "
+                                        "from '%s' to '%s'."),
+                                      propname, incoming_base->data,
+                                      original->data, mine->data);
+        }
+      else if (mine == NULL)
         {
-          /* We were trying to delete the correct property, but an edit
-             caused the conflict.  */
+          /* We were trying to delete the property, but we have locally
+             deleted the same property, but with a different value. */
           return svn_string_createf(result_pool,
                                     _("Trying to delete property '%s' with "
-                                      "value '%s'\nbut it has been modified "
-                                      "from '%s' to '%s'."),
+                                      "value '%s',\nbut property with value "
+                                      "'%s' is locally deleted."),
                                     propname, incoming_base->data,
-                                    original->data, mine->data);
+                                    original->data);          
         }
 
       /* We were trying to delete INCOMING_BASE but our ORIGINAL is
          something else entirely.  */
       SVN_ERR_ASSERT_NO_RETURN(!svn_string_compare(original, incoming_base));
 
-      /* ### wait. what if we had a different property and locally
-         ### deleted it? the statement below is gonna blow up.
-         ### we could have: local-add, local-edit, local-del, or just
-         ### something different (and unchanged).  */
       return svn_string_createf(result_pool,
                                 _("Trying to delete property '%s' with "
-                                  "value '%s'\nbut the local value is "
+                                  "value '%s',\nbut the local value is "
                                   "'%s'."),
                                 propname, incoming_base->data, mine->data);
     }
@@ -1466,12 +1483,27 @@ apply_single_prop_delete(svn_wc_notify_s
 
   if (! base_val)
     {
-      /* ### what about working_val? what if we locally-added?  */
-
-      apr_hash_set(working_props, propname, APR_HASH_KEY_STRING, NULL);
-      if (old_val)
-        /* This is a merge, merging a delete into non-existent */
-        set_prop_merge_state(state, svn_wc_notify_state_merged);
+      if (working_val
+          && !svn_string_compare(working_val, old_val))
+        {
+          /* We are trying to delete a locally-added prop. */
+          SVN_ERR(maybe_generate_propconflict(conflict_remains,
+                                              db, local_abspath,
+                                              left_version, right_version,
+                                              is_dir, propname,
+                                              working_props, old_val, NULL,
+                                              base_val, working_val,
+                                              conflict_func, conflict_baton,
+                                              dry_run, scratch_pool));
+        }
+      else
+        {
+          apr_hash_set(working_props, propname, APR_HASH_KEY_STRING, NULL);
+          if (old_val)
+            /* This is a merge, merging a delete into non-existent
+               property or a local addition of same prop value. */
+            set_prop_merge_state(state, svn_wc_notify_state_merged);
+        }
     }
 
   else if (svn_string_compare(base_val, old_val))
@@ -1496,7 +1528,8 @@ apply_single_prop_delete(svn_wc_notify_s
              }
          }
        else
-         /* The property is locally deleted, so it's a merge */
+         /* The property is locally deleted from the same value, so it's
+            a merge */
          set_prop_merge_state(state, svn_wc_notify_state_merged);
     }
 

Modified: subversion/trunk/subversion/tests/cmdline/prop_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/prop_tests.py?rev=951517&r1=951516&r2=951517&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/prop_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/prop_tests.py Fri Jun  4 18:23:02 2010
@@ -1787,6 +1787,7 @@ def prop_reject_grind(sbox):
 
   iota_path = sbox.ospath('iota')
   mu_path = sbox.ospath('A/mu')
+  mu_prej_path = sbox.ospath('A/mu.prej')
 
   # Create r2 with all the properties we intend to use as incoming-change,
   # and as incoming-delete. Also set up our local-edit and local-delete
@@ -1830,8 +1831,7 @@ def prop_reject_grind(sbox):
   sbox.simple_propdel('del.edit', iota_path)
   sbox.simple_propdel('del.edit2', iota_path)
   sbox.simple_propdel('del.diff', iota_path)
-  ### don't delete this. causes a segfault :-)
-  #sbox.simple_propdel('del.del', iota_path)
+  sbox.simple_propdel('del.del', iota_path)
   sbox.simple_propdel('del.add', iota_path)
   sbox.simple_commit()
 
@@ -1852,9 +1852,69 @@ def prop_reject_grind(sbox):
   svntest.main.run_svn(False, 'merge', '-r2:3', sbox.repo_url + '/iota',
                        mu_path)
 
-  ### need to verify mu.prej
-  ### note that del.add has been erroneously deleted!
-
+  # Check that A/mu.prej reports the expected conflicts:
+  expected_prej = svntest.verify.UnorderedOutput([
+   "Trying to change property 'edit.none' from 'repos' to 'repos.changed',\n"
+   "but the property does not exist.\n",
+
+   "Trying to delete property 'del.del' with value 'repos',\n"
+   "but property with value 'local' is locally deleted.\n",
+
+   "Trying to delete property 'del.edit' with value 'repos',\n"
+   "but the local value is 'local.changed'.\n",
+
+   "Trying to change property 'edit.del' from 'repos' to 'repos.changed',\n"
+   "but it has been locally deleted.\n",
+
+   "Trying to change property 'edit.edit' from 'repos' to 'repos.changed',\n"
+   "but the property has been locally changed from 'local' to 'local.changed'.\n",
+
+   "Trying to delete property 'del.edit2' with value 'repos',\n"
+   "but it has been modified from 'repos' to 'repos.changed'.\n",
+
+   "Trying to delete property 'del.add' with value 'repos',\n"
+   "but property has been locally added with value 'local'.\n",
+
+   "Trying to delete property 'del.diff' with value 'repos',\n"
+   "but the local value is 'local'.\n",
+
+   "Trying to change property 'edit.add' from 'repos' to 'repos.changed',\n"
+   "but property has been locally added with value 'local'.\n",
+
+   "Trying to change property 'edit.diff' from 'repos' to 'repos.changed',\n"
+   "but property already exists with value 'local'.\n",
+
+   "Trying to add new property 'add.add' with value 'repos',\n"
+   "but property already exists with value 'local'.\n",
+
+   "Trying to add new property 'add.diff' with value 'repos',\n"
+   "but property already exists with value 'local'.\n",
+
+   "Trying to create property 'add.del' with value 'repos',\n"
+   "but it has been locally deleted.\n",
+
+   "Trying to add new property 'add.edit' with value 'repos',\n"
+   "but property already exists with value 'local.changed'.\n",
+
+   "\n"  
+   ])
+
+  # Get the contents of mu.prej.  The error messages in the prej file are
+  # two lines each, but there is no guarantee as to order, so remove the
+  # newline between each two line error message and then split the whole
+  # thing into a list of strings on the remaining newline...
+  raw_prej = open(mu_prej_path,
+                     'r').read().replace('\nbut', ' but').split('\n')
+  # ...then put the newlines back in each list item.  That leaves us with
+  # list of two lines strings we can compare to the unordered expected
+  # prej file.
+  actual_prej = []
+  for line in raw_prej:
+      repaired_line = line.replace(' but', '\nbut')
+      actual_prej.append(repaired_line + '\n')
+  
+  svntest.verify.verify_outputs("Expected mu.prej doesn't match actual mu.prej",
+                                actual_prej, None, expected_prej, None)
 
 def obstructed_subdirs(sbox):
   """test properties of obstructed subdirectories"""