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

Possible incompatibility of svn_repos_verify_fs2() in 1.9.0-rc1

From: Evgeny Kotkov <evgeny.kotkov_at_visualsvn.com>
Date: Thu, 21 May 2015 18:23:22 +0300

Subversion 1.9.0-rc1 introduced a new svnadmin verify --keep-going mode [1].
In order to achieve this, we added a svn_repos_verify_fs3() API function and
deprecated its predecessor, svn_repos_verify_fs2(), that now calls the newer
function with keep_going argument set to FALSE.

However, svn_repos_verify_fs2() behaves differently in 1.9.0-rc1 and 1.8.13
when it comes to error reporting. As an example, the following code ...

  SVN_ERR(svn_repos_verify_fs2(repos, 1, 5, NULL, NULL, NULL, NULL, pool));

...would return two different errors depending on the binaries being used,
assuming that one of the revisions in the [r1:r5] range is corrupted:

 (With 1.8.13) E160004: Checksum mismatch in item at offset 0 of
    length 59 bytes in file path/asf/db/revs/0/2

 (With 1.9.0-rc1) E165011: Repository 'path/asf' failed to verify

Please note that the second error is generic, and that the actual information
about the error is lost. Existing API users of svn_repos_verify_fs2() are
going to lose an important bit of information about the root cause of the
corruption unless they update the code. Furthermore, even if an API caller
subscribes to notifications with a notify_func / notify_baton pair, it would
still be necessary to update the code to handle svn_repos_notify_failure that
did not exist before 1.9.

I did not find any discussion on the matter or the corresponding entry in
/notes/api-errata [2] that would describe this behavior change, so I am
inclined to think that this is accidental and, probably, undesirable.

There is an option of restoring the 1.8 behavior when svn_repos_verify_fs3()
is called with keep_going set to FALSE. We could immediately yield the error
in this case, and only use the notifications / generic error when keep_going
is set to TRUE. Doing so would change the output of svnadmin verify without
--keep-going, because now it wouldn't include the generic error message in
the end. I attached a proof-of-concept patch, that doesn't change the test
expectations and the documentation, i.e., it only includes the core change,
mostly as an illustration to these points.

Thoughts?

[1] https://svn.apache.org/r1492651
[2] https://svn.apache.org/repos/asf/subversion/trunk/notes/api-errata/1.9

Regards,
Evgeny Kotkov

Received on 2015-05-21 17:25:22 CEST

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