You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ko...@apache.org on 2015/10/19 12:55:50 UTC

svn commit: r1709388 - in /subversion/trunk/subversion: include/ libsvn_fs_base/ libsvn_fs_fs/ libsvn_repos/ tests/cmdline/

Author: kotkov
Date: Mon Oct 19 10:55:50 2015
New Revision: 1709388

URL: http://svn.apache.org/viewvc?rev=1709388&view=rev
Log:
Restore the 1.8 behavior of svn_fs_contents_changed() and _props_changed()
API.  Switch all calling sites of the new API, svn_fs_contents_different()
and _props_different(), back to using the old functions.

There are no user-visible problems associated with the old code.  The new
API doesn't improve any real use cases in the current code, but is causing
problems:

- We had a problem with misbehaving svn blame -g
  (http://svn.haxx.se/dev/archive-2015-06/0069.shtml, "Blame behaviour
   change in 1.9").

- We have an issue with repositories behaving differently in client-side
  operations like 'svn log' after dump/load
  (http://svn.haxx.se/dev/archive-2015-09/0269.shtml, "No-op changes no
   longer dumped by 'svnadmin dump' in 1.9"; also see issue #4598 in
   https://issues.apache.org/jira/browse/SVN-4598).

- We could experience same problems originating from other callers of the
  new API, because the low level behavior change associated with switching
  to it propagates up to higher levels like svn_repos or svn_ra and alters
  the behavior of many different callers like svn_ra_get_file_revs2() or
  the update reporter.  Third-party API callers could not be ready for it
  as well, because public API functions like svn_ra_get_file_revs2() didn't
  receive an erratum or a bump.

See the discussion in http://svn.haxx.se/dev/archive-2015-10/0022.shtml
("Re: No-op changes no longer dumped by 'svnadmin dump' in 1.9").

* subversion/libsvn_fs_base/dag.c
  (svn_fs_base__things_different): Compare the uniquifiers, as we did in 1.8.

* subversion/libsvn_fs_base/fs.h
  (node_revision_t.data_key_uniquifier): Remove the comment about not using
   this field.

* subversion/libsvn_fs_fs/fs_fs.c
  (svn_fs_fs__noderev_same_rep_key): Reintroduce this helper function.
  (svn_fs_fs__file_text_rep_equal, svn_fs_fs__prop_rep_equal): Always
   assume the strict mode in these helpers.

* subversion/libsvn_fs_fs/fs_fs.h
  (svn_fs_fs__noderev_same_rep_key): Declare this re-added helper.
  (svn_fs_fs__file_text_rep_equal, svn_fs_fs__prop_rep_equal): Update the
   docstrings for these helper functions.

* subversion/libsvn_fs_fs/dag.c
  (svn_fs_fs__dag_things_different): Preserve the current comparison behavior
   in strict mode.  Restore the 1.8 way of comparing the representation keys
   in non-strict mode.

* subversion/libsvn_fs_fs/tree.c
  (merge): Restore the 1.8 way of comparing property lists.

* subversion/libsvn_fs_fs/fs.h
  (representation_t.uniquifier): Remove the comment about not using this
   field.

* subversion/libsvn_repos/delta.c
  (delta_proplists): Switch back to using svn_fs_props_changed().
  (svn_repos__compare_files): Restore this function to its 1.8 state.
  (delta_files): Restore the 1.8 way of comparing files.

* subversion/libsvn_repos/dump.c
  (dump_node): Switch back to using svn_fs_contents_changed() and
   svn_fs_props_changed().

* subversion/libsvn_repos/reporter.c
  (delta_proplists): Switch back to using svn_fs_props_changed().

* subversion/libsvn_repos/rev_hunt.c
  (send_path_revision): Switch back to using svn_fs_contents_changed().
   Remove the no longer necessary hack for svn blame -g with older clients.

* subversion/include/svn_ra.h
  (svn_ra_get_file_revs2): Update @note in the docstring.

* subversion/include/svn_repos.h
  (svn_repos_get_file_revs2): Update @note in the docstring.

* subversion/tests/cmdline/svnadmin_tests.py
  (dump_no_op_change): No longer fails with fsfs and bdb.

Modified:
    subversion/trunk/subversion/include/svn_ra.h
    subversion/trunk/subversion/include/svn_repos.h
    subversion/trunk/subversion/libsvn_fs_base/dag.c
    subversion/trunk/subversion/libsvn_fs_base/fs.h
    subversion/trunk/subversion/libsvn_fs_fs/dag.c
    subversion/trunk/subversion/libsvn_fs_fs/fs.h
    subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
    subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h
    subversion/trunk/subversion/libsvn_fs_fs/tree.c
    subversion/trunk/subversion/libsvn_repos/delta.c
    subversion/trunk/subversion/libsvn_repos/dump.c
    subversion/trunk/subversion/libsvn_repos/reporter.c
    subversion/trunk/subversion/libsvn_repos/rev_hunt.c
    subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py

Modified: subversion/trunk/subversion/include/svn_ra.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_ra.h?rev=1709388&r1=1709387&r2=1709388&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_ra.h (original)
+++ subversion/trunk/subversion/include/svn_ra.h Mon Oct 19 10:55:50 2015
@@ -1797,11 +1797,10 @@ svn_ra_get_location_segments(svn_ra_sess
  * to support reversion of the revision range for @a include_merged_revision
  * @c FALSE reporting by switching  @a end with @a start.
  *
- * @note Prior to Subversion 1.9, this function may request delta handlers
- * from @a handler even for empty text deltas.  Starting with 1.9, the
- * delta handler / baton return arguments passed to @a handler will be
- * #NULL unless there is an actual difference in the file contents between
- * the current and the previous call.
+ * @note In Subversion 1.9.0-1.9.2, the delta handler / baton return
+ * arguments passed to @a handler were set to #NULL in case of an empty
+ * text delta.  All other versions may request delta handlers from
+ * @a handler even for empty text deltas.
  *
  * @since New in 1.5.
  */

Modified: subversion/trunk/subversion/include/svn_repos.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_repos.h?rev=1709388&r1=1709387&r2=1709388&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_repos.h (original)
+++ subversion/trunk/subversion/include/svn_repos.h Mon Oct 19 10:55:50 2015
@@ -2093,11 +2093,10 @@ svn_repos_fs_get_mergeinfo(svn_mergeinfo
  * the revision range for @a include_merged_revision @c FALSE reporting by
  * switching @a start with @a end.
  *
- * @note Prior to Subversion 1.9, this function may request delta handlers
- * from @a handler even for empty text deltas.  Starting with 1.9, the
- * delta handler / baton return arguments passed to @a handler will be
- * #NULL unless there is an actual difference in the file contents between
- * the current and the previous call.
+ * @note In Subversion 1.9.0-1.9.2, the delta handler / baton return
+ * arguments passed to @a handler were set to #NULL in case of an empty
+ * text delta.  All other versions may request delta handlers from
+ * @a handler even for empty text deltas.
  *
  * @since New in 1.5.
  */

Modified: subversion/trunk/subversion/libsvn_fs_base/dag.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_base/dag.c?rev=1709388&r1=1709387&r2=1709388&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_base/dag.c (original)
+++ subversion/trunk/subversion/libsvn_fs_base/dag.c Mon Oct 19 10:55:50 2015
@@ -1657,8 +1657,14 @@ svn_fs_base__things_different(svn_boolea
 
   /* Compare contents keys and their (optional) uniquifiers. */
   if (contents_changed != NULL)
-    *contents_changed = (! svn_fs_base__same_keys(noderev1->data_key,
-                                                  noderev2->data_key));
+    *contents_changed =
+      (! (svn_fs_base__same_keys(noderev1->data_key,
+                                 noderev2->data_key)
+          /* Technically, these uniquifiers aren't used and "keys",
+             but keys are base-36 stringified numbers, so we'll take
+             this liberty. */
+          && (svn_fs_base__same_keys(noderev1->data_key_uniquifier,
+                                     noderev2->data_key_uniquifier))));
 
   return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/libsvn_fs_base/fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_base/fs.h?rev=1709388&r1=1709387&r2=1709388&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_base/fs.h (original)
+++ subversion/trunk/subversion/libsvn_fs_base/fs.h Mon Oct 19 10:55:50 2015
@@ -195,11 +195,7 @@ typedef struct node_revision_t
      only because one or both of us decided to pick up a shared
      representation after-the-fact."  May be NULL (if this node
      revision isn't using a shared rep, or isn't the original
-     "assignee" of a shared rep).
-
-     This is no longer used by the 1.9 code but we have to keep
-     reading and writing it to remain compatible with 1.8, and
-     earlier, that require it. */
+     "assignee" of a shared rep). */
   const char *data_key_uniquifier;
 
   /* representation key for this node's text-data-in-progess (files

Modified: subversion/trunk/subversion/libsvn_fs_fs/dag.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/dag.c?rev=1709388&r1=1709387&r2=1709388&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/dag.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/dag.c Mon Oct 19 10:55:50 2015
@@ -1305,34 +1305,61 @@ svn_fs_fs__dag_things_different(svn_bool
                                 apr_pool_t *pool)
 {
   node_revision_t *noderev1, *noderev2;
-  svn_fs_t *fs;
-  svn_boolean_t same;
 
   /* If we have no place to store our results, don't bother doing
      anything. */
   if (! props_changed && ! contents_changed)
     return SVN_NO_ERROR;
 
-  fs = svn_fs_fs__dag_get_fs(node1);
-
   /* The node revision skels for these two nodes. */
   SVN_ERR(get_node_revision(&noderev1, node1));
   SVN_ERR(get_node_revision(&noderev2, node2));
 
-  /* Compare property keys. */
-  if (props_changed != NULL)
+  if (strict)
     {
-      SVN_ERR(svn_fs_fs__prop_rep_equal(&same, fs, noderev1, noderev2,
-                                        strict, pool));
-      *props_changed = !same;
+      /* In strict mode, compare text and property representations in the
+         svn_fs_contents_different() / svn_fs_props_different() manner.
+         These functions are currently not being used in our codebase, but
+         we released them as a part of 1.9, and keep them for compatibility
+         reasons.
+
+         See the "No-op changes no longer dumped by 'svnadmin dump' in 1.9"
+         discussion (http://svn.haxx.se/dev/archive-2015-09/0269.shtml) and
+         issue #4598 (https://issues.apache.org/jira/browse/SVN-4598). */
+      svn_fs_t *fs = svn_fs_fs__dag_get_fs(node1);
+      svn_boolean_t same;
+
+      /* Compare property keys. */
+      if (props_changed != NULL)
+        {
+          SVN_ERR(svn_fs_fs__prop_rep_equal(&same, fs, noderev1,
+                                            noderev2, pool));
+          *props_changed = !same;
+        }
+
+      /* Compare contents keys. */
+      if (contents_changed != NULL)
+        {
+          SVN_ERR(svn_fs_fs__file_text_rep_equal(&same, fs, noderev1,
+                                                 noderev2, pool));
+          *contents_changed = !same;
+        }
     }
-
-  /* Compare contents keys. */
-  if (contents_changed != NULL)
+  else
     {
-      SVN_ERR(svn_fs_fs__file_text_rep_equal(&same, fs, noderev1, noderev2,
-                                             strict, pool));
-      *contents_changed = !same;
+      /* Otherwise, compare representation keys -- as in Subversion 1.8. */
+
+      /* Compare property keys. */
+      if (props_changed != NULL)
+        *props_changed =
+          !svn_fs_fs__noderev_same_rep_key(noderev1->prop_rep,
+                                           noderev2->prop_rep);
+
+      /* Compare contents keys. */
+      if (contents_changed != NULL)
+        *contents_changed =
+          !svn_fs_fs__noderev_same_rep_key(noderev1->data_rep,
+                                           noderev2->data_rep);
     }
 
   return SVN_NO_ERROR;

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs.h?rev=1709388&r1=1709387&r2=1709388&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs.h Mon Oct 19 10:55:50 2015
@@ -550,13 +550,7 @@ typedef struct representation_t
   /* For rep-sharing, we need a way of uniquifying node-revs which share the
      same representation (see svn_fs_fs__noderev_same_rep_key() ).  So, we
      store the original txn of the node rev (not the rep!), along with some
-     intra-node uniqification content.
-
-     This is no longer used by the 1.9 code but we have to keep
-     reading and writing it for old formats to remain compatible with
-     1.8, and earlier, that require it.  We also read/write it in
-     format 7 even though it is not currently required by any code
-     that handles that format. */
+     intra-node uniqification content. */
   struct
     {
       /* unique context, i.e. txn ID, in which the noderev (!) got created */

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1709388&r1=1709387&r2=1709388&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Mon Oct 19 10:55:50 2015
@@ -1387,12 +1387,30 @@ svn_fs_fs__file_length(svn_filesize_t *l
   return SVN_NO_ERROR;
 }
 
+svn_boolean_t
+svn_fs_fs__noderev_same_rep_key(representation_t *a,
+                                representation_t *b)
+{
+  if (a == b)
+    return TRUE;
+
+  if (a == NULL || b == NULL)
+    return FALSE;
+
+  if (a->item_index != b->item_index)
+    return FALSE;
+
+  if (a->revision != b->revision)
+    return FALSE;
+
+  return memcmp(&a->uniquifier, &b->uniquifier, sizeof(a->uniquifier)) == 0;
+}
+
 svn_error_t *
 svn_fs_fs__file_text_rep_equal(svn_boolean_t *equal,
                                svn_fs_t *fs,
                                node_revision_t *a,
                                node_revision_t *b,
-                               svn_boolean_t strict,
                                apr_pool_t *scratch_pool)
 {
   svn_stream_t *contents_a, *contents_b;
@@ -1437,19 +1455,6 @@ svn_fs_fs__file_text_rep_equal(svn_boole
       return SVN_NO_ERROR;
     }
 
-  /* Old repositories may not have the SHA1 checksum handy.
-     This check becomes expensive.  Skip it unless explicitly required.
-
-     We already have seen that the ID is different, so produce a likely
-     false negative as allowed by the API description - even though the
-     MD5 matched, there is an extremely slim chance that the SHA1 wouldn't.
-   */
-  if (!strict)
-    {
-      *equal = FALSE;
-      return SVN_NO_ERROR;
-    }
-
   SVN_ERR(svn_fs_fs__get_contents(&contents_a, fs, rep_a, TRUE,
                                   scratch_pool));
   SVN_ERR(svn_fs_fs__get_contents(&contents_b, fs, rep_b, TRUE,
@@ -1465,7 +1470,6 @@ svn_fs_fs__prop_rep_equal(svn_boolean_t
                           svn_fs_t *fs,
                           node_revision_t *a,
                           node_revision_t *b,
-                          svn_boolean_t strict,
                           apr_pool_t *scratch_pool)
 {
   representation_t *rep_a = a->prop_rep;
@@ -1512,14 +1516,6 @@ svn_fs_fs__prop_rep_equal(svn_boolean_t
       return SVN_NO_ERROR;
     }
 
-  /* Skip the expensive bits unless we are in strict mode.
-     Simply assume that there is a difference. */
-  if (!strict)
-    {
-      *equal = FALSE;
-      return SVN_NO_ERROR;
-    }
-
   /* At least one of the reps has been modified in a txn.
      Fetch and compare them. */
   SVN_ERR(svn_fs_fs__get_proplist(&proplist_a, fs, a, scratch_pool));

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h?rev=1709388&r1=1709387&r2=1709388&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h Mon Oct 19 10:55:50 2015
@@ -90,28 +90,29 @@ svn_error_t *svn_fs_fs__file_length(svn_
                                     node_revision_t *noderev,
                                     apr_pool_t *pool);
 
+/* Return TRUE if the representation keys in A and B both point to the
+   same representation, else return FALSE. */
+svn_boolean_t svn_fs_fs__noderev_same_rep_key(representation_t *a,
+                                              representation_t *b);
+
 /* Set *EQUAL to TRUE if the text representations in A and B within FS
-   have equal contents, else set it to FALSE.  If STRICT is not set, allow
-   for false negatives.
+   have equal contents, else set it to FALSE.
    Use SCRATCH_POOL for temporary allocations. */
 svn_error_t *
 svn_fs_fs__file_text_rep_equal(svn_boolean_t *equal,
                                svn_fs_t *fs,
                                node_revision_t *a,
                                node_revision_t *b,
-                               svn_boolean_t strict,
                                apr_pool_t *scratch_pool);
 
 /* Set *EQUAL to TRUE if the property representations in A and B within FS
-   have equal contents, else set it to FALSE.  If STRICT is not set, allow
-   for false negatives.
+   have equal contents, else set it to FALSE.
    Use SCRATCH_POOL for temporary allocations. */
 svn_error_t *
 svn_fs_fs__prop_rep_equal(svn_boolean_t *equal,
                           svn_fs_t *fs,
                           node_revision_t *a,
                           node_revision_t *b,
-                          svn_boolean_t strict,
                           apr_pool_t *scratch_pool);
 
 

Modified: subversion/trunk/subversion/libsvn_fs_fs/tree.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/tree.c?rev=1709388&r1=1709387&r2=1709388&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/tree.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/tree.c Mon Oct 19 10:55:50 2015
@@ -1882,7 +1882,6 @@ merge(svn_stringbuf_t *conflict_p,
   */
   {
     node_revision_t *tgt_nr, *anc_nr, *src_nr;
-    svn_boolean_t same;
     apr_pool_t *scratch_pool;
 
     /* Get node revisions for our id's. */
@@ -1899,15 +1898,16 @@ merge(svn_stringbuf_t *conflict_p,
 
     /* Now compare the prop-keys of the skels.  Note that just because
        the keys are different -doesn't- mean the proplists have
-       different contents. */
-    SVN_ERR(svn_fs_fs__prop_rep_equal(&same, fs, src_nr, anc_nr, TRUE, pool));
-    if (! same)
+       different contents.  But merge() isn't concerned with contents;
+       it doesn't do a brute-force comparison on textual contents, so
+       it won't do that here either.  Checking to see if the propkey
+       atoms are `equal' is enough. */
+    if (! svn_fs_fs__noderev_same_rep_key(src_nr->prop_rep, anc_nr->prop_rep))
       return conflict_err(conflict_p, target_path);
 
     /* The directory entries got changed in the repository but the directory
        properties did not. */
-    SVN_ERR(svn_fs_fs__prop_rep_equal(&same, fs, tgt_nr, anc_nr, TRUE, pool));
-    if (! same)
+    if (! svn_fs_fs__noderev_same_rep_key(tgt_nr->prop_rep, anc_nr->prop_rep))
       {
         /* There is an incoming prop change for this directory.
            We will accept it only if the directory changes were mere updates

Modified: subversion/trunk/subversion/libsvn_repos/delta.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/delta.c?rev=1709388&r1=1709387&r2=1709388&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/delta.c (original)
+++ subversion/trunk/subversion/libsvn_repos/delta.c Mon Oct 19 10:55:50 2015
@@ -530,8 +530,8 @@ delta_proplists(struct context *c,
       svn_boolean_t changed;
 
       /* Is this deltification worth our time? */
-      SVN_ERR(svn_fs_props_different(&changed, c->target_root, target_path,
-                                     c->source_root, source_path, subpool));
+      SVN_ERR(svn_fs_props_changed(&changed, c->target_root, target_path,
+                                   c->source_root, source_path, subpool));
       if (! changed)
         goto cleanup;
 
@@ -611,8 +611,62 @@ svn_repos__compare_files(svn_boolean_t *
                          const char *path2,
                          apr_pool_t *pool)
 {
-  return svn_error_trace(svn_fs_contents_different(changed_p, root1, path1,
-                                                   root2, path2, pool));
+  svn_filesize_t size1, size2;
+  svn_checksum_t *checksum1, *checksum2;
+  svn_stream_t *stream1, *stream2;
+  svn_boolean_t same;
+
+  /* If the filesystem claims the things haven't changed, then they
+     haven't changed. */
+  SVN_ERR(svn_fs_contents_changed(changed_p, root1, path1,
+                                  root2, path2, pool));
+  if (!*changed_p)
+    return SVN_NO_ERROR;
+
+  /* If the SHA1 checksums match for these things, we'll claim they
+     have the same contents.  (We don't give quite as much weight to
+     MD5 checksums.)  */
+  SVN_ERR(svn_fs_file_checksum(&checksum1, svn_checksum_sha1,
+                               root1, path1, FALSE, pool));
+  SVN_ERR(svn_fs_file_checksum(&checksum2, svn_checksum_sha1,
+                               root2, path2, FALSE, pool));
+  if (checksum1 && checksum2)
+    {
+      *changed_p = !svn_checksum_match(checksum1, checksum2);
+      return SVN_NO_ERROR;
+    }
+
+  /* From this point on, our default answer is "Nothing's changed". */
+  *changed_p = FALSE;
+
+  /* Different filesizes means the contents are different. */
+  SVN_ERR(svn_fs_file_length(&size1, root1, path1, pool));
+  SVN_ERR(svn_fs_file_length(&size2, root2, path2, pool));
+  if (size1 != size2)
+    {
+      *changed_p = TRUE;
+      return SVN_NO_ERROR;
+    }
+
+  /* Different MD5 checksums means the contents are different. */
+  SVN_ERR(svn_fs_file_checksum(&checksum1, svn_checksum_md5, root1, path1,
+                               FALSE, pool));
+  SVN_ERR(svn_fs_file_checksum(&checksum2, svn_checksum_md5, root2, path2,
+                               FALSE, pool));
+  if (!svn_checksum_match(checksum1, checksum2))
+    {
+      *changed_p = TRUE;
+      return SVN_NO_ERROR;
+    }
+
+  /* And finally, different contents means the ... uh ... contents are
+     different. */
+  SVN_ERR(svn_fs_file_contents(&stream1, root1, path1, pool));
+  SVN_ERR(svn_fs_file_contents(&stream2, root2, path2, pool));
+  SVN_ERR(svn_stream_contents_same2(&same, stream1, stream2, pool));
+  *changed_p = !same;
+
+  return SVN_NO_ERROR;
 }
 
 
@@ -639,7 +693,19 @@ delta_files(struct context *c,
 
   if (source_path)
     {
-      SVN_ERR(svn_fs_contents_different(&changed,
+      /* Is this delta calculation worth our time?  If we are ignoring
+         ancestry, then our editor implementor isn't concerned by the
+         theoretical differences between "has contents which have not
+         changed with respect to" and "has the same actual contents
+         as".  We'll do everything we can to avoid transmitting even
+         an empty text-delta in that case.  */
+      if (c->ignore_ancestry)
+        SVN_ERR(svn_repos__compare_files(&changed,
+                                         c->target_root, target_path,
+                                         c->source_root, source_path,
+                                         subpool));
+      else
+        SVN_ERR(svn_fs_contents_changed(&changed,
                                         c->target_root, target_path,
                                         c->source_root, source_path,
                                         subpool));

Modified: subversion/trunk/subversion/libsvn_repos/dump.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/dump.c?rev=1709388&r1=1709387&r2=1709388&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/dump.c (original)
+++ subversion/trunk/subversion/libsvn_repos/dump.c Mon Oct 19 10:55:50 2015
@@ -1164,13 +1164,13 @@ dump_node(struct edit_baton *eb,
                                    svn_fs_root_fs(eb->fs_root),
                                    compare_rev, pool));
 
-      SVN_ERR(svn_fs_props_different(&must_dump_props,
-                                     compare_root, compare_path,
-                                     eb->fs_root, path, pool));
+      SVN_ERR(svn_fs_props_changed(&must_dump_props,
+                                   compare_root, compare_path,
+                                   eb->fs_root, path, pool));
       if (kind == svn_node_file)
-        SVN_ERR(svn_fs_contents_different(&must_dump_text,
-                                          compare_root, compare_path,
-                                          eb->fs_root, path, pool));
+        SVN_ERR(svn_fs_contents_changed(&must_dump_text,
+                                        compare_root, compare_path,
+                                        eb->fs_root, path, pool));
       break;
 
     case svn_node_action_delete:
@@ -1293,16 +1293,16 @@ dump_node(struct edit_baton *eb,
 
           /* Need to decide if the copied node had any extra textual or
              property mods as well.  */
-          SVN_ERR(svn_fs_props_different(&must_dump_props,
-                                         compare_root, compare_path,
-                                         eb->fs_root, path, pool));
+          SVN_ERR(svn_fs_props_changed(&must_dump_props,
+                                       compare_root, compare_path,
+                                       eb->fs_root, path, pool));
           if (kind == svn_node_file)
             {
               svn_checksum_t *checksum;
               const char *hex_digest;
-              SVN_ERR(svn_fs_contents_different(&must_dump_text,
-                                                compare_root, compare_path,
-                                                eb->fs_root, path, pool));
+              SVN_ERR(svn_fs_contents_changed(&must_dump_text,
+                                              compare_root, compare_path,
+                                              eb->fs_root, path, pool));
 
               SVN_ERR(svn_fs_file_checksum(&checksum, svn_checksum_md5,
                                            compare_root, compare_path,

Modified: subversion/trunk/subversion/libsvn_repos/reporter.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/reporter.c?rev=1709388&r1=1709387&r2=1709388&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/reporter.c (original)
+++ subversion/trunk/subversion/libsvn_repos/reporter.c Mon Oct 19 10:55:50 2015
@@ -578,8 +578,8 @@ delta_proplists(report_baton_t *b, svn_r
       SVN_ERR(get_source_root(b, &s_root, s_rev));
 
       /* Is this deltification worth our time? */
-      SVN_ERR(svn_fs_props_different(&changed, b->t_root, t_path, s_root,
-                                     s_path, pool));
+      SVN_ERR(svn_fs_props_changed(&changed, b->t_root, t_path, s_root,
+                                   s_path, pool));
       if (! changed)
         return SVN_NO_ERROR;
 

Modified: subversion/trunk/subversion/libsvn_repos/rev_hunt.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_repos/rev_hunt.c?rev=1709388&r1=1709387&r2=1709388&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_repos/rev_hunt.c (original)
+++ subversion/trunk/subversion/libsvn_repos/rev_hunt.c Mon Oct 19 10:55:50 2015
@@ -1374,31 +1374,17 @@ send_path_revision(struct path_revision
   SVN_ERR(svn_prop_diffs(&prop_diffs, props, sb->last_props,
                          sb->iterpool));
 
-  /* Check if the contents *may* have changed. */
+  /* Check if the contents changed. */
   if (! sb->last_root)
     {
       /* Special case: In the first revision, we always provide a delta. */
       contents_changed = TRUE;
     }
-  else if (sb->include_merged_revisions
-           && strcmp(sb->last_path, path_rev->path))
-    {
-      /* ### This is a HACK!!!
-       * Blame -g, in older clients anyways, relies on getting a notification
-       * whenever the path changes - even if there was no content change.
-       *
-       * TODO: A future release should take an extra parameter and depending
-       * on that either always send a text delta or only send it if there
-       * is a difference. */
-      contents_changed = TRUE;
-    }
   else
     {
-      /* Did the file contents actually change?
-       * It could e.g. be a property-only change. */
-      SVN_ERR(svn_fs_contents_different(&contents_changed, sb->last_root,
-                                        sb->last_path, root, path_rev->path,
-                                        sb->iterpool));
+      SVN_ERR(svn_fs_contents_changed(&contents_changed, sb->last_root,
+                                      sb->last_path, root, path_rev->path,
+                                      sb->iterpool));
     }
 
   /* We have all we need, give to the handler. */

Modified: subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py?rev=1709388&r1=1709387&r2=1709388&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py Mon Oct 19 10:55:50 2015
@@ -3202,7 +3202,7 @@ def dump_revprops(sbox):
   svntest.actions.run_and_verify_svnlook(log_msg, [], 'log', '-r1',
                                          sbox.repo_dir)
 
-@XFail()
+@XFail(svntest.main.is_fs_type_fsx)
 @Issue(4598)
 def dump_no_op_change(sbox):
   "svnadmin dump with no-op changes"