Stefan Fuhrmann <stefan.fuhrm...@wandisco.com> writes:
> Could you at least use the new API in svn_repos__compare_files instead
> of re-implementing parts of the FS back-end but worse? I know this is
> the code as it has been in 1.8 but that does not make the it any better.
Speaking of /branches/1.9.x, I would like to nominate this change as is.
It should be easier to review, because we would be restoring things to a
known previous state, instead of mixing new with old.
As for /trunk, I think that we could do that, so I sketched this option in
the attached patch. Currently I am not sure that there are no subtle but
important differences between two implementations, so doing this is going
to require a bit more time. Hopefully, I would be able to sort it out after
we're done with the backport.
Regards,
Evgeny Kotkov
Index: subversion/libsvn_fs_fs/dag.c
===================================================================
--- subversion/libsvn_fs_fs/dag.c (revision 1709790)
+++ subversion/libsvn_fs_fs/dag.c (working copy)
@@ -1319,9 +1319,6 @@ svn_fs_fs__dag_things_different(svn_boolean_t *pro
{
/* 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
Index: subversion/libsvn_repos/delta.c
===================================================================
--- subversion/libsvn_repos/delta.c (revision 1709790)
+++ subversion/libsvn_repos/delta.c (working copy)
@@ -603,73 +603,6 @@ send_text_delta(struct context *c,
}
}
-svn_error_t *
-svn_repos__compare_files(svn_boolean_t *changed_p,
- svn_fs_root_t *root1,
- const char *path1,
- svn_fs_root_t *root2,
- const char *path2,
- apr_pool_t *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;
-}
-
-
/* Make the appropriate edits on FILE_BATON to change its contents and
properties from those in SOURCE_PATH to those in TARGET_PATH. */
static svn_error_t *
@@ -700,10 +633,10 @@ delta_files(struct context *c,
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));
+ SVN_ERR(svn_fs_contents_different(&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,
Index: subversion/libsvn_repos/reporter.c
===================================================================
--- subversion/libsvn_repos/reporter.c (revision 1709790)
+++ subversion/libsvn_repos/reporter.c (working copy)
@@ -691,8 +691,8 @@ delta_files(report_baton_t *b, void *file_baton, s
contents which have not changed with respect to" and "has the same
actual contents as" when sending text-deltas. If we know the
delta is an empty one, we avoiding sending it in either case. */
- SVN_ERR(svn_repos__compare_files(&changed, b->t_root, t_path,
- s_root, s_path, pool));
+ SVN_ERR(svn_fs_contents_different(&changed, b->t_root, t_path,
+ s_root, s_path, pool));
if (!changed)
return SVN_NO_ERROR;
Index: subversion/libsvn_repos/repos.h
===================================================================
--- subversion/libsvn_repos/repos.h (revision 1709790)
+++ subversion/libsvn_repos/repos.h (working copy)
@@ -390,17 +390,6 @@ svn_repos__authz_validate(svn_authz_t *authz,
/*** Utility Functions ***/
-/* Set *CHANGED_P to TRUE if ROOT1/PATH1 and ROOT2/PATH2 have
- different contents, FALSE if they have the same contents.
- Use POOL for temporary allocation. */
-svn_error_t *
-svn_repos__compare_files(svn_boolean_t *changed_p,
- svn_fs_root_t *root1,
- const char *path1,
- svn_fs_root_t *root2,
- const char *path2,
- apr_pool_t *pool);
-
/* Set *PREV_PATH and *PREV_REV to the path and revision which
represent the location at which PATH in FS was located immediately
prior to REVISION iff there was a copy operation (to PATH or one of
