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 2013/10/28 19:13:56 UTC

svn commit: r1536464 - in /subversion/trunk/subversion: libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c tests/cmdline/move_tests.py tests/libsvn_wc/op-depth-test.c

Author: rhuijben
Date: Mon Oct 28 18:13:55 2013
New Revision: 1536464

URL: http://svn.apache.org/r1536464
Log:
Fix handling of move tracking when deleting a tree where some subtree was
moved through.

This fixes the move handling for:

$ svn mkdir A A/A
$ svn ci -m ""
$ svn mv A A2
$ svn mv A2/A A
$ svn rm A2
$ svn
$ svn mv A/mu A/NEW/mu

and:

$ svn mkdir --parent A/A/A
$ svn ci -m ""
$ svn mkdir B
$ svn mv A B/A
$ svn mv B/A/A B/AA
$ svn mv B/AA/A AA
$ svn rm B

that before this patch generate an inconsistent database state.

* subversion/libsvn_wc/wc-queries.sql
  (STMT_SELECT_MOVED_FROM_RELPATH): Remove unneeded comment.
    move_to is UNIQUE per working copy.
  (STMT_SELECT_MOVED_FROM_FOR_DELETE): New statement.

* subversion/libsvn_wc/wc_db.c
  (moved_node_t): Add variable.
  (resolve_moved_from): New helper function.
  (delete_node): When deleting a node that is moved through the
    to be deleted tree, properly record that it must be updated.
    Resolve paths of moved nodes when necessary.
    Remove moved from information for the root of the delete.

* subversion/tests/cmdline/move_tests.py
  (move_del_moved): Remove XFail marker.

* subversion/tests/libsvn_wc/op-depth-test.c
  (move_replace_ancestor_with_child): Fix assumption.
  (test_funcs): Remove two XFail markers.

Modified:
    subversion/trunk/subversion/libsvn_wc/wc-queries.sql
    subversion/trunk/subversion/libsvn_wc/wc_db.c
    subversion/trunk/subversion/tests/cmdline/move_tests.py
    subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c

Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=1536464&r1=1536463&r2=1536464&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
+++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Mon Oct 28 18:13:55 2013
@@ -1503,7 +1503,6 @@ WHERE wc_id = ?1
   AND presence=MAP_NORMAL
   AND file_external IS NULL
 
-/* ### FIXME: op-depth?  What about multiple moves? */
 -- STMT_SELECT_MOVED_FROM_RELPATH
 SELECT local_relpath, op_depth FROM nodes
 WHERE wc_id = ?1 AND moved_to = ?2 AND op_depth > 0
@@ -1548,6 +1547,16 @@ WHERE wc_id = ?1
                     WHERE o.wc_id = ?1
                       AND o.local_relpath = ?2)
 
+-- STMT_SELECT_MOVED_FROM_FOR_DELETE
+SELECT local_relpath, op_depth,
+       (SELECT CASE WHEN r.moved_here THEN r.op_depth END FROM nodes r
+        WHERE r.wc_id = ?1
+          AND r.local_relpath = n.local_relpath
+          AND r.op_depth < n.op_depth
+        ORDER BY r.op_depth DESC LIMIT 1) AS moved_here_op_depth
+ FROM nodes n
+WHERE wc_id = ?1 AND moved_to = ?2 AND op_depth > 0
+
 -- STMT_UPDATE_MOVED_TO_DESCENDANTS
 UPDATE nodes SET moved_to = RELPATH_SKIP_JOIN(?2, ?3, moved_to)
  WHERE wc_id = ?1

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1536464&r1=1536463&r2=1536464&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Mon Oct 28 18:13:55 2013
@@ -7460,8 +7460,86 @@ struct moved_node_t {
 
   /* The op-depth of the deleted node at the source of the move. */
   int op_depth;
+
+  /* When >= 1 the op_depth at which local_relpath was moved to its
+     location. Used to find its original location outside the delete */
+  int moved_from_depth;
 };
 
