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/02/21 14:29:26 UTC

svn commit: r1448636 - in /subversion/trunk/subversion: libsvn_wc/copy.c libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/cmdline/copy_tests.py tests/libsvn_wc/op-depth-test.c

Author: rhuijben
Date: Thu Feb 21 13:29:25 2013
New Revision: 1448636

URL: http://svn.apache.org/r1448636
Log:
When mixed revision moves are not an error, make the move handling degrade
to a proper copy+delete when encountering mixed revisions instead of still
recording a move, which most likely breaks later.

Also degrade/convert to a no-move when a node is moved back to its original
location.

* subversion/libsvn_wc/copy.c
  (copy_or_move):
    Always degrade to copy and delete when doing a multi-db move.
    Mixed revision-ness is only interesting when moving base nodes as
    the result only applies to BASE anyway. Degrade to copy when the
    mixed revision check says we can't move instead of recording a
    move for mixed revisions anyway.
    After a move ask wc_db to undo move-backs.

* subversion/libsvn_wc/wc-queries.sql
  (STMT_SELECT_MOVED_BACK,
   STMT_DELETE_MOVED_BACK): New query.

* subversion/libsvn_wc/wc_db.c
  (handle_move_back,
   svn_wc__db_op_handle_move_back): New function.

* subversion/libsvn_wc/wc_db.h
  (svn_wc__db_op_handle_move_back): New function.

* subversion/tests/cmdline/copy_tests.py
  (move_file_back_and_forth,
   move_dir_back_and_forth): Update expected result.

* subversion/tests/libsvn_wc/op-depth-test.c
  (move_to_swap): Extend testcase. Add commented section testing a different
    problem.
  (test_list): Mark mixed_rev_move() as WIMP, because this triggers a copy
    and delete now.

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

