subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From <b...@qqmail.nl>
Subject RE: svnsync crash on empty repo
Date Sat, 05 Sep 2015 11:15:44 GMT
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



Mime
View raw message