+/* Helper function to resolve the original location of local_relpath at OP_DEPTH
+   before it was moved into the tree rooted at ROOT_RELPATH. */
+static svn_error_t *
+resolve_moved_from(const char **moved_from_relpath,
+                   int *moved_from_op_depth,
+                   svn_wc__db_wcroot_t *wcroot,
+                   const char *root_relpath,
+                   const char *local_relpath,
+                   int op_depth,
+                   apr_pool_t *result_pool,
+                   apr_pool_t *scratch_pool)
+{
+  const char *suffix = "";
+  svn_sqlite__stmt_t *stmt;
+  const char *m_from_relpath;
+  int m_from_op_depth;
+  int m_move_from_depth;
+  svn_boolean_t have_row;
+
+  while (relpath_depth(local_relpath) > op_depth)
+    {
+      const char *name;
+      svn_relpath_split(&local_relpath, &name, local_relpath, scratch_pool);
+      suffix = svn_relpath_join(suffix, name, scratch_pool);
+    }
+
+  SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+                                    STMT_SELECT_MOVED_FROM_FOR_DELETE));
+  SVN_ERR(svn_sqlite__bindf(stmt, "is",
+                            wcroot->wc_id, local_relpath));
+  SVN_ERR(svn_sqlite__step(&have_row, stmt));
+
+  if (!have_row)
+    {
+      /* assert(have_row); */
+      *moved_from_relpath = NULL;
+      *moved_from_op_depth = -1;
+
+      SVN_ERR(svn_sqlite__reset(stmt));
+
+      return SVN_NO_ERROR;
+    }
+
+  m_from_relpath = svn_sqlite__column_text(stmt, 0, scratch_pool);
+  m_from_op_depth = svn_sqlite__column_int(stmt, 1);
+  m_move_from_depth = svn_sqlite__column_int(stmt, 2);
+
+  SVN_ERR(svn_sqlite__reset(stmt));
+
+  if (! svn_relpath_skip_ancestor(root_relpath, m_from_relpath))
+    {
+      *moved_from_relpath = svn_relpath_join(m_from_relpath, suffix,
+                                             result_pool);
+      *moved_from_op_depth = m_from_op_depth; /* ### Ok? */
+      return SVN_NO_ERROR;
+    }
+  else if (!m_move_from_depth)
+    {
+      *moved_from_relpath = NULL;
+      *moved_from_op_depth = -1;
+      return SVN_NO_ERROR;
+    }
+
+  return svn_error_trace(
+        resolve_moved_from(moved_from_relpath,
+                           moved_from_op_depth,
+                           wcroot,
+                           root_relpath,
+                           svn_relpath_join(m_from_relpath, suffix,
+                                            scratch_pool),
+                           m_move_from_depth,
+                           result_pool, scratch_pool));
+}
+
 static svn_error_t *
 delete_node(void *baton,
             svn_wc__db_wcroot_t *wcroot,
@@ -7564,6 +7642,7 @@ delete_node(void *baton,
                                                          part, scratch_pool);
           moved_node->op_depth = move_op_depth;
           moved_node->moved_to_relpath = b->moved_to_relpath;
+          moved_node->moved_from_depth = -1;
 
           APR_ARRAY_PUSH(moved_nodes, const struct moved_node_t *) = moved_node;
         }
@@ -7577,6 +7656,7 @@ delete_node(void *baton,
           moved_node->local_relpath = local_relpath;
           moved_node->op_depth = delete_depth;
           moved_node->moved_to_relpath = b->moved_to_relpath;
+          moved_node->moved_from_depth = -1;
 
           APR_ARRAY_PUSH(moved_nodes, const struct moved_node_t *) = moved_node;
         }
