Author: rhuijben
Date: Wed May 11 10:18:07 2011
New Revision: 1101817

URL: http://svn.apache.org/viewvc?rev=1101817&view=rev
Log:
Following up on r1101473 (and several other followups), resolve the regression
in our issue #1663 handling, by moving the special recorded information
handling exception in the revert code itself.

The revert code already stats the file, so we can just make it retrieve
the information it needs directly. This also allows removing two optional
output arguments from svn_wc__internal_file_modified_p that were only used
by the revert code.

* subversion/libsvn_wc/adm_ops.c
  (includes): Add private/svn_io_private.h
  (revert_restore): Extend our own stat to also retrieve size, mtime,
    read only and executable. Compare against the recorded information
    locally before calling svn_wc__internal_file_modified_p in the slow
    exact mode. Stop assuming that all symlinks should always be reverted
    on systems that don't support symlinks.
  (svn_wc__internal_remove_from_revision_control): Update caller.

* subversion/libsvn_wc/cleanup.c
  (repair_timestamps): Update caller. Handle symlink as file.

* subversion/libsvn_wc/copy.c
  (copy_versioned_file): Update caller.

* subversion/libsvn_wc/diff_editor.c
  (file_diff): Update caller.
  (close_file): Update caller.

* subversion/libsvn_wc/diff_local.c
  (file_diff): Update caller.

* subversion/libsvn_wc/externals.c
  (close_file): Update caller.

* subversion/libsvn_wc/questions.c
  (includes): Remove private/svn_io_private.h
  (svn_wc__internal_file_modified_p): Remove two arguments. Use
    svn_io_stat_dirent instead of svn_io_stat.

* subversion/libsvn_wc/status.c
  (assemble_status): Update caller.

* subversion/libsvn_wc/update_editor.c
  (merge_file): Update caller.

* subversion/libsvn_wc/wc.h
  (svn_wc__internal_file_modified_p): Remove two arguments. Update
    documentation.

* subversion/libsvn_wc/wc_db.c
  (has_local_mods): Update caller.

* subversion/libsvn_wc/workqueue.c
  (process_commit_file_install): Update caller.

* subversion/tests/cmdline/revert_tests.py
  (revert_reexpand_keyword): Remove XFail. Assert that the first part of the
    test succeeds, but that we don't revert the file after we fixed the
    recorded information.

Modified:
    subversion/trunk/subversion/libsvn_wc/adm_ops.c
    subversion/trunk/subversion/libsvn_wc/cleanup.c
    subversion/trunk/subversion/libsvn_wc/copy.c
    subversion/trunk/subversion/libsvn_wc/diff_editor.c
    subversion/trunk/subversion/libsvn_wc/diff_local.c
    subversion/trunk/subversion/libsvn_wc/externals.c
    subversion/trunk/subversion/libsvn_wc/questions.c
    subversion/trunk/subversion/libsvn_wc/status.c
    subversion/trunk/subversion/libsvn_wc/update_editor.c
    subversion/trunk/subversion/libsvn_wc/wc.h
    subversion/trunk/subversion/libsvn_wc/wc_db.c
    subversion/trunk/subversion/libsvn_wc/workqueue.c
    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=1101817&r1=1101816&r2=1101817&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Wed May 11 10:18:07 2011
@@ -56,6 +56,7 @@
 #include "workqueue.h"
 
 #include "svn_private_config.h"
+#include "private/svn_io_private.h"
 #include "private/svn_wc_private.h"
 
 
