[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: Tue, 30 Oct 2012 15:41:45 +0100

On Tue, Oct 30, 2012 at 07:22:31PM +0530, Prabhu Gnana Sundar wrote:
> Index: subversion/svnadmin/main.c
> ===================================================================
> --- subversion/svnadmin/main.c (revision 1402414)
> +++ subversion/svnadmin/main.c (working copy)

> @@ -738,6 +743,14 @@
> notify->warning_str));
> return;
>
> + case svn_repos_notify_failure:
> + svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
> + _("svnadmin: Error verifying revision %ld. "),
> + notify->revision));
> + svn_handle_error2(notify->err, stderr, FALSE /* non-fatal */,
> + ("svnadmin: "));

No brackets needed around "svnadmin: ".
It look as if you meant to write _("svnadmin: "), marking the string
for translation using _(). But the program name should not be translated.

> + return;
> +
> case svn_repos_notify_dump_rev_end:
> svn_error_clear(svn_stream_printf(feedback_stream, scratch_pool,
> _("* Dumped revision %ld.\n"),

On my system your patch produces the following output:

$ svnadmin verify --keep-going repos
svnadmin: Error verifying revision 0. subversion/libsvn_fs/fs-loader.c:500: (apr_err=160004)
subversion/libsvn_fs_fs/fs_fs.c:10167: (apr_err=160004)
subversion/libsvn_fs_fs/tree.c:608: (apr_err=160004)
subversion/libsvn_fs_fs/dag.c:611: (apr_err=160004)
subversion/libsvn_fs_fs/fs_fs.c:3024: (apr_err=160004)
subversion/libsvn_fs_fs/fs_fs.c:2775: (apr_err=160004)
svnadmin: E160004: Missing node-id in node-rev at r3 (offset 787)
* Verified revision 0.
* Verified revision 1.
* Verified revision 2.
svnadmin: Error verifying revision 3. subversion/libsvn_repos/dump.c:983: (apr_err=160004)
subversion/libsvn_fs/fs-loader.c:858: (apr_err=160004)
subversion/libsvn_fs_fs/tree.c:608: (apr_err=160004)
subversion/libsvn_fs_fs/dag.c:611: (apr_err=160004)
subversion/libsvn_fs_fs/fs_fs.c:3024: (apr_err=160004)
subversion/libsvn_fs_fs/fs_fs.c:2775: (apr_err=160004)
svnadmin: E160004: Missing node-id in node-rev at r3 (offset 787)
svnadmin: Error verifying revision 4. subversion/libsvn_delta/path_driver.c:263: (apr_err=140001)
subversion/libsvn_repos/replay.c:664: (apr_err=140001)
subversion/libsvn_delta/cancel.c:197: (apr_err=140001)
subversion/libsvn_repos/dump.c:829: (apr_err=140001)
subversion/libsvn_repos/dump.c:628: (apr_err=140001)
subversion/libsvn_subr/stream.c:143: (apr_err=140001)
subversion/libsvn_fs_fs/fs_fs.c:5058: (apr_err=140001)
subversion/libsvn_fs_fs/fs_fs.c:5033: (apr_err=140001)
subversion/libsvn_fs_fs/fs_fs.c:4886: (apr_err=140001)
subversion/libsvn_fs_fs/fs_fs.c:4849: (apr_err=140001)
subversion/libsvn_delta/svndiff.c:726: (apr_err=140001)
subversion/libsvn_delta/svndiff.c:563: (apr_err=140001)
subversion/libsvn_repos/dump.c:983: (apr_err=140001)
svnadmin: E140001: zlib (uncompress): corrupt data: Decompression of svndiff data failed
* Verified revision 5.
$

The first error seems to come from the fs-layer verification function.
This error also appears without your patch. Apparently fs-layer verification
finds corruption in r3. But fs-layer verification errors shouldn't be
associated with a particular revision in the output.

And I'd prefer to have each error message appear on a single line,
with prefixes separated by colons instead of full-stops.

So I'd expect the output to look as follows:

$ svnadmin verify --keep-going repos
subversion/libsvn_fs_fs/fs_fs.c:10167: (apr_err=160004)
subversion/libsvn_fs_fs/tree.c:608: (apr_err=160004)
subversion/libsvn_fs_fs/dag.c:611: (apr_err=160004)
subversion/libsvn_fs_fs/fs_fs.c:3024: (apr_err=160004)
subversion/libsvn_fs_fs/fs_fs.c:2775: (apr_err=160004)
svnadmin: E160004: Missing node-id in node-rev at r3 (offset 787)
* Verified revision 0.
* Verified revision 1.
* Verified revision 2.
subversion/libsvn_fs/fs-loader.c:858: (apr_err=160004)
subversion/libsvn_fs_fs/tree.c:608: (apr_err=160004)
subversion/libsvn_fs_fs/dag.c:611: (apr_err=160004)
subversion/libsvn_fs_fs/fs_fs.c:3024: (apr_err=160004)
subversion/libsvn_fs_fs/fs_fs.c:2775: (apr_err=160004)
svnadmin: Error verifying revision 3: E160004: Missing node-id in node-rev at r3 (offset 787)
subversion/libsvn_repos/replay.c:664: (apr_err=140001)
subversion/libsvn_delta/cancel.c:197: (apr_err=140001)
subversion/libsvn_repos/dump.c:829: (apr_err=140001)
subversion/libsvn_repos/dump.c:628: (apr_err=140001)
subversion/libsvn_subr/stream.c:143: (apr_err=140001)
subversion/libsvn_fs_fs/fs_fs.c:5058: (apr_err=140001)
subversion/libsvn_fs_fs/fs_fs.c:5033: (apr_err=140001)
subversion/libsvn_fs_fs/fs_fs.c:4886: (apr_err=140001)
subversion/libsvn_fs_fs/fs_fs.c:4849: (apr_err=140001)
subversion/libsvn_delta/svndiff.c:726: (apr_err=140001)
subversion/libsvn_delta/svndiff.c:563: (apr_err=140001)
subversion/libsvn_repos/dump.c:983: (apr_err=140001)
svnadmin: Error verifying revision 4: E140001: zlib (uncompress): corrupt data: Decompression of svndiff data failed
* Verified revision 5.
$

To get this kind of output, try modifying the error message prefix
passed to svn_handle_error2() instead of calling svn_stream_printf().

By default, the prefix is "svnadmin: ".
If notify->revision is not SVN_INVALID_REVNUM you can construct an error
message prefix with apr_psprintf(). In pseudo code:

  if (rev != INVALID_REVNUM)
    revstr = apr_psprintf(pool, _("Error verifying revision %ld: "), rev);
  else
    revstr = "";
  svn_handle_error2(..., apr_psprintf(pool, "svnadmin: %s", revstr));

> Index: subversion/libsvn_repos/dump.c
> ===================================================================
> --- subversion/libsvn_repos/dump.c (revision 1402414)
> +++ subversion/libsvn_repos/dump.c (working copy)

> @@ -1397,8 +1399,19 @@
> end_rev, youngest);
>
> /* Verify global/auxiliary data and backend-specific data first. */
> - SVN_ERR(svn_fs_verify(svn_fs_path(fs, pool), cancel_func, cancel_baton,
> - start_rev, end_rev, pool));
> + err = svn_fs_verify(svn_fs_path(fs, pool), cancel_func, cancel_baton,
> + start_rev, end_rev, pool);
> + if (err && keep_going)
> + {
> + svn_repos_notify_t *notify_failure;
> + notify_failure = svn_repos_notify_create(svn_repos_notify_failure,
> + iterpool);
> + notify_failure->err = err;

You should set notify_failure->revision to SVN_INVALID_REVNUM here to
indicate that this failure isn't associated with a particular revision.

> + notify_func(notify_baton, notify_failure, iterpool);
> + svn_error_clear(err);
> + }
> + else
> + return svn_error_trace(err);
>
> /* Create a notify object that we can reuse within the loop. */
> if (notify_func)
Received on 2012-10-30 15:42:32 CET

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