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

Re: [PATCH] Implement svnadmin verify --force

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 1 Nov 2012 14:17:01 +0000 (GMT)

Stefan Sperling wrote:

> On Thu, Nov 01, 2012 at 02:21:48PM +0530, Prabhu Gnana Sundar wrote:
>> On 10/31/2012 11:31 PM, Julian Foad wrote:
>>> Daniel Shahaf wrote:
>>>> The code will print
>>>>   svnadmin: E160004: Corrupt filesystem
>>>> or
>>>>   svnadmin: Error verifying revision 42: E160004: Corrupt filesystem
[...]
>>>> But the code moves the E160004 away from its current location
>>>> immediately after the "svnadmin:" prefix.  I am not sure I like that;
>>>> is there a good reason not to have the message be of the form
>>>>     svnadmin: E160004: %s
>>>> in the interest of parseability?
>>>
>>> I agree that would be better: the prefix should remain just
>>> "svnadmin: " and the error message should be adjusted instead.
>>
>> Does this look fine ? I personally feel this is easily readable.
>>
>> * Verified revision 0.
>> * Verified revision 1.
>> * Verified revision 2.
>> Error verifying revision 3. svnadmin: E160004: Missing node-id in node-rev
> at r3 (offset 787)
>> Error verifying revision 4. svnadmin: E140001: zlib (uncompress): corrupt
> data: Decompression of svndiff data failed
>> * Verified revision 5.

That's easily readable, but I don't like it: it's a funny mixture of styles.  We should choose either "notification" style (that is, messages that are not error messages), such as

* Verified revision 0.
* Verified revision 1.
* Verified revision 2.
* Error verifying revision 3: Missing node-id in node-rev at r3 (offset 787)
* Error verifying revision 4: zlib (uncompress): corrupt data: Decompression of svndiff data failed
* Verified revision 5.

or "error messages" style, in which case the messages should be formatted like all svn err msgs, for example:

* Verified revision 0.
* Verified revision 1.
* Verified revision 2.
svnadmin: E199999: Error verifying revision 3
svnadmin: E160004: Missing node-id in node-rev at r3 (offset 787)
svnadmin: E199999: Error verifying revision 4
svnadmin: E140001: zlib (uncompress): corrupt data: Decompression of svndiff data failed
* Verified revision 5.

But the output-style design here is linked to the question of what we're going to print at the end of the verification.  Prabhu, what's the plan for when the command exits?  Print a summary or an error message?  Exit with non-zero return code (I hope)?

> The lines should start with "svnamdin: ".

The more I look at it, the more I think it makes sense to use notification-style output, and then exit with an error (a standard svn-style error message and non-zero exit code) at the end.

> I believe what Julian was suggesting is to stop doing the apr_psprintf()
> dance I suggested, and make libsvn_repos return a better error message
> instead which includes the revision number. Is that right, Julian?

(Well, that's one way of implementing the behaviour that I was suggesting.)

- Julian
Received on 2012-11-01 15:17:39 CET

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