@@ -7597,6 +7677,7 @@ delete_node(void *baton,
    * is now implied by the deletion of their parent (i.e. this node). */
     {
       apr_pool_t *iterpool;
+      int i;
 
       SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
                                         STMT_SELECT_MOVED_FOR_DELETE));
@@ -7610,20 +7691,19 @@ delete_node(void *baton,
           const char *child_relpath = svn_sqlite__column_text(stmt, 0, NULL);
           const char *mv_to_relpath = svn_sqlite__column_text(stmt, 1, NULL);
           int child_op_depth = svn_sqlite__column_int(stmt, 2);
-          svn_boolean_t child_moved_here = !svn_sqlite__column_is_null(stmt, 3);
-          
+          int moved_from_depth = -1;
           svn_boolean_t fixup = FALSE;
 
-          if (! b->moved_to_relpath)
+          if (! b->moved_to_relpath
+              && ! svn_relpath_skip_ancestor(local_relpath, mv_to_relpath))
             {
-              int moved_here_depth;
+              /* a NULL moved_here_depth will be reported as 0 */
+              int moved_here_depth = svn_sqlite__column_int(stmt, 3);
 
               /* Plain delete. Fixup move information of descendants that were
                  moved here, or that were moved out */
 
-              if (child_moved_here
-                  && (moved_here_depth = svn_sqlite__column_int(stmt, 3)) 
-                               >= delete_depth)
+              if (moved_here_depth >= delete_depth)
                 {
                   /* The move we recorded here must be moved to the location
                      this node had before it was moved here.
@@ -7632,18 +7712,17 @@ delete_node(void *baton,
                      in several places within the to be deleted tree */
 
                   /* ### TODO: Add logic */
+                  fixup = TRUE;
+                  moved_from_depth = moved_here_depth;
                 }
               else
                 {
-                  if (!svn_relpath_skip_ancestor(local_relpath, mv_to_relpath))
-                    {
-                      /* Update the op-depth of an moved node below this tree */
-                      fixup = TRUE;
-                      child_op_depth = delete_depth;
-                    }
+                  /* Update the op-depth of an moved node below this tree */
+                  fixup = TRUE;
+                  child_op_depth = delete_depth;
                 }
             }
-          else
+          else if (b->moved_to_relpath)
             {
               /* The node is moved to a new location */
 
@@ -7683,11 +7762,12 @@ delete_node(void *baton,
 
           if (fixup)
             {
-              mn = apr_pcalloc(scratch_pool, sizeof(struct moved_node_t));
+              mn = apr_palloc(scratch_pool, sizeof(struct moved_node_t));
 
               mn->local_relpath = apr_pstrdup(scratch_pool, child_relpath);
               mn->moved_to_relpath = apr_pstrdup(scratch_pool, mv_to_relpath);
               mn->op_depth = child_op_depth;
+              mn->moved_from_depth = moved_from_depth;
 
               if (!moved_nodes)
                 moved_nodes = apr_array_make(scratch_pool, 1,
@@ -7697,17 +7777,48 @@ delete_node(void *baton,
 
           SVN_ERR(svn_sqlite__step(&have_row, stmt));
         }
-      svn_pool_destroy(iterpool);
       SVN_ERR(svn_sqlite__reset(stmt));
+
+      for (i = 0; moved_nodes && (i < moved_nodes->nelts); i++)
+        {
+          struct moved_node_t *mn = APR_ARRAY_IDX(moved_nodes, i,
+                                                  struct moved_node_t *);
+
+          if (mn->moved_from_depth > 0)
+            {
+              svn_pool_clear(iterpool);
+
+              SVN_ERR(resolve_moved_from(&mn->local_relpath, &mn->op_depth,
+                                         wcroot, local_relpath,
+                                         mn->local_relpath,
+                                         mn->moved_from_depth,
+                                         scratch_pool, iterpool));
+
+              if (!mn->local_relpath)
+                svn_sort__array_delete(moved_nodes, i--, 1);
+            }
+        }
+
+      svn_pool_destroy(iterpool);
     }
 
   if (!b->moved_to_relpath)
     {
       SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
-          STMT_CLEAR_MOVED_TO_DESCENDANTS));
+                                        STMT_CLEAR_MOVED_TO_DESCENDANTS));
       SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id,
-          local_relpath));
+                                local_relpath));
       SVN_ERR(svn_sqlite__update(NULL, stmt));
