subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Julian Foad <julianf...@gmail.com>
Subject svnsync crash on empty repo
Date Sat, 05 Sep 2015 10:01:51 GMT
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

Mime
View raw message