Branko Čibej <brane_at_wandisco.com> writes:
>> Should be fixed in r1684325.
>
> Meh ... siliness. It's actually r1684344. The correct fix is in
> svnadmin; always sending notifications is correct from API users'
> perspective, too.
Thanks for looking into this.
Frankly, I cannot say that adding a flag like b->silent_errors looks exactly
right to me. The calling site is now bound to an assumption that the error
notification is the *same* as the error returned by svn_repos_verify_fs3() —
otherwise we would be discarding a potentially useful part of the error info.
Maybe this indicates that we could do a better job in designing the API?
I think that if even we do this sort of silencing in svnadmin.c, potential
users of svn_repos_verify_fs3() would have to introduce a similar logic and
this could be confusing for them.
Furthermore, I can't get rid of the feeling that --keep-going implementation
has certain flaws in terms of both the API and the UI design. Another example
of what's currently wrong would be the behavior of svnadmin verify --keep-going
--quiet, because right now the output is useless for the end user. Here is a
quick sample:
svnadmin verify --keep-going -q asf-part
svnadmin: E165011: Verification of 14 out of 108221 revisions failed on
repository 'C:\asf-part'
We don't output errors with --quiet, and I think that this isn't something an
administrator wants to see after executing verify for a couple of hours, e.g.,
for a huge repository. She would have to re-run the same command to get a
hint of exactly what is wrong, even if the intention was just to suppress the
progress. Worth mentioning, the test suite didn't catch this problem or the
double error reporting bug, and I treat this is an indication that the tests
probably require more work.
I didn't yet look into fixing this, but I am thinking that we're now trying to
close the holes found in the --keep-going implementation through the STATUS
file — and it doesn't sound quite right to me. Maybe the API was overly bent
for a particular usage scenario or maybe we just need a fresh look on this, but
without fixing it we could be shipping a half-baked feature and would have to
maintain the corresponding API. Fixing this, on the other hand, could mean
blocking the 1.9.0 release, and I don't like this either.
A possible (although quite radical) option would be cutting the --keep-going
feature from 1.9.x and redesigning it in 1.10. I believe that there are some
possibilities to explore — i.e., maybe, having a separate callback for error
reporting or introducing svn_repos_notify_func2_t that would be yielding errors
or would provide the necessary flexibility in another way.
Regards,
Evgeny Kotkov
Received on 2015-06-11 14:34:29 CEST