You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2014/03/15 10:57:07 UTC

svn commit: r1577812 - in /subversion/trunk/subversion: libsvn_client/merge.c tests/cmdline/merge_tests.py

Author: rhuijben
Date: Sat Mar 15 09:57:06 2014
New Revision: 1577812

URL: http://svn.apache.org/r1577812
Log:
Resolve a segfault when merging a directory delete using --force.

This regression was introduced in 1.8.0, with the implementation of proper
tree conflict calculations for directory deletes.

This code path for directory deletes during merge doesn't have much test
coverage as it was very easy to trigger working copy side effects before we
introduced the single-db storage in 1.7.

* subversion/libsvn_client/merge.c
  (merge_dir_deleted): When using force_delete, don't continue looking for
    changes using a baton that isn't filled in this mode. This resolves a
    segfault.

* subversion/tests/cmdline/merge_tests.py
  (merge_dir_delete_force): Add very minimalistic testcase for this issue.

Modified:
    subversion/trunk/subversion/libsvn_client/merge.c
    subversion/trunk/subversion/tests/cmdline/merge_tests.py

Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1577812&r1=1577811&r2=1577812&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Sat Mar 15 09:57:06 2014
@@ -3116,66 +3116,69 @@ merge_dir_deleted(const char *relpath,
                             scratch_pool, scratch_pool));
 
   if (merge_b->force_delete)
-    same = TRUE;
+    {
+      /* In this legacy mode we just assume that a directory delete
+         matches any directory. db->delete_state is NULL */
+      same = TRUE;
+    }
   else
     {
       /* Compare the properties */
       SVN_ERR(properties_same_p(&same, left_props, working_props,
                                 scratch_pool));
-    }
+      delb = db->delete_state;
+      assert(delb != NULL);
 
-  delb = db->delete_state;
-  assert(delb != NULL);
+      if (! same)
+        {
+          delb->found_edit = TRUE;
+        }
+      else
+        {
+          store_path(delb->compared_abspaths, local_abspath);
+        }
 
-  if (! same)
-    {
-      delb->found_edit = TRUE;
-    }
-  else
-    {
-      store_path(delb->compared_abspaths, local_abspath);
-    }
+      if (delb->del_root != db)
+        return SVN_NO_ERROR;
 
-  if (delb->del_root != db)
-    return SVN_NO_ERROR;
+      if (delb->found_edit)
+        same = FALSE;
+      else if (merge_b->force_delete)
+        same = TRUE;
+      else
+        {
+          apr_array_header_t *ignores;
+          svn_error_t *err;
+          same = TRUE;
 
-  if (delb->found_edit)
-    same = FALSE;
-  else if (merge_b->force_delete)
-    same = TRUE;
-  else
-    {
-      apr_array_header_t *ignores;
-      svn_error_t *err;
-      same = TRUE;
+          SVN_ERR(svn_wc_get_default_ignores(&ignores, merge_b->ctx->config,
+                                             scratch_pool));
 
-      SVN_ERR(svn_wc_get_default_ignores(&ignores, merge_b->ctx->config,
-                                         scratch_pool));
+          /* None of the descendants was modified, but maybe there are
+             descendants we haven't walked?
 
-      /* None of the descendants was modified, but maybe there are
-         descendants we haven't walked?
+             Note that we aren't interested in changes, as we already verified
+             changes in the paths touched by the merge. And the existence of
+             other paths is enough to mark the directory edited */
+          err = svn_wc_walk_status(merge_b->ctx->wc_ctx, local_abspath,
+                                   svn_depth_infinity, TRUE /* get-all */,
+                                   FALSE /* no-ignore */,
+                                   TRUE /* ignore-text-mods */, ignores,
+                                   verify_touched_by_del_check, delb,
+                                   merge_b->ctx->cancel_func,
+                                   merge_b->ctx->cancel_baton,
+                                   scratch_pool);
 
-         Note that we aren't interested in changes, as we already verified
-         changes in the paths touched by the merge. And the existence of
-         other paths is enough to mark the directory edited */
-      err = svn_wc_walk_status(merge_b->ctx->wc_ctx, local_abspath,
-                               svn_depth_infinity, TRUE /* get-all */,
-                               FALSE /* no-ignore */,
-                               TRUE /* ignore-text-mods */, ignores,
-                               verify_touched_by_del_check, delb,
-                               merge_b->ctx->cancel_func,
-                               merge_b->ctx->cancel_baton,
-                               scratch_pool);
+          if (err)
+            {
+              if (err->apr_err != SVN_ERR_CEASE_INVOCATION)
+                return svn_error_trace(err);
 
-      if (err)
-        {
-          if (err->apr_err != SVN_ERR_CEASE_INVOCATION)
-            return svn_error_trace(err);
+              svn_error_clear(err);
+            }
 
-          svn_error_clear(err);
+          same = ! delb->found_edit;
         }
-
-      same = ! delb->found_edit;
     }
 
   if (same && !merge_b->dry_run)

Modified: subversion/trunk/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tests.py?rev=1577812&r1=1577811&r2=1577812&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tests.py Sat Mar 15 09:57:06 2014
@@ -18829,6 +18829,30 @@ def conflict_naming(sbox):
   svntest.actions.run_and_verify_status(wc_dir, expected_status)
   svntest.actions.verify_disk(wc_dir, expected_disk)
 
+def merge_dir_delete_force(sbox):
+  "merge a directory delete with --force"
+
+  sbox.build()
+
+  sbox.simple_rm('A/D/G')
+  sbox.simple_commit() # r2
+
+  sbox.simple_update(revision=1)
+
+  # Just merging r2 on r1 succeeds
+  svntest.actions.run_and_verify_svn(sbox.wc_dir, None, [],
+                                     'merge', '-c2', '^/', sbox.wc_dir,
+                                     '--ignore-ancestry')
+
+  # Bring working copy to r1 again
+  svntest.actions.run_and_verify_svn(sbox.wc_dir, None, [],
+                                     'revert', '-R', sbox.wc_dir)
+
+  # But when using --force this same merge caused a segfault in 1.8.0-1.8.8
+  svntest.actions.run_and_verify_svn(sbox.wc_dir, None, [],
+                                     'merge', '-c2', '^/', sbox.wc_dir,
+                                     '--ignore-ancestry', '--force')
+
 ########################################################################
 # Run the tests
 
@@ -18975,6 +18999,7 @@ test_list = [ None,
               conflicted_split_merge_with_resolve,
               merge_to_empty_target_merge_to_infinite_target,
               conflict_naming,
+              merge_dir_delete_force,
              ]
 
 if __name__ == '__main__':