Branko Čibej <br...@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
