You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2011/08/17 21:14:09 UTC

svn commit: r1158875 - in /subversion/trunk/subversion: libsvn_wc/adm_ops.c libsvn_wc/wc-queries.sql libsvn_wc/wc_db.c libsvn_wc/wc_db.h tests/cmdline/copy_tests.py tests/cmdline/revert_tests.py

Author: stsp
Date: Wed Aug 17 19:14:09 2011
New Revision: 1158875

URL: http://svn.apache.org/viewvc?rev=1158875&view=rev
Log:
Make 'svn revert' remove any kind of copied file, regardless of whether
the file was modified post-copy.

The rationale for this change can be found in a comment by Julian Foad
on issue #3601: http://subversion.tigris.org/issues/show_bug.cgi?id=3101#desc6
Quote:
  svn revert should undo the local text and prop changes, undo the scheduling,
  and also undo the copying of the file into its new location on disk,
  since that is (like prop changes) an operation that Subversion performed
  on the user's behalf as a "local modification" of the user's working copy.

Most parts of this change are concerned with removing the pristine checksum
comparison revert_restore_handle_copied_file() was doing. A fix for a bug
in revert_list_read() uncovered by a failing test is also included.

* subversion/tests/cmdline/copy_tests.py
  (remove_conflict_file): No need to remove 'pi' after revert.

* subversion/tests/cmdline/revert_tests.py
  (revert_tree_conflicts_in_updated_files): Expect 'A/D/G/rho' to be
   delete from disk after revert. No need to delete it manually.

* subversion/libsvn_wc/adm_ops.c
  (revert_restore_handle_copied_file): Remove. This was wrapping the
   comparison to the pristine file, which is now unnecessary.
  (revert_restore_handle_copied_dirs, revert_restore): Remove all reverted
   copied files from disk. Update some comments.

* subversion/libsvn_wc/wc-queries.sql
  (revert_list, trigger_revert_list_nodes, STMT_SELECT_REVERT_LIST,
   STMT_SELECT_REVERT_LIST_COPIED_CHILDREN): Drop the pristine CHECKSUM.
   It isn't needed anymore.

* subversion/libsvn_wc/wc_db.c
  (revert_list_read_baton): Remove PRISTINE_CHECKSUM.
  (revert_list_read): Remove handling of the pristine checksum.
   Fix a bug: The COPIED_HERE output parameter wasn't set correctly if a
   copied node also had an entry in the ACTUAL_NODES table. E.g. a copied
   node which was also a tree-conflict victim was not recognised as a copy
   by the revert code, and left behind unversioned on disk. Set COPIED_HERE
   correctly in this case, too.
  (svn_wc__db_revert_list_read): Remove PRISTINE_CHECKSUM output parameter.
  (revert_list_read_copied_children): Remove handling of the pristine checksum.
   Small indentation fixes.

* subversion/libsvn_wc/wc_db.h
  (svn_wc__db_revert_list_read): Remove PRISTINE_CHECKSUM output parameter.
  (svn_wc__db_revert_list_copied_child_info_t): Remove PRISTINE_CHECKSUM.

Modified:
    subversion/trunk/subversion/libsvn_wc/adm_ops.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/cmdline/revert_tests.py

Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=1158875&r1=1158874&r2=1158875&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Wed Aug 17 19:14:09 2011
@@ -1292,48 +1292,6 @@ remove_conflict_file(svn_boolean_t *noti
 }
 
 