+
+      if (op_root)
+        {
+          SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+                                            STMT_CLEAR_MOVED_TO_FROM_DEST));
+          SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id,
+                                    local_relpath));
+
+          SVN_ERR(svn_sqlite__update(NULL, stmt));
+        }
     }
 
   if (op_root)

Modified: subversion/trunk/subversion/tests/cmdline/move_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/move_tests.py?rev=1536464&r1=1536463&r2=1536464&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/move_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/move_tests.py Mon Oct 28 18:13:55 2013
@@ -1501,7 +1501,6 @@ def move_many_update_add(sbox):
                                         wc_dir, '--accept', 'mine-conflict')
 
 @Issue(4437)
-@XFail()
 def move_del_moved(sbox):
   "delete moved node, still a move"
 
@@ -1522,6 +1521,30 @@ def move_del_moved(sbox):
   # deleted there.
   svntest.actions.run_and_verify_status(wc_dir, expected_status)
 
+def copy_move_commit(sbox):
+  "copy, move and commit"
+
+  sbox.build()
+  wc_dir = sbox.wc_dir
+    #repro
+    # Prepare
+    #   - Create folder aaa
+    #   - Add file bbb.sql
+    #     create table bbb (Id int not null)
+    #   - Commit
+    # Repro Issue 2
+    #    - Copy folder aaa under same parent folder (i.e. as a sibling). (using Ctrl drag/drop). 
+    #      Creates Copy of aaa
+    #    - Rename Copy of aaa to eee
+    #    - Commit
+    #      Get error need to update
+    #    - Update
+    #    - Commit
+    #      Get error need to update
+
+  sbox.simple_copy('A/D/G', 'A/D/GG')
+  sbox.simple_move('A/D/GG', 'A/D/GG-moved')
+  sbox.simple_commit('A/D/GG-moved')
 
 #######################################################################
 # Run the tests
@@ -1538,6 +1561,7 @@ test_list = [ None,
               move_many_update_delete,
               move_many_update_add,
               move_del_moved,
+              copy_move_commit,
             ]
 
 if __name__ == '__main__':

Modified: subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c?rev=1536464&r1=1536463&r2=1536464&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c Mon Oct 28 18:13:55 2013
@@ -8658,7 +8658,7 @@ move_replace_ancestor_with_child(const s
         { 0, "A/A",         "normal",       1, "A/A" },
 
         { 1, "A",           "normal",       1, "A/A", MOVED_HERE },
-        { 1, "A/A",         "base-deleted", NO_COPY_FROM, "A/A" },
+        { 1, "A/A",         "base-deleted", NO_COPY_FROM, "A" },
 
         { 0 },
       };
@@ -8910,9 +8910,9 @@ struct svn_test_descriptor_t test_funcs[
                        "move more than once, delete intermediate"),
     SVN_TEST_OPTS_XFAIL(move_revert_intermediate,
                        "move more than once, revert intermediate"),
-    SVN_TEST_OPTS_XFAIL(move_replace_ancestor_with_child,
-                        "move replace ancestor with child"),
-    SVN_TEST_OPTS_XFAIL(move_twice_within_delete,
-                        "move twice and then delete"),
+    SVN_TEST_OPTS_PASS(move_replace_ancestor_with_child,
+                       "move replace ancestor with child"),
+    SVN_TEST_OPTS_PASS(move_twice_within_delete,
+                       "move twice and then delete"),
     SVN_TEST_NULL
   };