[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svnsync crash on empty repo

From: <bert_at_qqmail.nl>
Date: Sat, 5 Sep 2015 13:15:44 +0200

I don't think we should fix this with a 'revision+1' to explicitly allow many bad ranges, but I do think we should specifically fix the r0 case for empty repositories.

Bert

Sent from Mail for Windows 10

From: Julian Foad
Sent: zaterdag 5 september 2015 12:02
To: dev;Bert Huijben
Subject: svnsync crash on empty repo

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
Received on 2015-09-05 13:15:47 CEST

This is an archived mail posted to the Subversion Dev mailing list.