Modified: subversion/trunk/subversion/libsvn_wc/copy.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/copy.c?rev=1448636&r1=1448635&r2=1448636&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/copy.c (original)
+++ subversion/trunk/subversion/libsvn_wc/copy.c Thu Feb 21 13:29:25 2013
@@ -531,6 +531,7 @@ copy_or_move(svn_boolean_t *move_degrade
   const char *src_wcroot_abspath;
   const char *dst_wcroot_abspath;
   svn_boolean_t within_one_wc;
+  svn_wc__db_status_t src_status;
   svn_error_t *err;
 
   SVN_ERR_ASSERT(svn_dirent_is_absolute(src_abspath));
@@ -541,7 +542,7 @@ copy_or_move(svn_boolean_t *move_degrade
   /* Ensure DSTDIR_ABSPATH belongs to the same repository as SRC_ABSPATH;
      throw an error if not. */
   {
-    svn_wc__db_status_t src_status, dstdir_status;
+    svn_wc__db_status_t dstdir_status;
     const char *src_repos_root_url, *dst_repos_root_url;
     const char *src_repos_uuid, *dst_repos_uuid;
 
@@ -728,13 +729,13 @@ copy_or_move(svn_boolean_t *move_degrade
 
   within_one_wc = (strcmp(src_wcroot_abspath, dst_wcroot_abspath) == 0);
 
-  if (move_degraded_to_copy != NULL)
+  if (is_move
+      && !within_one_wc)
     {
-      /* Cross-WC moves cannot be tracked.
-       * Degrade such moves to a copy+delete. */
-      *move_degraded_to_copy = (is_move && !within_one_wc);
-      if (*move_degraded_to_copy)
-        is_move = FALSE;
+      if (move_degraded_to_copy)
+        *move_degraded_to_copy = TRUE;
+
+      is_move = FALSE;
     }
 
   if (!within_one_wc)
@@ -754,7 +755,8 @@ copy_or_move(svn_boolean_t *move_degrade
     }
   else
     {
-      if (is_move && !allow_mixed_revisions)
+      if (is_move
+          && src_status == svn_wc__db_status_normal)
         {
           svn_revnum_t min_rev;
           svn_revnum_t max_rev;
@@ -764,12 +766,20 @@ copy_or_move(svn_boolean_t *move_degrade
                                                src_abspath, FALSE, scratch_pool));
           if (SVN_IS_VALID_REVNUM(min_rev) && SVN_IS_VALID_REVNUM(max_rev) &&
               min_rev != max_rev)
-            return svn_error_createf(SVN_ERR_WC_MIXED_REVISIONS, NULL,
-                                     _("Cannot move mixed-revision subtree '%s' "
-                                       "[%ld:%ld]; try updating it first"),
-                                       svn_dirent_local_style(src_abspath,
-                                                              scratch_pool),
-                                       min_rev, max_rev);
+            {
+              if (!allow_mixed_revisions)
+                return svn_error_createf(SVN_ERR_WC_MIXED_REVISIONS, NULL,
+                                         _("Cannot move mixed-revision "
+                                           "subtree '%s' [%ld:%ld]; "
+                                           "try updating it first"),
+                                         svn_dirent_local_style(src_abspath,
+                                                                scratch_pool),
+                                         min_rev, max_rev);
+
+              is_move = FALSE;
+              if (move_degraded_to_copy)
+                *move_degraded_to_copy = TRUE;
+            }
         }
 
       err = copy_versioned_dir(db, src_abspath, dst_abspath, dst_abspath,
@@ -782,6 +792,13 @@ copy_or_move(svn_boolean_t *move_degrade
   if (err && svn_error_find_cause(err, SVN_ERR_CANCELLED))
     return svn_error_trace(err);
 
+  if (is_move)
+    err = svn_error_compose_create(err,
+                svn_wc__db_op_handle_move_back(move_degraded_to_copy,
+                                               db, dst_abspath, src_abspath,
+                                               NULL /* work_items */,
+                                               scratch_pool));
+
   /* Run the work queue with the remaining work */
   SVN_ERR(svn_error_compose_create(
                                 err,

Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=1448636&r1=1448635&r2=1448636&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
+++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Thu Feb 21 13:29:25 2013
@@ -432,7 +432,39 @@ SELECT moved_here, presence, repos_path,
 FROM nodes
 WHERE wc_id = ?1 AND local_relpath = ?2 AND op_depth >= ?3
 ORDER BY op_depth
-                  
+
+-- STMT_SELECT_MOVED_BACK
+SELECT u.local_relpath,
+       u.presence, u.repos_id, u.repos_path, u.revision,
+       l.presence, l.repos_id, l.repos_path, l.revision,
+       u.moved_here, u.moved_to
+FROM nodes u
+LEFT OUTER JOIN nodes l ON l.wc_id = ?1
+                       AND l.local_relpath = u.local_relpath
+                       AND l.op_depth = ?3
+WHERE u.wc_id = ?1
+  AND u.local_relpath = ?2
+  AND u.op_depth = ?4
+UNION ALL
+SELECT u.local_relpath,
+       u.presence, u.repos_id, u.repos_path, u.revision,
+       l.presence, l.repos_id, l.repos_path, l.revision,
+       u.moved_here, NULL
+FROM nodes u
+LEFT OUTER JOIN nodes l ON l.wc_id=?1
+                       AND l.local_relpath=u.local_relpath
+                       AND l.op_depth=?3
+WHERE u.wc_id = ?1
+  AND IS_STRICT_DESCENDANT_OF(u.local_relpath, ?2)
+  AND u.op_depth = ?4
+
+-- STMT_DELETE_MOVED_BACK
+DELETE FROM nodes
+WHERE wc_id = ?1
+  AND (local_relpath = ?2
+       OR IS_STRICT_DESCENDANT_OF(local_relpath, ?2))
+  AND op_depth = ?3
+
 -- STMT_DELETE_LOCK
 DELETE FROM lock
 WHERE repos_id = ?1 AND repos_relpath = ?2

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1448636&r1=1448635&r2=1448636&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Thu Feb 21 13:29:25 2013
@@ -4614,6 +4614,221 @@ svn_wc__db_op_copy(svn_wc__db_t *db,
   return SVN_NO_ERROR;
 }
 
+/* The txn body of svn_wc__db_op_handle_move_back */
+static svn_error_t *
+handle_move_back(svn_boolean_t *moved_back,
+                 svn_wc__db_wcroot_t *wcroot,
+                 const char *local_relpath,
+                 const char *moved_from_relpath,
+                 const svn_skel_t *work_items,
+                 apr_pool_t *scratch_pool)
+{
+  svn_sqlite__stmt_t *stmt;
+  svn_wc__db_status_t status;
+  svn_boolean_t op_root;
+  svn_boolean_t have_more_work;
+  int from_op_depth = 0;
+  svn_boolean_t have_row;
+  svn_boolean_t different = FALSE;
+
+  SVN_ERR(add_work_items(wcroot->sdb, work_items, scratch_pool));
+
+  SVN_ERR(svn_wc__db_read_info_internal(&status, NULL, NULL, NULL, NULL, NULL,
+                                        NULL, NULL, NULL, NULL, NULL, NULL,
+                                        NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+                                        &op_root, NULL, NULL, NULL,
+                                        &have_more_work, NULL,
+                                        wcroot, local_relpath,
+                                        scratch_pool, scratch_pool));
+
+  if (status != svn_wc__db_status_added || !op_root)
+    return SVN_NO_ERROR;
+
+  /* We have two cases here: BASE-move-back and WORKING-move-back */
+  if (have_more_work)
+    SVN_ERR(op_depth_of(&from_op_depth, wcroot,
+                        svn_relpath_dirname(local_relpath, scratch_pool)));
+  else
+    from_op_depth = 0;
+
+  SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+                                    STMT_SELECT_MOVED_BACK));
+
+  SVN_ERR(svn_sqlite__bindf(stmt, "isdd", wcroot->wc_id,
+                                          local_relpath,
+                                          from_op_depth,
+                                          relpath_depth(local_relpath)));
+
+  SVN_ERR(svn_sqlite__step(&have_row, stmt));
+
+  SVN_ERR_ASSERT(have_row); /* We checked that the node is an op-root */
+
+  {
+    svn_boolean_t moved_here = svn_sqlite__column_boolean(stmt, 9);
+    const char *moved_to = svn_sqlite__column_text(stmt, 10, NULL);
+
+    if (!moved_here
+        || !moved_to
+        || strcmp(moved_to, moved_from_relpath))
+      {
+        different = TRUE;
+        have_row = FALSE;
+      }
+  }
+
+  while (have_row)
+    {
+      svn_wc__db_status_t upper_status;
+      svn_wc__db_status_t lower_status;
+
+      upper_status = svn_sqlite__column_token(stmt, 1, presence_map);
+
+      if (svn_sqlite__column_is_null(stmt, 5))
+        {
+          /* No lower layer replaced. */
+          if (upper_status != svn_wc__db_status_not_present)
+            {
+              different = TRUE;
+              break;
+            }
+          continue;
+        }
+
+      lower_status = svn_sqlite__column_token(stmt, 5, presence_map);
+
+      if (upper_status != lower_status)
+        {
+          different = TRUE;
+          break;
+        }
+
+      if (upper_status == svn_wc__db_status_not_present
+          || upper_status == svn_wc__db_status_excluded)
+        continue; /* Nothing to check */
+      else if (upper_status != svn_wc__db_status_normal)
+        {
+          /* Not a normal move. Mixed revision move? */
+          different = TRUE;
+          break;
+        }
+
+      {
+        const char *upper_repos_relpath;
+        const char *lower_repos_relpath;
+
+        upper_repos_relpath = svn_sqlite__column_text(stmt, 3, NULL);
+        lower_repos_relpath = svn_sqlite__column_text(stmt, 7, NULL);
+
+        if (! upper_repos_relpath
+            || strcmp(upper_repos_relpath, lower_repos_relpath))
+          {
+            different = TRUE;
+            break;
+          }
+      }
+
+      {
+        svn_revnum_t upper_rev;
+        svn_revnum_t lower_rev;
+
+        upper_rev = svn_sqlite__column_revnum(stmt, 4);
+        lower_rev = svn_sqlite__column_revnum(stmt, 8);
+
+        if (upper_rev != lower_rev)
+          {
+            different = TRUE;
+            break;
+          }
+      }
+
+      {
+        apr_int64_t upper_repos_id;
+        apr_int64_t lower_repos_id;
+
+        upper_repos_id = svn_sqlite__column_int64(stmt, 2);
+        lower_repos_id = svn_sqlite__column_int64(stmt, 6);
+
+        if (upper_repos_id != lower_repos_id)
+          {
+            different = TRUE;
+            break;
+          }
+      }
+
+      /* Check moved_here? */
+
+      SVN_ERR(svn_sqlite__step(&have_row, stmt));
+    }
+  SVN_ERR(svn_sqlite__reset(stmt));
+
+  if (! different)
+    {
+      /* Ok, we can now safely remove this complete move, because we
+         determined that it 100% matches the layer below it. */
+
+      /* ### We could copy the recorded timestamps from the higher to the
+             lower layer in an attempt to improve status performance, but
+             generally these values should be the same anyway as it was
+             a no-op move. */
+      SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+                                        STMT_DELETE_MOVED_BACK));
+
+      SVN_ERR(svn_sqlite__bindf(stmt, "isd", wcroot->wc_id,
+                                             local_relpath,
+                                             relpath_depth(local_relpath)));
+
+      SVN_ERR(svn_sqlite__step_done(stmt));
+
+      if (moved_back)
+        *moved_back = TRUE;
+    }
+
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_wc__db_op_handle_move_back(svn_boolean_t *moved_back,
+                               svn_wc__db_t *db,
+                               const char *local_abspath,
+                               const char *moved_from_abspath,
+                               const svn_skel_t *work_items,
+                               apr_pool_t *scratch_pool)
+{
+  svn_wc__db_wcroot_t *wcroot;
+  const char *local_relpath;
+  const char *moved_from_relpath;
+  SVN_ERR_ASSERT(svn_dirent_is_absolute(local_abspath));
+
+  SVN_ERR(svn_wc__db_wcroot_parse_local_abspath(&wcroot, &local_relpath, db,
+                                                local_abspath,
+                                                scratch_pool, scratch_pool));
+  VERIFY_USABLE_WCROOT(wcroot);
+
+  if (moved_back)
+    *moved_back = FALSE;
+
+  moved_from_relpath = svn_dirent_skip_ancestor(wcroot->abspath,
+                                                moved_from_abspath);
+
+  if (! local_relpath[0]
+      || !moved_from_relpath)
+    {
+       /* WC-Roots can't be moved */
+      SVN_ERR(add_work_items(wcroot->sdb, work_items, scratch_pool));
+      return SVN_NO_ERROR;
+    }
+
+  SVN_WC__DB_WITH_TXN(handle_move_back(moved_back, wcroot, local_relpath,
+                                       moved_from_relpath, work_items,
+                                       scratch_pool),
+                      wcroot);
+
+  SVN_ERR(flush_entries(wcroot, local_abspath, svn_depth_infinity,
+                        scratch_pool));
+
+  return SVN_NO_ERROR;
+}
+
 
 /* The recursive implementation of svn_wc__db_op_copy_shadowed_layer.
  *

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=1448636&r1=1448635&r2=1448636&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Thu Feb 21 13:29:25 2013
@@ -1303,6 +1303,22 @@ svn_wc__db_op_copy(svn_wc__db_t *db,
                    const svn_skel_t *work_items,
                    apr_pool_t *scratch_pool);
 
+/* Checks if LOCAL_ABSPATH represents a move back to its original location,
+ * and if it is reverts the move while keeping local changes after it has been
+ * moved from MOVED_FROM_ABSPATH.
+ *
+ * If MOVED_BACK is not NULL, set *MOVED_BACK to TRUE when a move was reverted,
+ * otherwise to FALSE.
+ */
+svn_error_t *
+svn_wc__db_op_handle_move_back(svn_boolean_t *moved_back,
+                               svn_wc__db_t *db,
+                               const char *local_abspath,
+                               const char *moved_from_abspath,
+                               const svn_skel_t *work_items,
+                               apr_pool_t *scratch_pool);
+
+
 /* Copy the leaves of the op_depth layer directly shadowed by the operation
  * of SRC_ABSPATH (so SRC_ABSPATH must be an op_root) to dst_abspaths
  * parents layer.

Modified: subversion/trunk/subversion/tests/cmdline/copy_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/copy_tests.py?rev=1448636&r1=1448635&r2=1448636&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/copy_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/copy_tests.py Thu Feb 21 13:29:25 2013
@@ -2432,20 +2432,11 @@ def move_file_back_and_forth(sbox):
 
   # Check expected status before commit
   expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
-  expected_status.add({
-    'A/D/G/rho' : Item(status='R ', copied='+', wc_rev='-',
-                       moved_from='A/D/G/rho', moved_to='A/D/G/rho'),
-    })
   svntest.actions.run_and_verify_status(wc_dir, expected_status)
 
-  # Commit, and check expected output and status
-  expected_output = svntest.wc.State(wc_dir, {
-    'A/D/G/rho' : Item(verb='Replacing'),
-    })
+  # Try to commit and find out that there is nothing to commit.
+  expected_output = []
   expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
-  expected_status.add({
-    'A/D/G/rho' : Item(status='  ', wc_rev=2),
-    })
   svntest.actions.run_and_verify_commit(wc_dir,
                                         expected_output,
                                         expected_status,
@@ -2476,19 +2467,6 @@ def move_dir_back_and_forth(sbox):
 
   # Verify that the status indicates a replace with history
   expected_status = svntest.actions.get_virginal_state(wc_dir, 1)
-  expected_status.add({
-      'A/D'               : Item(status='R ', copied='+', wc_rev='-',
-                                 moved_from='A/D', moved_to='A/D'),
-      'A/D/G'             : Item(status='  ', copied='+', wc_rev='-'),
-      'A/D/G/pi'          : Item(status='  ', copied='+', wc_rev='-'),
-      'A/D/G/rho'         : Item(status='  ', copied='+', wc_rev='-'),
-      'A/D/G/tau'         : Item(status='  ', copied='+', wc_rev='-'),
-      'A/D/gamma'         : Item(status='  ', copied='+', wc_rev='-'),
-      'A/D/H'             : Item(status='  ', copied='+', wc_rev='-'),
-      'A/D/H/chi'         : Item(status='  ', copied='+', wc_rev='-'),
-      'A/D/H/omega'       : Item(status='  ', copied='+', wc_rev='-'),
-      'A/D/H/psi'         : Item(status='  ', copied='+', wc_rev='-'),
-  })
   svntest.actions.run_and_verify_status(wc_dir, expected_status)
 
 def copy_move_added_paths(sbox):

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=1448636&r1=1448635&r2=1448636&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_wc/op-depth-test.c Thu Feb 21 13:29:25 2013
@@ -4073,6 +4073,49 @@ move_to_swap(const svn_test_opts_t *opts
     SVN_ERR(check_db_rows(&b, "", nodes));
   }
 
+  /* And move this last bit back and check if the db state is restored */
+  SVN_ERR(sbox_wc_move(&b, "A/B", "X/B"));
+  SVN_ERR(sbox_wc_move(&b, "X/Y", "A/Y"));
+
+  {
+    nodes_row_t nodes[] = {
+      {0, "",    "normal",       1, ""},
+      {0, "A",   "normal",       1, "A"},
+      {0, "A/B", "normal",       1, "A/B"},
+      {0, "X",   "normal",       1, "X"},
+      {0, "X/Y", "normal",       1, "X/Y"},
+      {1, "A",   "normal",       1, "X",   FALSE, "X", TRUE},
+      {1, "A/Y", "normal",       1, "X/Y", MOVED_HERE},
+      {1, "A/B", "base-deleted", NO_COPY_FROM},
+      {1, "X",   "normal",       1, "A",   FALSE, "A", TRUE},
+      {1, "X/B", "normal",       1, "A/B", MOVED_HERE},
+      {1, "X/Y", "base-deleted", NO_COPY_FROM},
+      {0}
+    };
+    SVN_ERR(check_db_rows(&b, "", nodes));
+  }
+
+#if 0
+  /* And try to undo the rest */
+  SVN_ERR(sbox_wc_move(&b, "A", "A2"));
+  SVN_ERR(sbox_wc_move(&b, "X", "A"));
+  SVN_ERR(sbox_wc_move(&b, "A2", "X"));
+
+  {
+    nodes_row_t nodes[] = {
+      {0, "",    "normal",       1, ""},
+      {0, "A",   "normal",       1, "A"},
+      {0, "A/B", "normal",       1, "A/B"},
+      {0, "X",   "normal",       1, "X"},
+      {0, "X/Y", "normal",       1, "X/Y"},
+      {0}
+    };
+    /* ### Currently this breaks hard. Introducing not-present nodes, etc. 
+       ### but that is not caused by this change */
+    SVN_ERR(check_db_rows(&b, "", nodes));
+  }
+#endif
+
   return SVN_NO_ERROR;
 }
 
@@ -6810,8 +6853,9 @@ struct svn_test_descriptor_t test_funcs[
                        "scan_delete"),
     SVN_TEST_OPTS_PASS(test_follow_moved_to,
                        "follow_moved_to"),
-    SVN_TEST_OPTS_PASS(mixed_rev_move,
-                       "mixed_rev_move"),
+    SVN_TEST_OPTS_WIMP(mixed_rev_move,
+                       "mixed_rev_move",
+                       "needs different libsvn_wc entry point"),
     SVN_TEST_OPTS_PASS(update_prop_mod_into_moved,
                        "update_prop_mod_into_moved"),
     SVN_TEST_OPTS_PASS(nested_move_update,