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
