[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: Stefan Sperling <stsp_at_elego.de>
Date: Mon, 5 Nov 2012 12:37:19 +0100

On Mon, Nov 05, 2012 at 02:42:29PM +0530, Prabhu Gnana Sundar wrote:
> On 11/04/2012 06:14 AM, Daniel Shahaf wrote:
> >
> >>Index: subversion/svnadmin/main.c
> >>===================================================================
> >>--- subversion/svnadmin/main.c (revision 1402414)
> >>+++ subversion/svnadmin/main.c (working copy)
> >>@@ -738,6 +743,16 @@
> >> notify->warning_str));
> >> return;
> >>
> >>+ case svn_repos_notify_failure:
> >>+ if (notify->revision != SVN_INVALID_REVNUM)
> >>+ svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
> >>+ _("svnadmin: E%d: Error verifying revision %ld\n"),
> >-1 (layering violation). Use svn_error_quick_wrap().
> >
> >While you're at it, wrap to 80 columns.
>
> Thanks Daniel,
>
> Now I have made something like
>
> <snip>
>
> * 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.
> svnadmin: E165005: Repository 'testrepo' failed to verify.
>
> </snip>
>
>
> I have used "* Error verifying revision x" instead of "svnadmin:
> Error verifying revision x" because when a revision is successfully
> verified we just say it as "* Verified revision x". I feel that this
> is a bit more consistent.

I'm fine with this output.

> So that makes the code to look like,
>
> <snip>
>
> case svn_repos_notify_failure:
> if (notify->revision != SVN_INVALID_REVNUM)
> svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
> _("* Error verifying
> revision %ld.\n"),
> notify->revision));
> svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */,
> "svnadmin: ");
 
Please always align function parameters on consecutive lines along
the column of the opening brace, like this:

       svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */,
                         "svnadmin: ");
                         ^ this column
                                     ^ not this one!

Can you please send a complete patch we can try instead of
pasting code snippets?

> </snip>
>
> I guess, this also solves the layer violation problem. Correct me if
> am wrong.

Daniel suggested to use svn_error_quick_wrap() which you aren't using.
So I doubt you did what he had in mind. Honestly, I don't really understand
what he wants you to do either. Daniel, can you take the time to explain
a bit more clearly what you mean, please?

Have you checked whether you patch breaks 'make check'? You may need
to tweak expected output in some tests to prevent them from failing.
Received on 2012-11-05 12:38:08 CET

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