-/* Remove the reverted copied file at LOCAL_ABSPATH from disk if it was
- * not modified after being copied. Use FILESIZE and PRISTINE_CHECKSUM to
- * detect modification.
- * If NEW_KIND is not NULL, return the resulting on-disk kind of the node
- * at LOCAL_ABSPATH in *NEW_KIND. */
-static svn_error_t *
-revert_restore_handle_copied_file(svn_node_kind_t *new_kind,
-                                  svn_wc__db_t *db,
-                                  const char *local_abspath,
-                                  svn_filesize_t filesize,
-                                  const svn_checksum_t *pristine_checksum,
-                                  apr_pool_t *scratch_pool)
-{
-  svn_stream_t *pristine_stream;
-  svn_filesize_t pristine_size;
-  svn_boolean_t has_text_mods;
-
-  if (new_kind)
-    *new_kind = svn_node_file;
-
-  SVN_ERR(svn_wc__db_pristine_read(&pristine_stream, &pristine_size,
-                                   db, local_abspath, pristine_checksum,
-                                   scratch_pool, scratch_pool));
-  SVN_ERR(svn_wc__compare_file_with_pristine(&has_text_mods, db,
-                                             local_abspath,
-                                             filesize,
-                                             pristine_stream,
-                                             pristine_size,
-                                             FALSE, FALSE, FALSE,
-                                             scratch_pool));
-
-  if (!has_text_mods)
-    {
-      SVN_ERR(svn_io_remove_file2(local_abspath, FALSE, scratch_pool));
-      if (new_kind)
-        *new_kind = svn_node_none;
-    }
-
-  return SVN_NO_ERROR;
-}
-
-
 /* Sort copied children obtained from the revert list based on
  * their paths in descending order (longest paths first). */
 static int
@@ -1351,14 +1309,12 @@ compare_revert_list_copied_children(cons
 }
 
 
-/* Remove as many reverted copied children as possible from the directory at
- * LOCAL_ABSPATH. If REMOVE_SELF is TRUE, also remove LOCAL_ABSPATH itself
- * if possible (REMOVE_SELF should be set if LOCAL_ABSPATH is itself a
- * reverted copy).
+/* Remove all reverted copied children from the directory at LOCAL_ABSPATH.
+ * If REMOVE_SELF is TRUE, also remove LOCAL_ABSPATH itself if possible
+ * (REMOVE_SELF should be set if LOCAL_ABSPATH is itself a reverted copy).
  *
- * All reverted copied file children not modified after being copied are
- * removed from disk. Reverted copied directories left empty as a result
- * are also removed from disk.
+ * All reverted copied file children are removed from disk. Reverted copied
+ * directories left empty as a result are also removed from disk.
  *
  * If NEW_KIND is not NULL, return the resulting on-disk kind of the node
  * at LOCAL_ABSPATH in *NEW_KIND. */
@@ -1374,9 +1330,8 @@ revert_restore_handle_copied_dirs(svn_no
   const apr_array_header_t *copied_children;
   svn_wc__db_revert_list_copied_child_info_t *child_info;
   int i;
-  apr_finfo_t finfo;
+  svn_node_kind_t on_disk;
   apr_pool_t *iterpool;
-  svn_error_t *err;
 
   if (new_kind)
     *new_kind = svn_node_dir;
@@ -1387,7 +1342,7 @@ revert_restore_handle_copied_dirs(svn_no
                                                       scratch_pool));
   iterpool = svn_pool_create(scratch_pool);
 
