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 12:33:12 UTC

svn commit: r1158617 - in /subversion/trunk/subversion: libsvn_wc/ tests/cmdline/

Author: stsp
Date: Wed Aug 17 10:33:12 2011
New Revision: 1158617

URL: http://svn.apache.org/viewvc?rev=1158617&view=rev
Log:
Remove reverted copied files and directories from disk, if they weren't
modified after being copied.

To achieve this we add more columns to rows in the revert list:
the node kind, the repos_id (which is not NULL for copies),
the op_root, and, if the node is a file, the pristine checksum.

While restoring the BASE on-disk state, process nodes in the revert list
marked as copies in a special way: Compare reverted copied files with
their pristines, and remove the ones that match from disk. Next, remove
any reverted copied directories which are left empty as a result.

This commit addresses issues #3101 and #876, and my own annoyance at
unversioned things left behind in my testing WCs (been testing 'svn move'
with subsequent 'revert' a lot lately).

We could take this further and remove copied files which were modified
post-copy (after all, we also destroy textual modifications to files which
were not copied). But that's a decision for another day.

Review by: julianfoad

* subversion/tests/cmdline/merge_tests.py
  (merge_away_subtrees_noninheritable_ranges): No longer need to remove
   unversioned files after revert.

* subversion/tests/cmdline/depth_tests.py
  (excluded_path_misc_operation): No longer need to remove unversioned files
   after revert.

* subversion/tests/cmdline/revert_tests.py
  (status_of_missing_dir_after_revert_replaced_with_history_dir): No longer 
   need to remove unversioned files. And don't expect them in status output.

* subversion/libsvn_wc/adm_ops.c
  (revert_restore_handle_copied_file, revert_restore_handle_copied_dirs,
   compare_revert_list_copied_children): New.
  (revert_restore): When reverting copies, remove their remains from disk,
   except for files which were modified after being copied, and directories
   that contain unversioned files.

* subversion/libsvn_wc/wc-queries.sql
  (revert_list): Add new columns OP_DEPTH, REPOS_ID, KIND, CHECKSUM.
   Copy values from rows deleted from the NODES table into them.
   Nothing changes for actual-only nodes.
  (STMT_SELECT_REVERT_LIST): Get the new columns.
  (STMT_SELECT_REVERT_LIST_COPIED_CHILDREN): New statement which returns
   all copied children within a given reverted local_relpath.

* subversion/libsvn_wc/wc.h
  (svn_wc__compare_file_with_pristine): Declare.

* subversion/libsvn_wc/questions.c
  (compare_and_verify): Renamed to ...
  (svn_wc__compare_file_with_pristine): ... this, so the revert code can
   access this function. No modifications were made to the implementation.
  (svn_wc__internal_file_modified_p): Track above function rename.

* subversion/libsvn_wc/wc_db.c
  (revert_list_read_baton): Add COPIED_HERE, KIND, and PRISTINE_CHECKSUM.
  (revert_list_read): Populate the new baton fields.
  (svn_wc__db_revert_list_read): Add COPIED_HERE, KIND, and PRISTINE_CHECKSUM
   output parameters.
  (revert_list_read_copied_children_baton): New.
  (revert_list_read_copied_children): New. Returns a list of all copied
   children which existed within a reverted subtree.
  (svn_wc__db_revert_list_read_copied_children): Exposes the 
   revert_list_read_copied_children() function, wrapping it in an sqlite
   transaction.

* subversion/libsvn_wc/wc_db.h
  (svn_wc__db_revert_list_read): Declare new output parameters COPIED_HERE,
   KIND, and PRISTINE_CHECKSUM. Update docstring.
  (svn_wc__db_revert_list_read_copied_children): Declare.

Modified:
    subversion/trunk/subversion/libsvn_wc/adm_ops.c
    subversion/trunk/subversion/libsvn_wc/questions.c
    subversion/trunk/subversion/libsvn_wc/wc-queries.sql
    subversion/trunk/subversion/libsvn_wc/wc.h
    subversion/trunk/subversion/libsvn_wc/wc_db.c
    subversion/trunk/subversion/libsvn_wc/wc_db.h
    subversion/trunk/subversion/tests/cmdline/depth_tests.py
    subversion/trunk/subversion/tests/cmdline/merge_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=1158617&r1=1158616&r2=1158617&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Wed Aug 17 10:33:12 2011
@@ -29,6 +29,7 @@
 
 
 #include <string.h>
+#include <stdlib.h>
 
 #include <apr_pools.h>
 #include <apr_tables.h>
