On 21.05.2015 17:23, Evgeny Kotkov wrote:
> 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
I completed your patch and committed the fix in r1683311. Please review!
-- Brane
Received on 2015-06-03 14:34:11 CEST