-  /* Remove all file children, if possible. */
+  /* Remove all copied file children. */
   for (i = 0; i < copied_children->nelts; i++)
     {
       child_info = APR_ARRAY_IDX(
@@ -1402,33 +1357,21 @@ revert_restore_handle_copied_dirs(svn_no
 
       svn_pool_clear(iterpool);
 
-      err = svn_io_stat(&finfo, child_info->abspath,
-                        APR_FINFO_TYPE | APR_FINFO_SIZE,
-                        iterpool);
-      if (err && (APR_STATUS_IS_ENOENT(err->apr_err)
-                  || SVN__APR_STATUS_IS_ENOTDIR(err->apr_err)))
-        {
-          svn_error_clear(err);
-          continue;
-        }
-      else
-        SVN_ERR(err);
-
-      if (finfo.filetype != APR_REG)
+      /* Make sure what we delete from disk is really a file. */
+      SVN_ERR(svn_io_check_path(child_info->abspath, &on_disk, iterpool));
+      if (on_disk != svn_node_file)
         continue;
 
-      SVN_ERR(revert_restore_handle_copied_file(NULL, db, child_info->abspath,
-                                                finfo.size,
-                                                child_info->pristine_checksum,
-                                                iterpool));
+      SVN_ERR(svn_io_remove_file2(child_info->abspath, TRUE, iterpool));
     }
 
   /* Try to delete every child directory, ignoring errors.
    * This is a bit crude but good enough for our purposes.
    *
    * We cannot delete children recursively since we want to keep any files
-   * that still exist on disk. So sort the children list such that longest
-   * paths come first and try to remove each child directory in order. */
+   * that still exist on disk (e.g. unversioned files within the copied tree).
+   * So sort the children list such that longest paths come first and try to
+   * remove each child directory in order. */
   qsort(copied_children->elts, copied_children->nelts,
         sizeof(svn_wc__db_revert_list_copied_child_info_t *),
         compare_revert_list_copied_children);
@@ -1504,7 +1447,6 @@ revert_restore(svn_wc__db_t *db,
 #endif
   svn_boolean_t copied_here;
   svn_wc__db_kind_t reverted_kind;
-  const svn_checksum_t *pristine_checksum;
 
   if (cancel_func)
     SVN_ERR(cancel_func(cancel_baton));
@@ -1513,7 +1455,6 @@ revert_restore(svn_wc__db_t *db,
                                       &conflict_old, &conflict_new,
                                       &conflict_working, &prop_reject,
                                       &copied_here, &reverted_kind,
-                                      &pristine_checksum,
                                       db, local_abspath,
                                       scratch_pool, scratch_pool));
 
@@ -1581,10 +1522,10 @@ revert_restore(svn_wc__db_t *db,
     {
       /* The revert target itself is the op-root of a copy. */
       if (reverted_kind == svn_wc__db_kind_file && on_disk == svn_node_file)
-        SVN_ERR(revert_restore_handle_copied_file(&on_disk, db, local_abspath,
-                                                  finfo.size,
-                                                  pristine_checksum,
-                                                  scratch_pool));
+        {
+          SVN_ERR(svn_io_remove_file2(local_abspath, TRUE, scratch_pool));
+          on_disk = svn_node_none;
+        }
       else if (reverted_kind == svn_wc__db_kind_dir && on_disk == svn_node_dir)
         SVN_ERR(revert_restore_handle_copied_dirs(&on_disk, db, local_abspath,
                                                   TRUE, 

Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=1158875&r1=1158874&r2=1158875&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
+++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Wed Aug 17 19:14:09 2011
@@ -1115,7 +1115,6 @@ CREATE TEMPORARY TABLE revert_list (
    op_depth INTEGER,
    repos_id INTEGER,
    kind TEXT,
-   checksum TEXT,
    PRIMARY KEY (local_relpath, actual)
    );
 DROP TRIGGER IF EXISTS   trigger_revert_list_nodes;
@@ -1123,9 +1122,8 @@ CREATE TEMPORARY TRIGGER trigger_revert_
 BEFORE DELETE ON nodes
 BEGIN
    INSERT OR REPLACE INTO revert_list(local_relpath, actual, op_depth,
-                                      repos_id, kind, checksum)
-   SELECT OLD.local_relpath, 0, OLD.op_depth, OLD.repos_id, OLD.kind,
-          OLD.checksum;
+                                      repos_id, kind)
+   SELECT OLD.local_relpath, 0, OLD.op_depth, OLD.repos_id, OLD.kind;
 END;
 DROP TRIGGER IF EXISTS   trigger_revert_list_actual_delete;
 CREATE TEMPORARY TRIGGER trigger_revert_list_actual_delete
@@ -1163,13 +1161,13 @@ DROP TRIGGER IF EXISTS trigger_revert_li
 
 -- STMT_SELECT_REVERT_LIST
 SELECT conflict_old, conflict_new, conflict_working, prop_reject, notify,
-       actual, op_depth, repos_id, kind, checksum
+       actual, op_depth, repos_id, kind
 FROM revert_list
 WHERE local_relpath = ?1
 ORDER BY actual DESC
 
 -- STMT_SELECT_REVERT_LIST_COPIED_CHILDREN
-SELECT local_relpath, kind, checksum
+SELECT local_relpath, kind
 FROM revert_list
 WHERE local_relpath LIKE ?1 ESCAPE '#'
   AND op_depth >= ?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=1158875&r1=1158874&r2=1158875&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed Aug 17 19:14:09 2011
@@ -5473,7 +5473,6 @@ struct revert_list_read_baton {
   const char **prop_reject;
   svn_boolean_t *copied_here;
   svn_wc__db_kind_t *kind;
-  const svn_checksum_t **pristine_checksum;
   apr_pool_t *result_pool;
 };
 
@@ -5491,7 +5490,6 @@ revert_list_read(void *baton,
   *(b->conflict_new) = *(b->conflict_old) = *(b->conflict_working) = NULL;
   *(b->prop_reject) = NULL;
   *(b->copied_here) = FALSE;
-  *(b->pristine_checksum) = NULL;
   *(b->kind) = svn_wc__db_kind_unknown;
 
   SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
@@ -5500,9 +5498,10 @@ revert_list_read(void *baton,
   SVN_ERR(svn_sqlite__step(&have_row, stmt));
   if (have_row)
     {
-      svn_boolean_t another_row;
+      svn_boolean_t is_actual = (svn_sqlite__column_int64(stmt, 5) != 0);
+      svn_boolean_t another_row = FALSE;
 
-      if (svn_sqlite__column_int64(stmt, 5))
+      if (is_actual)
         {
           if (!svn_sqlite__column_is_null(stmt, 4))
             *(b->reverted) = TRUE;
@@ -5532,18 +5531,9 @@ revert_list_read(void *baton,
                                 b->result_pool);
 
           SVN_ERR(svn_sqlite__step(&another_row, stmt));
-          if (another_row)
-            {
-              *(b->reverted) = TRUE;
-              *(b->kind) = svn_sqlite__column_token(stmt, 8,
-                                                    kind_map);
-              if (!svn_sqlite__column_is_null(stmt, 9))
-                SVN_ERR(svn_sqlite__column_checksum(b->pristine_checksum,
-                                                    stmt, 9,
-                                                    b->result_pool));
-            }
         }
-      else
+
+      if (!is_actual || another_row)
         {
           *(b->reverted) = TRUE;
           if (!svn_sqlite__column_is_null(stmt, 7))
@@ -5552,9 +5542,6 @@ revert_list_read(void *baton,
               *(b->copied_here) = (op_depth == relpath_depth(local_relpath));
             }
           *(b->kind) = svn_sqlite__column_token(stmt, 8, kind_map);
-          if (!svn_sqlite__column_is_null(stmt, 9))
-            SVN_ERR(svn_sqlite__column_checksum(b->pristine_checksum,
-                                                stmt, 9, b->result_pool));
         }
 
     }
@@ -5579,7 +5566,6 @@ svn_wc__db_revert_list_read(svn_boolean_
                             const char **prop_reject,
                             svn_boolean_t *copied_here,
                             svn_wc__db_kind_t *kind,
-                            const svn_checksum_t **pristine_checksum,
                             svn_wc__db_t *db,
                             const char *local_abspath,
                             apr_pool_t *result_pool,
@@ -5589,8 +5575,7 @@ svn_wc__db_revert_list_read(svn_boolean_
   const char *local_relpath;
   struct revert_list_read_baton b = {reverted, conflict_old, conflict_new,
                                      conflict_working, prop_reject,
-                                     copied_here, kind, pristine_checksum,
-                                     result_pool};
+                                     copied_here, kind, result_pool};
 
   SVN_ERR(svn_wc__db_wcroot_parse_local_abspath(&wcroot, &local_relpath,
                               db, local_abspath, scratch_pool, scratch_pool));
@@ -5639,16 +5624,11 @@ revert_list_read_copied_children(void *b
       child_info->abspath = svn_dirent_join(wcroot->abspath, child_relpath,
                                             b->result_pool);
       child_info->kind = svn_sqlite__column_token(stmt, 1, kind_map);
-      if (svn_sqlite__column_is_null(stmt, 2))
-        child_info->pristine_checksum = NULL;
-      else
-        SVN_ERR(svn_sqlite__column_checksum(&child_info->pristine_checksum,
-                                             stmt, 2, b->result_pool));
-       APR_ARRAY_PUSH(
-         children,
-         svn_wc__db_revert_list_copied_child_info_t *) = child_info;
+      APR_ARRAY_PUSH(
+        children,
+        svn_wc__db_revert_list_copied_child_info_t *) = child_info;
 
-       SVN_ERR(svn_sqlite__step(&have_row, stmt));
+      SVN_ERR(svn_sqlite__step(&have_row, stmt));
     }
    SVN_ERR(svn_sqlite__reset(stmt));
 

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=1158875&r1=1158874&r2=1158875&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Wed Aug 17 19:14:09 2011
@@ -1510,9 +1510,6 @@ svn_wc__db_op_revert(svn_wc__db_t *db,
  * Set *COPIED_HERE if the reverted node was copied here and is the
  * operation root of the copy.
  * Set *KIND to the node kind of the reverted node.
- * If the node was a file, set *PRISTINE_CHECKSUM to the checksum
- * of the pristine associated with the node. If the file had no
- * pristine set *PRISTINE_CHECKSUM to NULL.
  *
  * Removes the row for LOCAL_ABSPATH from the revert list.
  */
@@ -1524,7 +1521,6 @@ svn_wc__db_revert_list_read(svn_boolean_
                             const char **prop_reject,
                             svn_boolean_t *copied_here,
                             svn_wc__db_kind_t *kind,
-                            const svn_checksum_t **pristine_checksum,
                             svn_wc__db_t *db,
                             const char *local_abspath,
                             apr_pool_t *result_pool,
@@ -1535,11 +1531,10 @@ svn_wc__db_revert_list_read(svn_boolean_
 typedef struct svn_wc__db_revert_list_copied_child_info_t {
   const char *abspath;
   svn_wc__db_kind_t kind;
-  const svn_checksum_t *pristine_checksum;
 } svn_wc__db_revert_list_copied_child_info_t ;
 
-/* Return in *CHILDREN a list of reverted copied children within the
- * reverted tree rooted at LOCAL_ABSPATH.
+/* Return in *CHILDREN a list of reverted copied nodes at or within
+ * LOCAL_ABSPATH (which is a reverted file or a reverted directory).
  * Allocate *COPIED_CHILDREN and its elements in RESULT_POOL.
  * The elements are of type svn_wc__db_revert_list_copied_child_info_t. */
 svn_error_t *

Modified: subversion/trunk/subversion/tests/cmdline/copy_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/copy_tests.py?rev=1158875&r1=1158874&r2=1158875&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/copy_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/copy_tests.py Wed Aug 17 19:14:09 2011
@@ -955,7 +955,6 @@ def repos_to_wc(sbox):
   svntest.actions.run_and_verify_svn(None, None, [], 'revert', '-R', wc_dir)
 
   svntest.main.safe_rmtree(os.path.join(wc_dir, 'E'))
-  os.unlink(os.path.join(wc_dir, 'pi'))
 
   expected_output = svntest.actions.get_virginal_state(wc_dir, 1)
   svntest.actions.run_and_verify_status(wc_dir, expected_output)

Modified: subversion/trunk/subversion/tests/cmdline/revert_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/revert_tests.py?rev=1158875&r1=1158874&r2=1158875&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/revert_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/revert_tests.py Wed Aug 17 19:14:09 2011
@@ -971,8 +971,7 @@ def revert_tree_conflicts_in_updated_fil
   expected_status.remove('A/D/G/tau')
 
   expected_disk = svntest.main.greek_state.copy()
-  expected_disk.tweak('A/D/G/rho',
-                      contents="This is the file 'rho'.\nLocal edit.\n")
+  expected_disk.remove('A/D/G/rho')
   expected_disk.tweak('A/D/G/pi',
                       contents="This is the file 'pi'.\nIncoming edit.\n")
   expected_disk.remove('A/D/G/tau')