@@ -1291,6 +1292,186 @@ 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
+compare_revert_list_copied_children(const void *a, const void *b)
+{
+  const svn_wc__db_revert_list_copied_child_info_t *ca = a;
+  const svn_wc__db_revert_list_copied_child_info_t *cb = b;
+  int i;
+
+  i = svn_path_compare_paths(ca->abspath, cb->abspath);
+
+  /* Reverse the result of svn_path_compare_paths() to achieve
+   * descending order. */
+  return -i;
+}
+
+
+/* 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).
+ *
+ * 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.
+ *
+ * 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_dirs(svn_node_kind_t *new_kind,
+                                  svn_wc__db_t *db,
+                                  const char *local_abspath,
+                                  svn_boolean_t remove_self,
+                                  svn_cancel_func_t cancel_func,
+                                  void *cancel_baton,
+                                  apr_pool_t *scratch_pool)
+{
+  const apr_array_header_t *copied_children;
+  svn_wc__db_revert_list_copied_child_info_t *child_info;
+  int i;
+  apr_finfo_t finfo;
+  apr_pool_t *iterpool;
+  svn_error_t *err;
+
+  if (new_kind)
+    *new_kind = svn_node_dir;
+
+  SVN_ERR(svn_wc__db_revert_list_read_copied_children(&copied_children,
+                                                      db, local_abspath,
+                                                      scratch_pool,
+                                                      scratch_pool));
+  iterpool = svn_pool_create(scratch_pool);
+
+  /* Remove all file children, if possible. */
+  for (i = 0; i < copied_children->nelts; i++)
+    {
+      child_info = APR_ARRAY_IDX(
+                     copied_children, i,
+                     svn_wc__db_revert_list_copied_child_info_t *);
+
+      if (cancel_func)
+        SVN_ERR(cancel_func(cancel_baton));
+
+      if (child_info->kind != svn_wc__db_kind_file)
+        continue;
+
+      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)
+        continue;
+
+      SVN_ERR(revert_restore_handle_copied_file(NULL, db, child_info->abspath,
+                                                finfo.size,
+                                                child_info->pristine_checksum,
+                                                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. */
+  qsort(copied_children->elts, copied_children->nelts,
+        sizeof(svn_wc__db_revert_list_copied_child_info_t *),
+        compare_revert_list_copied_children);
+  for (i = 0; i < copied_children->nelts; i++)
+    {
+      child_info = APR_ARRAY_IDX(
+                     copied_children, i,
+                     svn_wc__db_revert_list_copied_child_info_t *);
+
+      if (cancel_func)
+        SVN_ERR(cancel_func(cancel_baton));
+
+      if (child_info->kind != svn_wc__db_kind_dir)
+        continue;
+
+      svn_pool_clear(iterpool);
+
+      svn_error_clear(svn_io_dir_remove_nonrecursive(child_info->abspath,
+                                                     iterpool));
+    }
+
+  if (remove_self)
+    {
+      apr_hash_t *remaining_children;
+
+      /* Delete LOCAL_ABSPATH itself if no children are left. */
+      SVN_ERR(svn_io_get_dirents3(&remaining_children, local_abspath, TRUE,
+                                  iterpool, iterpool));
+      if (apr_hash_count(remaining_children) == 0)
+        {
+          SVN_ERR(svn_io_remove_dir2(local_abspath, FALSE, NULL, NULL,
+                                     iterpool));
+          if (new_kind)
+            *new_kind = svn_node_none;
+        }
+    }
+
+  svn_pool_destroy(iterpool);
+
+  return SVN_NO_ERROR;
+}
+
+
 /* Make the working tree under LOCAL_ABSPATH to depth DEPTH match the
    versioned tree.  This function is called after svn_wc__db_op_revert
    has done the database revert and created the revert list.  Notifies
@@ -1321,6 +1502,9 @@ revert_restore(svn_wc__db_t *db,
 #ifdef HAVE_SYMLINK
   svn_boolean_t special;
 #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));
@@ -1328,6 +1512,8 @@ revert_restore(svn_wc__db_t *db,
   SVN_ERR(svn_wc__db_revert_list_read(&notify_required,
                                       &conflict_old, &conflict_new,
                                       &conflict_working, &prop_reject,
+                                      &copied_here, &reverted_kind,
+                                      &pristine_checksum,
                                       db, local_abspath,
                                       scratch_pool, scratch_pool));
 
@@ -1342,17 +1528,21 @@ revert_restore(svn_wc__db_t *db,
     {
       svn_error_clear(err);
 
-      if (notify_func && notify_required)
-        notify_func(notify_baton,
-                    svn_wc_create_notify(local_abspath, svn_wc_notify_revert,
-                                         scratch_pool),
-                    scratch_pool);
-
-      if (notify_func)
-        SVN_ERR(svn_wc__db_revert_list_notify(notify_func, notify_baton,
-                                              db, local_abspath,
-                                              scratch_pool));
-      return SVN_NO_ERROR;
+      if (!copied_here)
+        {
+          if (notify_func && notify_required)
+            notify_func(notify_baton,
+                        svn_wc_create_notify(local_abspath,
+                                             svn_wc_notify_revert,
+                                             scratch_pool),
+                        scratch_pool);
+
+          if (notify_func)
+            SVN_ERR(svn_wc__db_revert_list_notify(notify_func, notify_baton,
+                                                  db, local_abspath,
+                                                  scratch_pool));
+          return SVN_NO_ERROR;
+        }
     }
   else if (err)
     return svn_error_trace(err);
@@ -1387,6 +1577,21 @@ revert_restore(svn_wc__db_t *db,
 #endif
     }
 
+  if (copied_here)
+    {
+      /* 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));
+      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, 
+                                                  cancel_func, cancel_baton,
+                                                  scratch_pool));
+    }
+
   /* If we expect a versioned item to be present then check that any
      item on disk matches the versioned item, if it doesn't match then
      fix it or delete it.  */
@@ -1567,6 +1772,10 @@ revert_restore(svn_wc__db_t *db,
       const apr_array_header_t *children;
       int i;
 
+      SVN_ERR(revert_restore_handle_copied_dirs(NULL, db, local_abspath, FALSE,
+                                                cancel_func, cancel_baton,
+                                                iterpool));
+
       SVN_ERR(svn_wc__db_read_children_of_working_node(&children, db,
                                                        local_abspath,
                                                        scratch_pool,

Modified: subversion/trunk/subversion/libsvn_wc/questions.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/questions.c?rev=1158617&r1=1158616&r2=1158617&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/questions.c (original)
+++ subversion/trunk/subversion/libsvn_wc/questions.c Wed Aug 17 10:33:12 2011
@@ -79,38 +79,17 @@
 */
 
 
-/* Set *MODIFIED_P to TRUE if (after translation) VERSIONED_FILE_ABSPATH
- * (of VERSIONED_FILE_SIZE bytes) differs from PRISTINE_STREAM (of
- * PRISTINE_SIZE bytes), else to FALSE if not.
- *
- * If EXACT_COMPARISON is FALSE, translate VERSIONED_FILE_ABSPATH's EOL
- * style and keywords to repository-normal form according to its properties,
- * and compare the result with PRISTINE_STREAM.  If EXACT_COMPARISON is
- * TRUE, translate PRISTINE_STREAM's EOL style and keywords to working-copy
- * form according to VERSIONED_FILE_ABSPATH's properties, and compare the
- * result with VERSIONED_FILE_ABSPATH.
- *
- * HAS_PROPS should be TRUE if the file had properties when it was not
- * modified, otherwise FALSE.
- *
- * PROPS_MOD should be TRUE if the file's properties have been changed,
- * otherwise FALSE.
- *
- * PRISTINE_STREAM will be closed before a successful return.
- *
- * DB is a wc_db; use SCRATCH_POOL for temporary allocation.
- */
-static svn_error_t *
-compare_and_verify(svn_boolean_t *modified_p,
-                   svn_wc__db_t *db,
-                   const char *versioned_file_abspath,
-                   svn_filesize_t versioned_file_size,
-                   svn_stream_t *pristine_stream,
-                   svn_filesize_t pristine_size,
-                   svn_boolean_t has_props,
-                   svn_boolean_t props_mod,
-                   svn_boolean_t exact_comparison,
-                   apr_pool_t *scratch_pool)
+svn_error_t *
+svn_wc__compare_file_with_pristine(svn_boolean_t *modified_p,
+                                   svn_wc__db_t *db,
+                                   const char *versioned_file_abspath,
+                                   svn_filesize_t versioned_file_size,
+                                   svn_stream_t *pristine_stream,
+                                   svn_filesize_t pristine_size,
+                                   svn_boolean_t has_props,
+                                   svn_boolean_t props_mod,
+                                   svn_boolean_t exact_comparison,
+                                   apr_pool_t *scratch_pool)
 {
   svn_boolean_t same;
   svn_subst_eol_style_t eol_style;
@@ -337,12 +316,12 @@ svn_wc__internal_file_modified_p(svn_boo
   /* Check all bytes, and verify checksum if requested. */
   {
     svn_error_t *err;
-    err = compare_and_verify(modified_p, db,
-                             local_abspath, dirent->filesize,
-                             pristine_stream, pristine_size,
-                             has_props, props_mod,
-                             exact_comparison,
-                             scratch_pool);
+    err = svn_wc__compare_file_with_pristine(modified_p, db,
+                                             local_abspath, dirent->filesize,
+                                             pristine_stream, pristine_size,
+                                             has_props, props_mod,
+                                             exact_comparison,
+                                             scratch_pool);
 
     /* At this point we already opened the pristine file, so we know that
        the access denied applies to the working copy path */

Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=1158617&r1=1158616&r2=1158617&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
+++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Wed Aug 17 10:33:12 2011
@@ -1112,14 +1112,20 @@ CREATE TEMPORARY TABLE revert_list (
    conflict_working TEXT,
    prop_reject TEXT,
    notify INTEGER,         /* 1 if an actual row had props or tree conflict */
+   op_depth INTEGER,
+   repos_id INTEGER,
+   kind TEXT,
+   checksum TEXT,
    PRIMARY KEY (local_relpath, actual)
    );
 DROP TRIGGER IF EXISTS   trigger_revert_list_nodes;
 CREATE TEMPORARY TRIGGER trigger_revert_list_nodes
 BEFORE DELETE ON nodes
 BEGIN
-   INSERT OR REPLACE INTO revert_list(local_relpath, actual)
-   SELECT OLD.local_relpath, 0;
+   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;
 END;
 DROP TRIGGER IF EXISTS   trigger_revert_list_actual_delete;
 CREATE TEMPORARY TRIGGER trigger_revert_list_actual_delete
@@ -1156,11 +1162,20 @@ DROP TRIGGER IF EXISTS trigger_revert_li
 DROP TRIGGER IF EXISTS trigger_revert_list_actual_update
 
 -- STMT_SELECT_REVERT_LIST
-SELECT conflict_old, conflict_new, conflict_working, prop_reject, notify, actual
+SELECT conflict_old, conflict_new, conflict_working, prop_reject, notify,
+       actual, op_depth, repos_id, kind, checksum
 FROM revert_list
 WHERE local_relpath = ?1
 ORDER BY actual DESC
 
+-- STMT_SELECT_REVERT_LIST_COPIED_CHILDREN
+SELECT local_relpath, kind, checksum
+FROM revert_list
+WHERE local_relpath LIKE ?1 ESCAPE '#'
+  AND op_depth >= ?2
+  AND repos_id IS NOT NULL
+ORDER BY local_relpath
+
 -- STMT_DELETE_REVERT_LIST
 DELETE FROM revert_list WHERE local_relpath = ?1
 

Modified: subversion/trunk/subversion/libsvn_wc/wc.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc.h?rev=1158617&r1=1158616&r2=1158617&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc.h Wed Aug 17 10:33:12 2011
@@ -725,6 +725,39 @@ svn_wc__perform_file_merge(svn_skel_t **
                            apr_pool_t *result_pool,
                            apr_pool_t *scratch_pool);
 
+/* Set *MODIFIED_P to TRUE if (after translation) VERSIONED_FILE_ABSPATH
+ * (of VERSIONED_FILE_SIZE bytes) differs from PRISTINE_STREAM (of
+ * PRISTINE_SIZE bytes), else to FALSE if not.
+ *
+ * If EXACT_COMPARISON is FALSE, translate VERSIONED_FILE_ABSPATH's EOL
+ * style and keywords to repository-normal form according to its properties,
+ * and compare the result with PRISTINE_STREAM.  If EXACT_COMPARISON is
+ * TRUE, translate PRISTINE_STREAM's EOL style and keywords to working-copy
+ * form according to VERSIONED_FILE_ABSPATH's properties, and compare the
+ * result with VERSIONED_FILE_ABSPATH.
+ *
+ * HAS_PROPS should be TRUE if the file had properties when it was not
+ * modified, otherwise FALSE.
+ *
+ * PROPS_MOD should be TRUE if the file's properties have been changed,
+ * otherwise FALSE.
+ *
+ * PRISTINE_STREAM will be closed before a successful return.
+ *
+ * DB is a wc_db; use SCRATCH_POOL for temporary allocation.
+ */
+svn_error_t *
+svn_wc__compare_file_with_pristine(svn_boolean_t *modified_p,
+                                   svn_wc__db_t *db,
+                                   const char *versioned_file_abspath,
+                                   svn_filesize_t versioned_file_size,
+                                   svn_stream_t *pristine_stream,
+                                   svn_filesize_t pristine_size,
+                                   svn_boolean_t has_props,
+                                   svn_boolean_t props_mod,
+                                   svn_boolean_t exact_comparison,
+                                   apr_pool_t *scratch_pool);
+
 
 #ifdef __cplusplus
 }

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1158617&r1=1158616&r2=1158617&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed Aug 17 10:33:12 2011
@@ -5471,6 +5471,9 @@ struct revert_list_read_baton {
   const char **conflict_new;
   const char **conflict_working;
   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;
 };
 
@@ -5487,6 +5490,9 @@ revert_list_read(void *baton,
   *(b->reverted) = FALSE;
   *(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,
                                     STMT_SELECT_REVERT_LIST));
@@ -5527,10 +5533,30 @@ revert_list_read(void *baton,
 
           SVN_ERR(svn_sqlite__step(&another_row, stmt));
           if (another_row)
-            *(b->reverted) = TRUE;
+            {
+              *(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
-        *(b->reverted) = TRUE;
+        {
+          *(b->reverted) = TRUE;
+          if (!svn_sqlite__column_is_null(stmt, 7))
+            {
+              apr_int64_t op_depth = svn_sqlite__column_int64(stmt, 6);
+              *(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));
+        }
+
     }
   SVN_ERR(svn_sqlite__reset(stmt));
 
@@ -5551,6 +5577,9 @@ svn_wc__db_revert_list_read(svn_boolean_
                             const char **conflict_new,
                             const char **conflict_working,
                             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,
@@ -5560,6 +5589,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};
 
   SVN_ERR(svn_wc__db_wcroot_parse_local_abspath(&wcroot, &local_relpath,
@@ -5571,6 +5601,85 @@ svn_wc__db_revert_list_read(svn_boolean_
   return SVN_NO_ERROR;
 }
 
+
+struct revert_list_read_copied_children_baton {
+  const apr_array_header_t **children;
+  apr_pool_t *result_pool;
+};
+
+static svn_error_t *
+revert_list_read_copied_children(void *baton,
+                                 svn_wc__db_wcroot_t *wcroot,
+                                 const char *local_relpath,
+                                 apr_pool_t *scratch_pool)
+{
+  struct revert_list_read_copied_children_baton *b = baton;
+  svn_sqlite__stmt_t *stmt;
+  svn_boolean_t have_row;
+  apr_array_header_t *children;
+
+  children =
+    apr_array_make(b->result_pool, 0,
+                  sizeof(svn_wc__db_revert_list_copied_child_info_t *));
+
+  SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb,
+                                    STMT_SELECT_REVERT_LIST_COPIED_CHILDREN));
+  SVN_ERR(svn_sqlite__bindf(stmt, "si",
+                            construct_like_arg(local_relpath, scratch_pool),
+                            relpath_depth(local_relpath)));
+  SVN_ERR(svn_sqlite__step(&have_row, stmt));
+  while (have_row)
+    {
+      svn_wc__db_revert_list_copied_child_info_t *child_info;
+      const char *child_relpath;
+
+      child_info = apr_palloc(b->result_pool, sizeof(*child_info));
+
+      child_relpath = svn_sqlite__column_text(stmt, 0, NULL);
+      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;
+
+       SVN_ERR(svn_sqlite__step(&have_row, stmt));
+    }
+   SVN_ERR(svn_sqlite__reset(stmt));
+
+  *b->children = children;
+
+  return SVN_NO_ERROR;
+}
+
+
+svn_error_t *
+svn_wc__db_revert_list_read_copied_children(const apr_array_header_t **children,
+                                            svn_wc__db_t *db,
+                                            const char *local_abspath,
+                                            apr_pool_t *result_pool,
+                                            apr_pool_t *scratch_pool)
+{
+  svn_wc__db_wcroot_t *wcroot;
+  const char *local_relpath;
+  struct revert_list_read_copied_children_baton b = {children, result_pool};
+
+  SVN_ERR(svn_wc__db_wcroot_parse_local_abspath(&wcroot, &local_relpath,
+                              db, local_abspath, scratch_pool, scratch_pool));
+  VERIFY_USABLE_WCROOT(wcroot);
+
+  SVN_ERR(svn_wc__db_with_txn(wcroot, local_relpath,
+                              revert_list_read_copied_children, &b,
+                              scratch_pool));
+  return SVN_NO_ERROR;
+}
+
+
 svn_error_t *
 svn_wc__db_revert_list_notify(svn_wc_notify_func2_t notify_func,
                               void *notify_baton,

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=1158617&r1=1158616&r2=1158617&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Wed Aug 17 10:33:12 2011
@@ -1506,6 +1506,13 @@ svn_wc__db_op_revert(svn_wc__db_t *db,
  * path was reverted.  Set *CONFLICT_OLD, *CONFLICT_NEW,
  * *CONFLICT_WORKING and *PROP_REJECT to the names of the conflict
  * files, or NULL if the names are not stored.
+ * 
+ * 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.
  */
@@ -1515,11 +1522,34 @@ svn_wc__db_revert_list_read(svn_boolean_
                             const char **conflict_new,
                             const char **conflict_working,
                             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,
                             apr_pool_t *scratch_pool);
 
+/* The type of elements in the array returned by
+ * svn_wc__db_revert_list_read_copied_children(). */
+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.
+ * 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 *
+svn_wc__db_revert_list_read_copied_children(const apr_array_header_t **children,
+                                            svn_wc__db_t *db,
+                                            const char *local_abspath,
+                                            apr_pool_t *result_pool,
+                                            apr_pool_t *scratch_pool);
+
+
 /* Make revert notifications for all paths in the revert list that are
  * equal to LOCAL_ABSPATH or below LOCAL_ABSPATH.
  *

Modified: subversion/trunk/subversion/tests/cmdline/depth_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/depth_tests.py?rev=1158617&r1=1158616&r2=1158617&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/depth_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/depth_tests.py Wed Aug 17 10:33:12 2011
@@ -2229,10 +2229,6 @@ def excluded_path_misc_operation(sbox):
   svntest.actions.run_and_verify_svn(None, expected_output, [],
                                      'revert', '--depth=infinity', L_path)
 
-  # Get rid of A/L.
-  svntest.actions.run_and_verify_svn(None, None, [],
-                                     'rm', '--force', L_path)
-
   # copy A/B to A/L and then cp A/L to A/M, excluded entry should be
   # copied both times
   expected_output = ['A         '+L_path+'\n']

Modified: subversion/trunk/subversion/tests/cmdline/merge_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tests.py?rev=1158617&r1=1158616&r2=1158617&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/merge_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/merge_tests.py Wed Aug 17 10:33:12 2011
@@ -7482,7 +7482,6 @@ def merge_away_subtrees_noninheritable_r
   #
   # First revert all local changes and remove A_COPY/C/nu from disk.
   svntest.actions.run_and_verify_svn(None, None, [], 'revert', '-R', wc_dir)
-  os.remove(os.path.join(wc_dir, "A_COPY", "nu"))
 
   # Make a text change to A_COPY_2/mu in r11 and then merge that
   # change to A/mu in r12.  This will create mergeinfo of '/A_COPY_2/mu:11'

Modified: subversion/trunk/subversion/tests/cmdline/revert_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/revert_tests.py?rev=1158617&r1=1158616&r2=1158617&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/revert_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/revert_tests.py Wed Aug 17 10:33:12 2011
@@ -890,13 +890,7 @@ def status_of_missing_dir_after_revert_r
   svntest.actions.run_and_verify_svn(None, expected_output, [], "revert", "-R",
                                      G_path)
 
-
-  # Revert leaves these added nodes as unversioned
-  expected_output = svntest.verify.UnorderedOutput(
-    ["?       " + os.path.join(G_path, "pi") + "\n",
-     "?       " + os.path.join(G_path, "rho") + "\n",
-     "?       " + os.path.join(G_path, "tau") + "\n"])
-  svntest.actions.run_and_verify_svn(None, expected_output, [],
+  svntest.actions.run_and_verify_svn(None, [], [],
                                      "status", wc_dir)
 
   svntest.main.safe_rmtree(G_path)
@@ -1464,8 +1458,6 @@ def revert_tree_conflicts_with_replaceme
 
   # Remove a few unversioned files that revert left behind.
   os.remove(wc('A/B/E/loc_beta'))
-  os.remove(wc('A/D/G/rho'))
-  os.remove(wc('A/D/G/tau'))
   os.remove(wc('A/D/H/loc_psi'))
 
   # The update operation should have put all incoming items in place.



Re: svn commit: r1158617 - in /subversion/trunk/subversion: libsvn_wc/ tests/cmdline/

Posted by Stefan Sperling <st...@elego.de>.
On Wed, Aug 17, 2011 at 01:03:09PM +0200, Bert Huijben wrote:
> > +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;
> > +}
> 
> Why do you reimplement svn_wc__internal_file_modified_p() here without the timestamp optimization?
> Is the recorded timestamp + size already lost?
> Or can we move this check a bit to allow using the existing functions?

The file has already been nuked from the DB at this point.
It is unversioned. The existing svn_wc__internal_file_modified_p()
implementation doesn't like that.

I see your point about the timestamp check. However, Julian's comment at
http://subversion.tigris.org/issues/show_bug.cgi?id=3101#desc6
got me convinced that we should just nuke the copied files, modified or not.
Revert is _supposed_ to destroy local mods, and tip-toeing around this
just because the file was copied is silly.
So this entire function will be replaced by svn_io_remove_file2().
 
> svn revert is commonly used to resolve missing file problems. I assume this code will never be used in this case?

No, it's only run on unversioned files which were copied before being
reverted.

> > +      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)
> > +        continue;
> 
> Using svn_io_stat_dirent would make this code much easier (and you could probably forward the dirent)

Thanks, I'll look into that.

> > +  /* 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. */
> > +  qsort(copied_children->elts, copied_children->nelts,
> > +        sizeof(svn_wc__db_revert_list_copied_child_info_t *),
> > +        compare_revert_list_copied_children);
> > +  for (i = 0; i < copied_children->nelts; i++)
> > +    {
> > +      child_info = APR_ARRAY_IDX(
> > +                     copied_children, i,
> > +                     svn_wc__db_revert_list_copied_child_info_t *);
> > +
> > +      if (cancel_func)
> > +        SVN_ERR(cancel_func(cancel_baton));
> > +
> > +      if (child_info->kind != svn_wc__db_kind_dir)
> > +        continue;
> > +
> > +      svn_pool_clear(iterpool);
> > +
> > +      svn_error_clear(svn_io_dir_remove_nonrecursive(child_info->abspath,
> > +                                                     iterpool));
> 
> Please handle explicit errors instead of ignoring all errors.
> 
> It is not nice that your operating system warns you that your filesystem is broken while the client just goes on as if nothing happened.
> (Common problem in Subversion 1.6 on Windows 7 pre servicepack 1).

OK, will do.

> > +  if (remove_self)
> > +    {
> > +      apr_hash_t *remaining_children;
> > +
> > +      /* Delete LOCAL_ABSPATH itself if no children are left. */
> > +      SVN_ERR(svn_io_get_dirents3(&remaining_children, local_abspath,
> > TRUE,
> > +                                  iterpool, iterpool));
> > +      if (apr_hash_count(remaining_children) == 0)
> > +        {
> > +          SVN_ERR(svn_io_remove_dir2(local_abspath, FALSE, NULL, NULL,
> > +                                     iterpool));
> > +          if (new_kind)
> > +            *new_kind = svn_node_none;
> > +        }
> 
> This is an inefficient reimplementation of svn_io_dir_empty() and a call to svn_io_dir_remove_nonrecursive() would handle this check for you.

Right, a simple call to svn_io_dir_remove_nonrecursive() will do. Thanks!

RE: svn commit: r1158617 - in /subversion/trunk/subversion: libsvn_wc/ tests/cmdline/

Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: stsp@apache.org [mailto:stsp@apache.org]
> Sent: woensdag 17 augustus 2011 12:33
> To: commits@subversion.apache.org
> Subject: svn commit: r1158617 - in /subversion/trunk/subversion: libsvn_wc/
> tests/cmdline/
> 
> Author: stsp
> Date: Wed Aug 17 10:33:12 2011
> New Revision: 1158617
> 
> URL: http://svn.apache.org/viewvc?rev=1158617&view=rev
> Log:
> Remove reverted copied files and directories from disk, if they weren't
> modified after being copied.
> 
> To achieve this we add more columns to rows in the revert list:
> the node kind, the repos_id (which is not NULL for copies),
> the op_root, and, if the node is a file, the pristine checksum.
> 
> While restoring the BASE on-disk state, process nodes in the revert list
> marked as copies in a special way: Compare reverted copied files with
> their pristines, and remove the ones that match from disk. Next, remove
> any reverted copied directories which are left empty as a result.
> 
> This commit addresses issues #3101 and #876, and my own annoyance at
> unversioned things left behind in my testing WCs (been testing 'svn move'
> with subsequent 'revert' a lot lately).
> 
> We could take this further and remove copied files which were modified
> post-copy (after all, we also destroy textual modifications to files which
> were not copied). But that's a decision for another day.
> 
> Review by: julianfoad
>
> * subversion/tests/cmdline/merge_tests.py
>   (merge_away_subtrees_noninheritable_ranges): No longer need to
> remove
>    unversioned files after revert.
> 
> * subversion/tests/cmdline/depth_tests.py
>   (excluded_path_misc_operation): No longer need to remove unversioned
> files
>    after revert.
> 
> * subversion/tests/cmdline/revert_tests.py
>   (status_of_missing_dir_after_revert_replaced_with_history_dir): No
> longer
>    need to remove unversioned files. And don't expect them in status output.
> 
> * subversion/libsvn_wc/adm_ops.c
>   (revert_restore_handle_copied_file, revert_restore_handle_copied_dirs,
>    compare_revert_list_copied_children): New.
>   (revert_restore): When reverting copies, remove their remains from disk,
>    except for files which were modified after being copied, and directories
>    that contain unversioned files.
> 
> * subversion/libsvn_wc/wc-queries.sql
>   (revert_list): Add new columns OP_DEPTH, REPOS_ID, KIND, CHECKSUM.
>    Copy values from rows deleted from the NODES table into them.
>    Nothing changes for actual-only nodes.
>   (STMT_SELECT_REVERT_LIST): Get the new columns.
>   (STMT_SELECT_REVERT_LIST_COPIED_CHILDREN): New statement which
> returns
>    all copied children within a given reverted local_relpath.
> 
> * subversion/libsvn_wc/wc.h
>   (svn_wc__compare_file_with_pristine): Declare.
> 
> * subversion/libsvn_wc/questions.c
>   (compare_and_verify): Renamed to ...
>   (svn_wc__compare_file_with_pristine): ... this, so the revert code can
>    access this function. No modifications were made to the implementation.
>   (svn_wc__internal_file_modified_p): Track above function rename.
> 
> * subversion/libsvn_wc/wc_db.c
>   (revert_list_read_baton): Add COPIED_HERE, KIND, and
> PRISTINE_CHECKSUM.
>   (revert_list_read): Populate the new baton fields.
>   (svn_wc__db_revert_list_read): Add COPIED_HERE, KIND, and
> PRISTINE_CHECKSUM
>    output parameters.
>   (revert_list_read_copied_children_baton): New.
>   (revert_list_read_copied_children): New. Returns a list of all copied
>    children which existed within a reverted subtree.
>   (svn_wc__db_revert_list_read_copied_children): Exposes the
>    revert_list_read_copied_children() function, wrapping it in an sqlite
>    transaction.
> 
> * subversion/libsvn_wc/wc_db.h
>   (svn_wc__db_revert_list_read): Declare new output parameters
> COPIED_HERE,
>    KIND, and PRISTINE_CHECKSUM. Update docstring.
>   (svn_wc__db_revert_list_read_copied_children): Declare.
> 
> Modified:
>     subversion/trunk/subversion/libsvn_wc/adm_ops.c
>     subversion/trunk/subversion/libsvn_wc/questions.c
>     subversion/trunk/subversion/libsvn_wc/wc-queries.sql
>     subversion/trunk/subversion/libsvn_wc/wc.h
>     subversion/trunk/subversion/libsvn_wc/wc_db.c
>     subversion/trunk/subversion/libsvn_wc/wc_db.h
>     subversion/trunk/subversion/tests/cmdline/depth_tests.py
>     subversion/trunk/subversion/tests/cmdline/merge_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=1158617&r1=1158616&r2=1158617&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Wed Aug 17
> 10:33:12 2011
> @@ -29,6 +29,7 @@
> 
>  

>  #include <string.h>
> +#include <stdlib.h>
> 
>  #include <apr_pools.h>
>  #include <apr_tables.h>
> @@ -1291,6 +1292,186 @@ 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;
> +}

Why do you reimplement svn_wc__internal_file_modified_p() here without the timestamp optimization?

Is the recorded timestamp + size already lost?
Or can we move this check a bit to allow using the existing functions?

(compare_and_verify usually accesses properties so I would assume the information is not lost, or your pristine check fails on some conditions like svn:eol-style native)

svn revert is commonly used to resolve missing file problems. I assume this code will never be used in this case?

> +
> +
> +/* Sort copied children obtained from the revert list based on
> + * their paths in descending order (longest paths first). */
> +static int
> +compare_revert_list_copied_children(const void *a, const void *b)
> +{
> +  const svn_wc__db_revert_list_copied_child_info_t *ca = a;
> +  const svn_wc__db_revert_list_copied_child_info_t *cb = b;
> +  int i;
> +
> +  i = svn_path_compare_paths(ca->abspath, cb->abspath);
> +
> +  /* Reverse the result of svn_path_compare_paths() to achieve
> +   * descending order. */
> +  return -i;
> +}
> +
> +
> +/* 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).
> + *
> + * 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.
> + *
> + * 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_dirs(svn_node_kind_t *new_kind,
> +                                  svn_wc__db_t *db,
> +                                  const char *local_abspath,
> +                                  svn_boolean_t remove_self,
> +                                  svn_cancel_func_t cancel_func,
> +                                  void *cancel_baton,
> +                                  apr_pool_t *scratch_pool)
> +{
> +  const apr_array_header_t *copied_children;
> +  svn_wc__db_revert_list_copied_child_info_t *child_info;
> +  int i;
> +  apr_finfo_t finfo;
> +  apr_pool_t *iterpool;
> +  svn_error_t *err;
> +
> +  if (new_kind)
> +    *new_kind = svn_node_dir;
> +
> +
> SVN_ERR(svn_wc__db_revert_list_read_copied_children(&copied_children,
> +                                                      db, local_abspath,
> +                                                      scratch_pool,
> +                                                      scratch_pool));
> +  iterpool = svn_pool_create(scratch_pool);
> +
> +  /* Remove all file children, if possible. */
> +  for (i = 0; i < copied_children->nelts; i++)
> +    {
> +      child_info = APR_ARRAY_IDX(
> +                     copied_children, i,
> +                     svn_wc__db_revert_list_copied_child_info_t *);
> +
> +      if (cancel_func)
> +        SVN_ERR(cancel_func(cancel_baton));
> +
> +      if (child_info->kind != svn_wc__db_kind_file)
> +        continue;
> +
> +      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)
> +        continue;

Using svn_io_stat_dirent would make this code much easier (and you could probably forward the dirent)

> +
> +      SVN_ERR(revert_restore_handle_copied_file(NULL, db, child_info-
> >abspath,
> +                                                finfo.size,
> +                                                child_info->pristine_checksum,
> +                                                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. */
> +  qsort(copied_children->elts, copied_children->nelts,
> +        sizeof(svn_wc__db_revert_list_copied_child_info_t *),
> +        compare_revert_list_copied_children);
> +  for (i = 0; i < copied_children->nelts; i++)
> +    {
> +      child_info = APR_ARRAY_IDX(
> +                     copied_children, i,
> +                     svn_wc__db_revert_list_copied_child_info_t *);
> +
> +      if (cancel_func)
> +        SVN_ERR(cancel_func(cancel_baton));
> +
> +      if (child_info->kind != svn_wc__db_kind_dir)
> +        continue;
> +
> +      svn_pool_clear(iterpool);
> +
> +      svn_error_clear(svn_io_dir_remove_nonrecursive(child_info->abspath,
> +                                                     iterpool));

Please handle explicit errors instead of ignoring all errors.

It is not nice that your operating system warns you that your filesystem is broken while the client just goes on as if nothing happened.
(Common problem in Subversion 1.6 on Windows 7 pre servicepack 1).

> +    }
> +
> +  if (remove_self)
> +    {
> +      apr_hash_t *remaining_children;
> +
> +      /* Delete LOCAL_ABSPATH itself if no children are left. */
> +      SVN_ERR(svn_io_get_dirents3(&remaining_children, local_abspath,
> TRUE,
> +                                  iterpool, iterpool));
> +      if (apr_hash_count(remaining_children) == 0)
> +        {
> +          SVN_ERR(svn_io_remove_dir2(local_abspath, FALSE, NULL, NULL,
> +                                     iterpool));
> +          if (new_kind)
> +            *new_kind = svn_node_none;
> +        }

This is an inefficient reimplementation of svn_io_dir_empty() and a call to svn_io_dir_remove_nonrecursive() would handle this check for you.

> +    }
> +
> +  svn_pool_destroy(iterpool);
> +
> +  return SVN_NO_ERROR;
> +}
> +
> +
>  /* Make the working tree under LOCAL_ABSPATH to depth DEPTH match
> the
>     versioned tree.  This function is called after svn_wc__db_op_revert
>     has done the database revert and created the revert list.  Notifies
> @@ -1321,6 +1502,9 @@ revert_restore(svn_wc__db_t *db,
>  #ifdef HAVE_SYMLINK
>    svn_boolean_t special;
>  #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));
> @@ -1328,6 +1512,8 @@ revert_restore(svn_wc__db_t *db,
>    SVN_ERR(svn_wc__db_revert_list_read(&notify_required,
>                                        &conflict_old, &conflict_new,
>                                        &conflict_working, &prop_reject,
> +                                      &copied_here, &reverted_kind,
> +                                      &pristine_checksum,
>                                        db, local_abspath,
>                                        scratch_pool, scratch_pool));
> 
> @@ -1342,17 +1528,21 @@ revert_restore(svn_wc__db_t *db,
>      {
>        svn_error_clear(err);
> 
> -      if (notify_func && notify_required)
> -        notify_func(notify_baton,
> -                    svn_wc_create_notify(local_abspath, svn_wc_notify_revert,
> -                                         scratch_pool),
> -                    scratch_pool);
> -
> -      if (notify_func)
> -        SVN_ERR(svn_wc__db_revert_list_notify(notify_func, notify_baton,
> -                                              db, local_abspath,
> -                                              scratch_pool));
> -      return SVN_NO_ERROR;
> +      if (!copied_here)
> +        {
> +          if (notify_func && notify_required)
> +            notify_func(notify_baton,
> +                        svn_wc_create_notify(local_abspath,
> +                                             svn_wc_notify_revert,
> +                                             scratch_pool),
> +                        scratch_pool);
> +
> +          if (notify_func)
> +            SVN_ERR(svn_wc__db_revert_list_notify(notify_func, notify_baton,
> +                                                  db, local_abspath,
> +                                                  scratch_pool));
> +          return SVN_NO_ERROR;
> +        }
>      }
>    else if (err)
>      return svn_error_trace(err);
> @@ -1387,6 +1577,21 @@ revert_restore(svn_wc__db_t *db,
>  #endif
>      }
> 
> +  if (copied_here)
> +    {
> +      /* 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));
> +      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,
> +                                                  cancel_func, cancel_baton,
> +                                                  scratch_pool));
> +    }
> +
>    /* If we expect a versioned item to be present then check that any
>       item on disk matches the versioned item, if it doesn't match then
>       fix it or delete it.  */
> @@ -1567,6 +1772,10 @@ revert_restore(svn_wc__db_t *db,
>        const apr_array_header_t *children;
>        int i;
> 
> +      SVN_ERR(revert_restore_handle_copied_dirs(NULL, db, local_abspath,
> FALSE,
> +                                                cancel_func, cancel_baton,
> +                                                iterpool));
> +
>        SVN_ERR(svn_wc__db_read_children_of_working_node(&children, db,
>                                                         local_abspath,
>                                                         scratch_pool,
> 
> Modified: subversion/trunk/subversion/libsvn_wc/questions.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/ques
> tions.c?rev=1158617&r1=1158616&r2=1158617&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/questions.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/questions.c Wed Aug 17
> 10:33:12 2011
> @@ -79,38 +79,17 @@
>  */
> 
> 
> -/* Set *MODIFIED_P to TRUE if (after translation)
> VERSIONED_FILE_ABSPATH
> - * (of VERSIONED_FILE_SIZE bytes) differs from PRISTINE_STREAM (of
> - * PRISTINE_SIZE bytes), else to FALSE if not.
> - *
> - * If EXACT_COMPARISON is FALSE, translate VERSIONED_FILE_ABSPATH's
> EOL
> - * style and keywords to repository-normal form according to its properties,
> - * and compare the result with PRISTINE_STREAM.  If EXACT_COMPARISON
> is
> - * TRUE, translate PRISTINE_STREAM's EOL style and keywords to working-
> copy
> - * form according to VERSIONED_FILE_ABSPATH's properties, and compare
> the
> - * result with VERSIONED_FILE_ABSPATH.
> - *
> - * HAS_PROPS should be TRUE if the file had properties when it was not
> - * modified, otherwise FALSE.
> - *
> - * PROPS_MOD should be TRUE if the file's properties have been changed,
> - * otherwise FALSE.
> - *
> - * PRISTINE_STREAM will be closed before a successful return.
> - *
> - * DB is a wc_db; use SCRATCH_POOL for temporary allocation.
> - */
> -static svn_error_t *
> -compare_and_verify(svn_boolean_t *modified_p,
> -                   svn_wc__db_t *db,
> -                   const char *versioned_file_abspath,
> -                   svn_filesize_t versioned_file_size,
> -                   svn_stream_t *pristine_stream,
> -                   svn_filesize_t pristine_size,
> -                   svn_boolean_t has_props,
> -                   svn_boolean_t props_mod,
> -                   svn_boolean_t exact_comparison,
> -                   apr_pool_t *scratch_pool)
> +svn_error_t *
> +svn_wc__compare_file_with_pristine(svn_boolean_t *modified_p,
> +                                   svn_wc__db_t *db,
> +                                   const char *versioned_file_abspath,
> +                                   svn_filesize_t versioned_file_size,
> +                                   svn_stream_t *pristine_stream,
> +                                   svn_filesize_t pristine_size,
> +                                   svn_boolean_t has_props,
> +                                   svn_boolean_t props_mod,
> +                                   svn_boolean_t exact_comparison,
> +                                   apr_pool_t *scratch_pool)
>  {
>    svn_boolean_t same;
>    svn_subst_eol_style_t eol_style;
> @@ -337,12 +316,12 @@ svn_wc__internal_file_modified_p(svn_boo
>    /* Check all bytes, and verify checksum if requested. */
>    {
>      svn_error_t *err;
> -    err = compare_and_verify(modified_p, db,
> -                             local_abspath, dirent->filesize,
> -                             pristine_stream, pristine_size,
> -                             has_props, props_mod,
> -                             exact_comparison,
> -                             scratch_pool);
> +    err = svn_wc__compare_file_with_pristine(modified_p, db,
> +                                             local_abspath, dirent->filesize,
> +                                             pristine_stream, pristine_size,
> +                                             has_props, props_mod,
> +                                             exact_comparison,
> +                                             scratch_pool);
> 
>      /* At this point we already opened the pristine file, so we know that
>         the access denied applies to the working copy path */
> 
> Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-
> queries.sql?rev=1158617&r1=1158616&r2=1158617&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Wed Aug 17
> 10:33:12 2011
> @@ -1112,14 +1112,20 @@ CREATE TEMPORARY TABLE revert_list (
>     conflict_working TEXT,
>     prop_reject TEXT,
>     notify INTEGER,         /* 1 if an actual row had props or tree conflict */
> +   op_depth INTEGER,
> +   repos_id INTEGER,
> +   kind TEXT,
> +   checksum TEXT,
>     PRIMARY KEY (local_relpath, actual)
>     );
>  DROP TRIGGER IF EXISTS   trigger_revert_list_nodes;
>  CREATE TEMPORARY TRIGGER trigger_revert_list_nodes
>  BEFORE DELETE ON nodes
>  BEGIN
> -   INSERT OR REPLACE INTO revert_list(local_relpath, actual)
> -   SELECT OLD.local_relpath, 0;
> +   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;
>  END;
>  DROP TRIGGER IF EXISTS   trigger_revert_list_actual_delete;
>  CREATE TEMPORARY TRIGGER trigger_revert_list_actual_delete
> @@ -1156,11 +1162,20 @@ DROP TRIGGER IF EXISTS trigger_revert_li
>  DROP TRIGGER IF EXISTS trigger_revert_list_actual_update
> 
>  -- STMT_SELECT_REVERT_LIST
> -SELECT conflict_old, conflict_new, conflict_working, prop_reject, notify,
> actual
> +SELECT conflict_old, conflict_new, conflict_working, prop_reject, notify,
> +       actual, op_depth, repos_id, kind, checksum
>  FROM revert_list
>  WHERE local_relpath = ?1
>  ORDER BY actual DESC
> 
> +-- STMT_SELECT_REVERT_LIST_COPIED_CHILDREN
> +SELECT local_relpath, kind, checksum
> +FROM revert_list
> +WHERE local_relpath LIKE ?1 ESCAPE '#'
> +  AND op_depth >= ?2
> +  AND repos_id IS NOT NULL
> +ORDER BY local_relpath
> +
>  -- STMT_DELETE_REVERT_LIST
>  DELETE FROM revert_list WHERE local_relpath = ?1
> 
> 
> Modified: subversion/trunk/subversion/libsvn_wc/wc.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc.h
> ?rev=1158617&r1=1158616&r2=1158617&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/wc.h (original)
> +++ subversion/trunk/subversion/libsvn_wc/wc.h Wed Aug 17 10:33:12 2011
> @@ -725,6 +725,39 @@ svn_wc__perform_file_merge(svn_skel_t **
>                             apr_pool_t *result_pool,
>                             apr_pool_t *scratch_pool);
> 
> +/* Set *MODIFIED_P to TRUE if (after translation)
> VERSIONED_FILE_ABSPATH
> + * (of VERSIONED_FILE_SIZE bytes) differs from PRISTINE_STREAM (of
> + * PRISTINE_SIZE bytes), else to FALSE if not.
> + *
> + * If EXACT_COMPARISON is FALSE, translate VERSIONED_FILE_ABSPATH's
> EOL
> + * style and keywords to repository-normal form according to its properties,
> + * and compare the result with PRISTINE_STREAM.  If EXACT_COMPARISON
> is
> + * TRUE, translate PRISTINE_STREAM's EOL style and keywords to working-
> copy
> + * form according to VERSIONED_FILE_ABSPATH's properties, and compare
> the
> + * result with VERSIONED_FILE_ABSPATH.
> + *
> + * HAS_PROPS should be TRUE if the file had properties when it was not
> + * modified, otherwise FALSE.
> + *
> + * PROPS_MOD should be TRUE if the file's properties have been changed,
> + * otherwise FALSE.
> + *
> + * PRISTINE_STREAM will be closed before a successful return.
> + *
> + * DB is a wc_db; use SCRATCH_POOL for temporary allocation.
> + */
> +svn_error_t *
> +svn_wc__compare_file_with_pristine(svn_boolean_t *modified_p,
> +                                   svn_wc__db_t *db,
> +                                   const char *versioned_file_abspath,
> +                                   svn_filesize_t versioned_file_size,
> +                                   svn_stream_t *pristine_stream,
> +                                   svn_filesize_t pristine_size,
> +                                   svn_boolean_t has_props,
> +                                   svn_boolean_t props_mod,
> +                                   svn_boolean_t exact_comparison,
> +                                   apr_pool_t *scratch_pool);
> +
> 
>  #ifdef __cplusplus
>  }
> 

<snip>

	Bert