I (Julian Foad) wrote:
> On trunk (released versions are unaffected), "svnsync sync" with an
> empty source repo fails an assertion in svn_ra_replay_range():
In fact this problem is not specific to an empty repository, but to
any repository that is already fully synchronized. An empty repository
just happened to be the first case I came across.
- Julian
> 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
Received on 2015-09-05 12:12:39 CEST