Prabhu Gnana Sundar wrote on 14 December 2012:

> On 12/10/2012 08:15 PM, Julian Foad wrote:
>>  Prabhu Gnana Sundar
>> 
>>>  This patch is a follow up of the long discussion we had in thread [1]. 
>>
>>  Please will you summarize the issues that were raised in the previous 
>> discussion and how you have chosen to proceed.  I'm thinking in
>> particular of the discussion about how the output is notified -- how
>> to display each error, on what output stream, and what to print at the
>> end of the whole verification and what exit code to return.  There may
>> have been other issues too.
> 
> Thanks Julian,
> 
> Before this patch:
> -----------------------
> 
> When we run svnadmin verify on a repo, the verification process would stop 
> once 
> a failure/corruption is found. The error is notified via the error stream and 
> the exit code would be 1.
> Upon successful verification of each revision of the repo "Verified 
> revision r" is also notified via the stderr stream and the exit code is 0.
> 
> 
> Eg:
> 
> $ svnadmin verify 
> subversion/tests/cmdline/svn-test-work/repositories/svnadmin_tests-31
> svnadmin: E200004: Could not convert '' into a number
> 
> 
> After this patch:
> ---------------------
> 
> When we run svnadmin verify on a repo, the verification process would stop 
> once 
> a failure/corruption is found. The error is notified via the error stream and 
> the exit code would be 1. The error stream would end as "svnadmin: E165005: 
> Repository 'reponame' failed to verify".
> The behaviour is unchanged for successful verification.
> 
> When we run svnadmin verify --keep-going, the verification process continues 
> after detecting any failure/corruption. The failure is reported for whichever 
> revision it is found. The error message is notified via the stderr stream. 
> The 
> successful verification of revisions will also be reported via the stderr 
> stream 
> (no change in behaviour of notifying success). The repo verification failure 
> will be notified at the end as "svnadmin: E165005: Repository 
> 'reponame' failed to verify". The exit code is 1.
> 
> Eg:
> 
> $ svnadmin verify 
> subversion/tests/cmdline/svn-test-work/repositories/svnadmin_tests-31
> svnadmin: E200004: Could not convert '' into a number
> svnadmin: E165005: Repository 
> 'subversion/tests/cmdline/svn-test-work/repositories/svnadmin_tests-31' 
> failed to verify
> 
> 
> $ svnadmin verify 
> subversion/tests/cmdline/svn-test-work/repositories/svnadmin_tests-31 
> --keep-going
> * Verified revision 0.
> * Verified revision 1.
> * Verified revision 2.
> * Verified revision 3.
> * Verified revision 4.
> * Verified revision 5.
> * Error verifying revision 6.
> svnadmin: E200004: Could not convert '' into a number
> svnadmin: E165005: Repository 
> 'subversion/tests/cmdline/svn-test-work/repositories/svnadmin_tests-31' 
> failed to verify

Thanks, Prabhu, for explaining this.

That sounds good.  I was surprised to learn that the progress notifications ("* 
Verified...") were already going to stderr.

(I was meaning to review the patch before sending this reply, but I have not 
got around to it yet, and now I see you have just sent a new version in 
response to Daniel's comments.)

> Earlier, Mike suggested to notify the errors in both stdout and stderr stream 
> so 
> that they would be independently valuable to the user.
> But I have not notified the errors on both the streams, rather only via the 
> stderr stream, because the error notification would appear twice for each 
> failure when the verification is run from the command line. Thoughts ?

The way I understood C-Mike's suggestion was that all the "* Verified revision 
R" and "* Error verifying revision R" messages (and probably the final summary 
line too) would go to stdout, whereas all the "svnadmin: Exxxxxx: ..." messages 
would go to stderr.  Therefore the output would appear just like you describe 
above -- except perhaps within a given revision the error messages might come 
before the "Failed to verify revision" message instead of after it.

However, that suggestion is something we can consider as a possible future 
change -- it seems to me that your solution is fine.

- Julian

Reply via email to