Hi, Bert.
On trunk (released versions are unaffected), "svnsync sync" with an
empty source repo fails an assertion in svn_ra_replay_range():
subversion/libsvn_ra/ra_loader.c' line 1198: assertion failed
(SVN_IS_VALID_REVNUM(start_revision) &&
SVN_IS_VALID_REVNUM(end_revision) && start_revision <= end_revision &&
SVN_IS_VALID_REVNUM(low_water_mark))
because start_revision is 1 and end_revision is 0.
The assertion was added in r1665480
<http://svn.apache.org/viewvc?revision=1665480&view=revision>, with
the log msg mentioning "Add assertions here that already apply to some
ra layers."
One possible fix is to avoid calling it with an empty revision range, like this:
Index: subversion/svnsync/svnsync.c
===================================================================
--- subversion/svnsync/svnsync.c (revision 1701278)
+++ subversion/svnsync/svnsync.c (working copy)
@@ -1551,3 +1551,3 @@ do_synchronize(svn_ra_session_t *to_sess
- if (from_latest < last_merged)
+ if (from_latest <= last_merged)
return SVN_NO_ERROR;
That makes the caller return early when there are no revisions to
sync. The only other real caller (svnrdump) already uses a similar
condition.
However, I think we need to continue allowing the historical usage of
svn_ra_replay_range() for backward compatibility. Should the assertion
should be changed like this?
- && start_revision <= end_revision
+ && start_revision <= (end_revision + 1)
I haven't yet seen where the problem or restriction existed in some RA
layers, so I don't know if that's a good enough fix on its own. My new
svnsync test (empty repo) passes over all RA layers with this change.
The attached patch contains both possible fixes, a log msg and a
regression test.
(I found this by running tests with the "--dump-load-cross-check"
option, which I have not done for a while. Doing so also reports
several other test failures, mostly just because those tests use
deliberately invalid data.)
- Julian
Fix a crash in 'svnsync sync' when the source repository is empty.
### This patch currently contains two possible fixes. See the email
discussion about which one or ones we want.
* subversion/libsvn_ra/ra_loader.c
(svn_ra_replay_range): Relax the assertion to allow an empty range.
* subversion/svnsync/svnsync.c
(do_synchronize): Return earlier if there are no new revisions to
synchronize.
* subversion/tests/cmdline/svnsync_tests_data/empty-repository.dump
New file.
* subversion/tests/cmdline/svnsync_tests.py
(setup_and_sync,
run_sync): Allow specifying the expected output.
(empty_repository): New test.
(run_test): Run it.
--This line, and those below, will be ignored--
Index: subversion/libsvn_ra/ra_loader.c
===================================================================
--- subversion/libsvn_ra/ra_loader.c (revision 1701278)
+++ subversion/libsvn_ra/ra_loader.c (working copy)
@@ -1191,13 +1191,13 @@ svn_ra_replay_range(svn_ra_session_t *se
apr_pool_t *pool)
{
svn_error_t *err;
SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(start_revision)
&& SVN_IS_VALID_REVNUM(end_revision)
- && start_revision <= end_revision
+ && start_revision <= (end_revision + 1)
&& SVN_IS_VALID_REVNUM(low_water_mark));
err =
session->vtable->replay_range(session, start_revision, end_revision,
low_water_mark, text_deltas,
revstart_func, revfinish_func,
Index: subversion/svnsync/svnsync.c
===================================================================
--- subversion/svnsync/svnsync.c (revision 1701278)
+++ subversion/svnsync/svnsync.c (working copy)
@@ -1546,13 +1546,13 @@ do_synchronize(svn_ra_session_t *to_sess
to_latest, last_merged);
}
/* Now check to see if there are any revisions to copy. */
SVN_ERR(svn_ra_get_latest_revnum(from_session, &from_latest, pool));
- if (from_latest < last_merged)
+ if (from_latest <= last_merged)
return SVN_NO_ERROR;
/* Ok, so there are new revisions, iterate over them copying them
into the destination repository. */
SVN_ERR(make_replay_baton(&rb, from_session, to_session, baton, pool));
Index: subversion/tests/cmdline/svnsync_tests_data/empty-repository.dump
===================================================================
--- subversion/tests/cmdline/svnsync_tests_data/empty-repository.dump (revision 0)
+++ subversion/tests/cmdline/svnsync_tests_data/empty-repository.dump (working copy)
@@ -0,0 +1,14 @@
+SVN-fs-dump-format-version: 2
+
+UUID: 6ad9f820-0205-0410-94a2-c8cf366bb2b3
+
+Revision-number: 0
+Prop-content-length: 56
+Content-length: 56
+
+K 8
+svn:date
+V 27
+2005-11-07T23:36:48.095832Z
+PROPS-END
+
Index: subversion/tests/cmdline/svnsync_tests.py
===================================================================
--- subversion/tests/cmdline/svnsync_tests.py (revision 1701278)
+++ subversion/tests/cmdline/svnsync_tests.py (working copy)
@@ -102,13 +102,14 @@ def run_info(url, expected_output=AnyOut
svntest.actions.run_and_verify_svnsync(expected_output, expected_error,
"info", url)
def setup_and_sync(sbox, dump_file_contents, subdir=None,
bypass_prop_validation=False, source_prop_encoding=None,
- is_src_ra_local=None, is_dest_ra_local=None):
+ is_src_ra_local=None, is_dest_ra_local=None,
+ expected_output_sync=AnyOutput):
"""Create a repository for SBOX, load it with DUMP_FILE_CONTENTS, then create a mirror repository and sync it with SBOX. If is_src_ra_local or is_dest_ra_local is True, then run_init, run_sync, and run_copy_revprops will use the file:// scheme for the source and destination URLs. Return the mirror sandbox."""
# Create the empty master repository.
sbox.build(create_wc=False, empty=True)
# Load the repository from DUMP_FILE_PATH.
@@ -139,13 +140,14 @@ def setup_and_sync(sbox, dump_file_conte
dest_repo_url = dest_sbox.repo_url
if is_dest_ra_local:
dest_repo_url = dest_sbox.file_protocol_repo_url()
run_init(dest_repo_url, repo_url, source_prop_encoding)
run_sync(dest_repo_url, repo_url,
- source_prop_encoding=source_prop_encoding)
+ source_prop_encoding=source_prop_encoding,
+ expected_output=expected_output_sync)
run_copy_revprops(dest_repo_url, repo_url,
source_prop_encoding=source_prop_encoding)
return dest_sbox
def verify_mirror(dest_sbox, exp_dump_file_contents):
@@ -168,13 +170,14 @@ def verify_mirror(dest_sbox, exp_dump_fi
svntest.verify.compare_dump_files(
"Dump files", "DUMP", exp_dump_file_contents, dest_dump)
def run_test(sbox, dump_file_name, subdir=None, exp_dump_file_name=None,
bypass_prop_validation=False, source_prop_encoding=None,
- is_src_ra_local=None, is_dest_ra_local=None):
+ is_src_ra_local=None, is_dest_ra_local=None,
+ expected_output_sync=AnyOutput):
"""Load a dump file, sync repositories, and compare contents with the original
or another dump file."""
# This directory contains all the dump files
svnsync_tests_dir = os.path.join(os.path.dirname(sys.argv[0]),
@@ -184,13 +187,14 @@ or another dump file."""
master_dumpfile_contents = open(os.path.join(svnsync_tests_dir,
dump_file_name),
'rb').readlines()
dest_sbox = setup_and_sync(sbox, master_dumpfile_contents, subdir,
bypass_prop_validation, source_prop_encoding,
- is_src_ra_local, is_dest_ra_local)
+ is_src_ra_local, is_dest_ra_local,
+ expected_output_sync=expected_output_sync)
# Compare the dump produced by the mirror repository with either the original
# dump file (used to create the master repository) or another specified dump
# file.
if exp_dump_file_name:
exp_dump_file_contents = open(os.path.join(svnsync_tests_dir,
@@ -569,12 +573,18 @@ PROPS-END
# Run the sync
dest_sbox = setup_and_sync(sbox, dump_in, bypass_prop_validation=True)
# Compare the dump produced by the mirror repository with expected
verify_mirror(dest_sbox, dump_out)
+#----------------------------------------------------------------------
+
+def empty_repository(sbox):
+ "empty repository"
+ run_test(sbox, "empty-repository.dump", expected_output_sync=[])
+
########################################################################
# Run the tests
# list all tests here, starting with None:
@@ -606,12 +616,13 @@ test_list = [ None,
delete_svn_props,
commit_a_copy_of_root,
descend_into_replace,
delete_revprops,
fd_leak_sync_from_serf_to_local, # calls setrlimit
mergeinfo_contains_r0,
+ empty_repository,
]
if __name__ == '__main__':
svntest.main.run_tests(test_list)
# NOTREACHED