@@ -1318,6 +1319,9 @@ revert_restore(svn_wc__db_t *db,
   const char *conflict_new;
   const char *conflict_working;
   const char *prop_reject;
+  svn_filesize_t recorded_size;
+  apr_time_t recorded_mod_time;
+  apr_finfo_t finfo;
 
   if (cancel_func)
     SVN_ERR(cancel_func(cancel_baton));
@@ -1330,9 +1334,9 @@ revert_restore(svn_wc__db_t *db,
 
   err = svn_wc__db_read_info(&status, &kind,
                              NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-                             NULL, NULL, NULL, NULL, NULL, NULL, NULL, 
                              NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-                             NULL, NULL, NULL,
+                             &recorded_size, &recorded_mod_time, NULL,
+                             NULL, NULL, NULL, NULL, NULL, NULL, NULL,
                              db, local_abspath, scratch_pool, scratch_pool);
 
   if (err && err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
@@ -1356,8 +1360,31 @@ revert_restore(svn_wc__db_t *db,
   else if (err)
     return svn_error_return(err);
 
-  SVN_ERR(svn_io_check_special_path(local_abspath, &on_disk, &special,
-                                    scratch_pool));
+  err = svn_io_stat(&finfo, local_abspath, 
+                    APR_FINFO_TYPE | APR_FINFO_LINK
+                    | APR_FINFO_SIZE | APR_FINFO_MTIME
+                    | SVN__APR_FINFO_EXECUTABLE
+                    | SVN__APR_FINFO_READONLY,
+                    scratch_pool);
+
+  if (err && (APR_STATUS_IS_ENOENT(err->apr_err)
+              || SVN__APR_STATUS_IS_ENOTDIR(err->apr_err)))
+    {
+      svn_error_clear(err);
+      on_disk = svn_node_none;
+      special = FALSE;
+    }
+  else
+    {
+      if (finfo.filetype == APR_REG || finfo.filetype == APR_LNK)
+        on_disk = svn_node_file;
+      else if (finfo.filetype == APR_DIR)
+        on_disk = svn_node_dir;
+      else
+        on_disk = svn_node_unknown;
+
+      special = (finfo.filetype == APR_LNK);
+    }
 
   /* 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
@@ -1393,18 +1420,47 @@ revert_restore(svn_wc__db_t *db,
           special_prop = apr_hash_get(props, SVN_PROP_SPECIAL,
                                       APR_HASH_KEY_STRING);
 
-          if ((special_prop != NULL) != special)
+#ifdef HAVE_SYMLINK
+          if ((special_prop != NULL) != dirent->special)
             {
               /* File/symlink mismatch. */
               SVN_ERR(svn_io_remove_file2(local_abspath, FALSE, scratch_pool));
               on_disk = svn_node_none;
             }
           else
+#endif
             {
-              SVN_ERR(svn_wc__internal_file_modified_p(&modified, &executable,
-                                                       &read_only,
-                                                       db, local_abspath,
-                                                       FALSE, scratch_pool));
+              /* Issue #1663 asserts that we should compare a file in its
+                 working copy format here, but before r1101473 we would only
+                 do that if the file was already unequal to its recorded
+                 information.
+
+                 r1101473 removes the option of asking for a working format
+                 compare but *also* check the recorded information first, as
+                 that combination doesn't guarantee a stable behavior.
+                 (See the revert_test.py: revert_reexpand_keyword)
+
+                 But to have the same issue #1663 behavior for revert as we
+                 had in <=1.6 we only have to check the recorded information
+                 ourselves. And we already have everything we need, because
+                 we called stat ourselves. */
+              if (recorded_size != SVN_INVALID_FILESIZE
+                  && recorded_mod_time != 0
+                  && recorded_size == finfo.size
+                  && recorded_mod_time == finfo.mtime)
+                {
+                  modified = FALSE;
+                }
+              else
+                SVN_ERR(svn_wc__internal_file_modified_p(&modified,
+                                                         db, local_abspath,
+                                                         TRUE, scratch_pool));
+
+              SVN_ERR(svn_io__is_finfo_executable(&executable, &finfo,
+                                                  scratch_pool));
+              SVN_ERR(svn_io__is_finfo_read_only(&read_only, &finfo,
+                                                 scratch_pool));
+
               if (modified)
                 {
                   SVN_ERR(svn_io_remove_file2(local_abspath, FALSE,
@@ -1911,8 +1967,7 @@ svn_wc__internal_remove_from_revision_co
           if (on_disk == svn_node_file)
             {
               /* Check for local mods. before removing entry */
-              SVN_ERR(svn_wc__internal_file_modified_p(&text_modified_p, NULL,
-                                                       NULL, db,
+              SVN_ERR(svn_wc__internal_file_modified_p(&text_modified_p, db,
                                                        local_abspath, FALSE,
                                                        scratch_pool));
               if (text_modified_p && instant_error)

Modified: subversion/trunk/subversion/libsvn_wc/cleanup.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/cleanup.c?rev=1101817&r1=1101816&r2=1101817&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/cleanup.c (original)
+++ subversion/trunk/subversion/libsvn_wc/cleanup.c Wed May 11 10:18:07 2011
@@ -95,10 +95,11 @@ repair_timestamps(svn_wc__db_t *db,
       || status == svn_wc__db_status_not_present)
     return SVN_NO_ERROR;
 
-  if (kind == svn_wc__db_kind_file)
+  if (kind == svn_wc__db_kind_file
+      || kind == svn_wc__db_kind_symlink)
     {
       svn_boolean_t modified;
-      SVN_ERR(svn_wc__internal_file_modified_p(&modified, NULL, NULL,
+      SVN_ERR(svn_wc__internal_file_modified_p(&modified,
                                                db, local_abspath, FALSE,
                                                scratch_pool));
     }

Modified: subversion/trunk/subversion/libsvn_wc/copy.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/copy.c?rev=1101817&r1=1101816&r2=1101817&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/copy.c (original)
+++ subversion/trunk/subversion/libsvn_wc/copy.c Wed May 11 10:18:07 2011
@@ -287,7 +287,7 @@ copy_versioned_file(svn_wc__db_t *db,
                  the timestamp might match, than to examine the
                  destination later as the destination timestamp will
                  never match. */
-              SVN_ERR(svn_wc__internal_file_modified_p(&modified, NULL, NULL,
+              SVN_ERR(svn_wc__internal_file_modified_p(&modified,
                                                        db, src_abspath,
                                                        FALSE, scratch_pool));
               if (!modified)

Modified: subversion/trunk/subversion/libsvn_wc/diff_editor.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/diff_editor.c?rev=1101817&r1=1101816&r2=1101817&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/diff_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/diff_editor.c Wed May 11 10:18:07 2011
@@ -700,9 +700,8 @@ file_diff(struct edit_baton *eb,
       svn_boolean_t modified;
 
       /* Here we deal with showing pure modifications. */
-      SVN_ERR(svn_wc__internal_file_modified_p(&modified, NULL, NULL, db,
-                                               local_abspath, FALSE,
-                                               scratch_pool));
+      SVN_ERR(svn_wc__internal_file_modified_p(&modified, db, local_abspath,
+                                               FALSE, scratch_pool));
       if (modified)
         {
           /* Note that this might be the _second_ time we translate
@@ -1735,7 +1734,7 @@ close_file(void *file_baton,
      (BASE:WORKING) modifications. */
   modified = (fb->temp_file_path != NULL);
   if (!modified && !eb->use_text_base)
-    SVN_ERR(svn_wc__internal_file_modified_p(&modified, NULL, NULL, eb->db,
+    SVN_ERR(svn_wc__internal_file_modified_p(&modified, eb->db,
                                              fb->local_abspath,
                                              FALSE, scratch_pool));
 

Modified: subversion/trunk/subversion/libsvn_wc/diff_local.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/diff_local.c?rev=1101817&r1=1101816&r2=1101817&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/diff_local.c (original)
+++ subversion/trunk/subversion/libsvn_wc/diff_local.c Wed May 11 10:18:07 2011
@@ -354,9 +354,8 @@ file_diff(struct diff_baton *eb,
       svn_boolean_t modified;
 
       /* Here we deal with showing pure modifications. */
-      SVN_ERR(svn_wc__internal_file_modified_p(&modified, NULL, NULL, db,
-                                               local_abspath, FALSE,
-                                               scratch_pool));
+      SVN_ERR(svn_wc__internal_file_modified_p(&modified, db, local_abspath,
+                                               FALSE, scratch_pool));
       if (modified)
         {
           /* Note that this might be the _second_ time we translate

Modified: subversion/trunk/subversion/libsvn_wc/externals.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/externals.c?rev=1101817&r1=1101816&r2=1101817&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/externals.c (original)
+++ subversion/trunk/subversion/libsvn_wc/externals.c Wed May 11 10:18:07 2011
@@ -739,7 +739,7 @@ close_file(void *file_baton,
         else
           {
             svn_boolean_t is_mod;
-            SVN_ERR(svn_wc__internal_file_modified_p(&is_mod, NULL, NULL,
+            SVN_ERR(svn_wc__internal_file_modified_p(&is_mod,
                                                      eb->db, eb->local_abspath,
                                                      FALSE, pool));
 

Modified: subversion/trunk/subversion/libsvn_wc/questions.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/questions.c?rev=1101817&r1=1101816&r2=1101817&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/questions.c (original)
+++ subversion/trunk/subversion/libsvn_wc/questions.c Wed May 11 10:18:07 2011
@@ -47,7 +47,6 @@
 
 #include "svn_private_config.h"
 #include "private/svn_wc_private.h"
-#include "private/svn_io_private.h"
 
 
 
@@ -237,8 +236,6 @@ compare_and_verify(svn_boolean_t *modifi
 
 svn_error_t *
 svn_wc__internal_file_modified_p(svn_boolean_t *modified_p,
-                                 svn_boolean_t *executable_p,
-                                 svn_boolean_t *read_only_p,
                                  svn_wc__db_t *db,
                                  const char *local_abspath,
                                  svn_boolean_t exact_comparison,
@@ -253,40 +250,10 @@ svn_wc__internal_file_modified_p(svn_boo
   apr_time_t recorded_mod_time;
   svn_boolean_t has_props;
   svn_boolean_t props_mod;
-  svn_error_t *err;
-  apr_finfo_t finfo;
-  apr_int32_t wanted
-    = APR_FINFO_SIZE | APR_FINFO_MTIME | APR_FINFO_TYPE | APR_FINFO_LINK;
-
-  if (executable_p)
-    wanted |= SVN__APR_FINFO_EXECUTABLE;
-  if (read_only_p)
-    wanted |= SVN__APR_FINFO_READONLY;
-
-  /* No matter which way you look at it, the file needs to exist. */
-  err = svn_io_stat(&finfo, local_abspath, wanted, scratch_pool);
-  if ((err && APR_STATUS_IS_ENOENT(err->apr_err))
-      || (!err && !(finfo.filetype == APR_REG ||
-                    finfo.filetype == APR_LNK)))
-    {
-      /* There is no entity, or, the entity is not a regular file or link.
-         So, it can't be modified. */
-      svn_error_clear(err);
-      *modified_p = FALSE;
-      if (executable_p)
-        *executable_p = FALSE;
-      if (read_only_p)
-        *read_only_p = FALSE;
-
-      return SVN_NO_ERROR;
-    }
-  else if (err)
-    return err;
+  const svn_io_dirent2_t *dirent;
 
-  if (executable_p)
-    SVN_ERR(svn_io__is_finfo_executable(executable_p, &finfo, scratch_pool));
-  if (read_only_p)
-    SVN_ERR(svn_io__is_finfo_read_only(read_only_p, &finfo, scratch_pool));
+  SVN_ERR(svn_io_stat_dirent(&dirent, local_abspath, TRUE,
+                             scratch_pool, scratch_pool));
 
   /* Read the relevant info */
   SVN_ERR(svn_wc__db_read_info(&status, &kind, NULL, NULL, NULL, NULL, NULL,
@@ -340,7 +307,7 @@ svn_wc__internal_file_modified_p(svn_boo
 
       /* Compare the sizes, if applicable */
       if (recorded_size != SVN_INVALID_FILESIZE
-          && finfo.size != recorded_size)
+          && dirent->filesize != recorded_size)
         goto compare_them;
 
       /* Compare the timestamps
@@ -348,7 +315,7 @@ svn_wc__internal_file_modified_p(svn_boo
          Note: recorded_mod_time == 0 means not available,
                which also means the timestamps won't be equal,
                so there's no need to explicitly check the 'absent' value. */
-      if (recorded_mod_time != finfo.mtime)
+      if (recorded_mod_time != dirent->mtime)
         goto compare_them;
 
       *modified_p = FALSE;
@@ -362,7 +329,7 @@ svn_wc__internal_file_modified_p(svn_boo
 
   /* Check all bytes, and verify checksum if requested. */
   SVN_ERR(compare_and_verify(modified_p, db,
-                             local_abspath, finfo.size,
+                             local_abspath, dirent->filesize,
                              pristine_stream, pristine_size,
                              has_props, props_mod,
                              exact_comparison,
@@ -377,7 +344,8 @@ svn_wc__internal_file_modified_p(svn_boo
                                           scratch_pool));
       if (own_lock)
         SVN_ERR(svn_wc__db_global_record_fileinfo(db, local_abspath,
-                                                  finfo.size, finfo.mtime,
+                                                  dirent->filesize,
+                                                  dirent->mtime,
                                                   scratch_pool));
     }
 
@@ -394,7 +362,7 @@ svn_wc_text_modified_p2(svn_boolean_t *m
 {
   /* ### We ignore FORCE_COMPARISON, but we also fixed its only
          remaining use-case */
-  return svn_wc__internal_file_modified_p(modified_p, NULL, NULL, wc_ctx->db,
+  return svn_wc__internal_file_modified_p(modified_p, wc_ctx->db,
                                           local_abspath, FALSE, scratch_pool);
 }
 

Modified: subversion/trunk/subversion/libsvn_wc/status.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/status.c?rev=1101817&r1=1101816&r2=1101817&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/status.c (original)
+++ subversion/trunk/subversion/libsvn_wc/status.c Wed May 11 10:18:07 2011
@@ -516,8 +516,8 @@ assemble_status(svn_wc_status3_t **statu
             text_modified_p = FALSE;
           else
             {
-              err = svn_wc__internal_file_modified_p(&text_modified_p, NULL,
-                                                     NULL, db, local_abspath,
+              err = svn_wc__internal_file_modified_p(&text_modified_p,
+                                                     db, local_abspath,
                                                      FALSE, scratch_pool);
 
               if (err)

Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=1101817&r1=1101816&r2=1101817&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Wed May 11 10:18:07 
2011
@@ -3398,8 +3398,8 @@ merge_file(svn_skel_t **work_items,
          This function sets is_locally_modified to FALSE for
          files that do not exist and for directories. */
 
-      SVN_ERR(svn_wc__internal_file_modified_p(&is_locally_modified, NULL,
-                                               NULL, eb->db, fb->local_abspath,
+      SVN_ERR(svn_wc__internal_file_modified_p(&is_locally_modified,
+                                               eb->db, fb->local_abspath,
                                                FALSE /* exact_comparison */,
                                                scratch_pool));
     }

Modified: subversion/trunk/subversion/libsvn_wc/wc.h
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc.h?rev=1101817&r1=1101816&r2=1101817&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc.h (original)
+++ subversion/trunk/subversion/libsvn_wc/wc.h Wed May 11 10:18:07 2011
@@ -360,17 +360,7 @@ void svn_wc__compat_call_notify_func(voi
                                      apr_pool_t *pool);
 
 /* Set *MODIFIED_P to non-zero if LOCAL_ABSPATH's text is modified with
- * regard to the base revision, else set *MODIFIED_P to zero.  Also
- * set *EXECUTABLE_P and *READ_ONLY_P based on the files current
- * permissions.  (EXECUTABLE_P and READ_ONLY_P can individually be
- * NULL if the caller doesn't care about those attributes of the file.)
- *
- * If FORCE_COMPARISON is true, this function will not allow early
- * return mechanisms that avoid actual content comparison.  Instead,
- * if there is a text base, a full byte-by-byte comparison will be
- * done, and the entry checksum verified as well.  (This means that if
- * the text base is much longer than the working file, every byte of
- * the text base will still be examined.)
+ * regard to the base revision, else set *MODIFIED_P to zero.
  *
  * If EXACT_COMPARISON is FALSE, translate LOCAL_ABSPATH's EOL
  * style and keywords to repository-normal form according to its properties,
@@ -392,8 +382,6 @@ void svn_wc__compat_call_notify_func(voi
  */
 svn_error_t *
 svn_wc__internal_file_modified_p(svn_boolean_t *modified_p,
-                                 svn_boolean_t *executable_p,
-                                 svn_boolean_t *read_only_p,
                                  svn_wc__db_t *db,
                                  const char *local_abspath,
                                  svn_boolean_t exact_comparison,

Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1101817&r1=1101816&r2=1101817&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed May 11 10:18:07 2011
@@ -12004,8 +12004,8 @@ has_local_mods(svn_boolean_t *is_modifie
           node_kind = svn_sqlite__column_token(stmt, 1, kind_map);
           if (node_kind == svn_wc__db_kind_file)
             {
-              SVN_ERR(svn_wc__internal_file_modified_p(is_modified, NULL,
-                                                       NULL, db, node_abspath,
+              SVN_ERR(svn_wc__internal_file_modified_p(is_modified,
+                                                       db, node_abspath,
                                                        FALSE, iterpool));
               if (*is_modified)
                 break;

Modified: subversion/trunk/subversion/libsvn_wc/workqueue.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/workqueue.c?rev=1101817&r1=1101816&r2=1101817&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/workqueue.c (original)
+++ subversion/trunk/subversion/libsvn_wc/workqueue.c Wed May 11 10:18:07 2011
@@ -495,7 +495,7 @@ process_commit_file_install(svn_wc__db_t
          that already does implement this when it notices that we have the
          right kind of lock (and we ignore the result)
        */
-      SVN_ERR(svn_wc__internal_file_modified_p(&modified, NULL, NULL,
+      SVN_ERR(svn_wc__internal_file_modified_p(&modified,
                                                db, local_abspath, FALSE,
                                                scratch_pool));
     }

Modified: subversion/trunk/subversion/tests/cmdline/revert_tests.py
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/revert_tests.py?rev=1101817&r1=1101816&r2=1101817&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/revert_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/revert_tests.py Wed May 11 
10:18:07 2011
@@ -231,28 +231,6 @@ def revert_from_wc_root(sbox):
 
   svntest.actions.run_and_verify_status('', expected_output)
 
-# I temporarily mark this test XFail as the current implementation
-# (and the one in 1.5/1.6) contain a contradiction.
-#
-# If the recorded timestamp and size match the file (E.g. after 'svn cleanup')
-# then revert won't reinstall the file as the file is 'not modified'
-# when compared in repository normal form.
-#
-# But if there is no recorded information, revert would ask to compare the file
-# in translated form, so it would notice that the file was modified.
-#
-# We should decide:
-#  * if we want to perform a full scan (and catch these expansion changes
-#    always), at the cost of performing a full scan from revert.
-#
-#  * Don't catch these changes (and do nothing). Keep this test broken.
-#
-#  * Implement another level of recording 'unmodified'
-#    (one for unmodified in translated form and one for normalized or similar).
-#
-#  * Or revert r1101473, which removed this contradiction. So this option would
-#    would fix the test, but not the real problem.
-@XFail()
 @Issue(1663)
 def revert_reexpand_keyword(sbox):
   "revert reexpands manually contracted keyword"
@@ -306,7 +284,18 @@ def revert_reexpand_keyword(sbox):
   # Verify that the keyword got re-expanded.
   check_expanded(newfile_path)
 
-  # Now un-expand the keyword again.
+  # Ok, the first part of this test was written in 2004. We are now in 2011
+  # and note that there is more to test:
+
+  # If the recorded timestamp and size match the file then revert won't
+  # reinstall the file as the file was not modified when last compared in
+  # the repository normal form.
+  #
+  # The easiest way to get the information recorded would be calling cleanup,
+  # because that 'repairs' the recorded information. But some developers
+  # (including me) would call that cheating, so I just use a failed commit.
+
+  # Un-expand the keyword again.
   svntest.main.file_write(newfile_path, unexpanded_contents)
 
   # And now we trick svn in ignoring the file on newfile_path
@@ -316,17 +305,14 @@ def revert_reexpand_keyword(sbox):
   os.remove(newfile2_path)
 
   # This commit fails because newfile2_path is missing, but only after
-  # we call svn_wc__internal_file_modified_p() on new_file
+  # we call svn_wc__internal_file_modified_p() on new_file.
   svntest.actions.run_and_verify_commit(wc_dir, None, None, "2' is scheduled"+
                                         " for addition, but is missing",
                                         newfile_path, newfile2_path,
                                         '-m', "Shouldn't be committed")
 
-  # Revert the file.  The keyword should reexpand.
-  svntest.main.run_svn(None, 'revert', newfile_path)
-
-  # Verify that the keyword got re-expanded.
-  check_expanded(newfile_path)
+  # Revert the file.  The file is not reverted!
+  svntest.actions.run_and_verify_svn(None, [], [], 'revert', newfile_path)
 
 
 #----------------------------------------------------------------------


Reply via email to