You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <da...@elego.de> on 2011/08/28 15:08:49 UTC
[PATCH] Don't repeatedly notify the same warning
[[[
Make 'svnadmin dump' print the following warning:
WARNING 0x0001: Mergeinfo referencing revision(s) prior to the
oldest dumped revision (4). Loading this dump may result in invalid
mergeinfo.
only once per dump operation, rather once per affected node. The
repetition is deemed more noisy than useful. (There is already
a warning at the end pointing the existence of warnings throughout
the dump.)
* subversion/libsvn_repos/dump.c:
(TBD):
Store the 'saw it' bit, not per-revision (in the edit baton) but per
dump operation (in stack storage pointed to by the edit baton).
(dump_node):
Check the bit before generating the notification/warning.
]]]
[[[
Index: subversion/libsvn_repos/dump.c
===================================================================
--- subversion/libsvn_repos/dump.c (revision 1162490)
+++ subversion/libsvn_repos/dump.c (working copy)
@@ -116,11 +116,11 @@ struct edit_baton
/* Set to true if any references to revisions older than
OLDEST_DUMPED_REV were found in the dumpstream. */
- svn_boolean_t found_old_reference;
+ svn_boolean_t *found_old_reference;
/* Set to true if any mergeinfo was dumped which contains revisions
older than OLDEST_DUMPED_REV. */
- svn_boolean_t found_old_mergeinfo;
+ svn_boolean_t *found_old_mergeinfo;
/* reusable buffer for writing file contents */
char buffer[SVN__STREAM_CHUNK_SIZE];
@@ -389,7 +389,7 @@ dump_node(struct edit_baton *eb,
" into an empty repository"
" will fail."),
cmp_rev, eb->oldest_dumped_rev);
- eb->found_old_reference = TRUE;
+ *eb->found_old_reference = TRUE;
eb->notify_func(eb->notify_baton, notify, pool);
}
@@ -480,7 +480,7 @@ dump_node(struct edit_baton *eb,
&old_mergeinfo, mergeinfo,
eb->oldest_dumped_rev - 1, 0,
TRUE, pool, pool));
- if (apr_hash_count(old_mergeinfo))
+ if (! *eb->found_old_mergeinfo && apr_hash_count(old_mergeinfo))
{
svn_repos_notify_t *notify =
svn_repos_notify_create(svn_repos_notify_warning, pool);
@@ -494,7 +494,7 @@ dump_node(struct edit_baton *eb,
"mergeinfo."),
eb->oldest_dumped_rev);
- eb->found_old_mergeinfo = TRUE;
+ *eb->found_old_mergeinfo = TRUE;
eb->notify_func(eb->notify_baton, notify, pool);
}
}
@@ -863,6 +863,8 @@ get_dump_editor(const svn_delta_editor_t **editor,
svn_revnum_t oldest_dumped_rev,
svn_boolean_t use_deltas,
svn_boolean_t verify,
+ svn_boolean_t *found_old_reference,
+ svn_boolean_t *found_old_mergeinfo,
apr_pool_t *pool)
{
/* Allocate an edit baton to be stored in every directory baton.
@@ -882,6 +884,8 @@ get_dump_editor(const svn_delta_editor_t **editor,
eb->current_rev = to_rev;
eb->use_deltas = use_deltas;
eb->verify = verify;
+ eb->found_old_reference = found_old_reference;
+ eb->found_old_mergeinfo = found_old_mergeinfo;
/* Set up the editor. */
dump_editor->open_root = open_root;
@@ -1094,7 +1098,9 @@ svn_repos_dump_fs3(svn_repos_t *repos,
use_deltas_for_rev = use_deltas && (incremental || i != start_rev);
SVN_ERR(get_dump_editor(&dump_editor, &dump_edit_baton, fs, to_rev,
"", stream, notify_func, notify_baton,
- start_rev, use_deltas_for_rev, FALSE, subpool));
+ start_rev, use_deltas_for_rev, FALSE,
+ &found_old_reference, &found_old_mergeinfo,
+ subpool));
/* Drive the editor in one way or another. */
SVN_ERR(svn_fs_revision_root(&to_root, fs, to_rev, subpool));
@@ -1310,6 +1316,7 @@ svn_repos_verify_fs2(svn_repos_t *repos,
notify_func, notify_baton,
start_rev,
FALSE, TRUE, /* use_deltas, verify */
+ NULL, NULL,
iterpool));
dump_editor->close_directory = verify_close_directory;
SVN_ERR(svn_delta_get_cancellation_editor(cancel_func, cancel_baton,
]]]
Re: [PATCH] Don't repeatedly notify the same warning
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 08/30/2011 03:26 PM, C. Michael Pilato wrote:
> On 08/30/2011 10:11 AM, Daniel Shahaf wrote:
>> C. Michael Pilato wrote on Tue, Aug 30, 2011 at 10:01:26 -0400:
>>> Would it make more sense to just replace the warning at the end with a
>>> single instance of each of the various mid-stream warnings? Or are all such
>>> warnings not quite as redundant as this one?
>>
>> But then people have to finish the dump (successfully! ie, without
>> hitting any corruption or other error partway through) before they see
>> that "FYI, the data you've just dumped isn't self-contained" warning.
>>
>> Reporting error conditions as early as possible seems preferable.
>
> Yep, makes perfect sense. +1 to move forward per your plan.
Actually, I'll make one more suggestion: change the singularly printed
warning message to include a phrase such as "(Future notifications of this
same warning will be suppressed.)" Just so seasoned Subversionites (used to
seeing tons of these messages) don't make false assumptions about how
prevalent the problem is/isn't now.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: [PATCH] Don't repeatedly notify the same warning
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 08/30/2011 10:11 AM, Daniel Shahaf wrote:
> C. Michael Pilato wrote on Tue, Aug 30, 2011 at 10:01:26 -0400:
>> Would it make more sense to just replace the warning at the end with a
>> single instance of each of the various mid-stream warnings? Or are all such
>> warnings not quite as redundant as this one?
>
> But then people have to finish the dump (successfully! ie, without
> hitting any corruption or other error partway through) before they see
> that "FYI, the data you've just dumped isn't self-contained" warning.
>
> Reporting error conditions as early as possible seems preferable.
Yep, makes perfect sense. +1 to move forward per your plan.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: [PATCH] Don't repeatedly notify the same warning
Posted by Daniel Shahaf <da...@elego.de>.
C. Michael Pilato wrote on Tue, Aug 30, 2011 at 10:01:26 -0400:
> On 08/28/2011 09:08 AM, Daniel Shahaf wrote:
> > [[[
> > Make 'svnadmin dump' print the following warning:
> >
> > WARNING 0x0001: Mergeinfo referencing revision(s) prior to the
> > oldest dumped revision (4). Loading this dump may result in invalid
> > mergeinfo.
> >
> > only once per dump operation, rather once per affected node. The
> > repetition is deemed more noisy than useful. (There is already
> > a warning at the end pointing the existence of warnings throughout
> > the dump.)
> >
> > * subversion/libsvn_repos/dump.c:
> > (TBD):
> > Store the 'saw it' bit, not per-revision (in the edit baton) but per
> > dump operation (in stack storage pointed to by the edit baton).
> > (dump_node):
> > Check the bit before generating the notification/warning.
> > ]]]
>
> Would it make more sense to just replace the warning at the end with a
> single instance of each of the various mid-stream warnings? Or are all such
> warnings not quite as redundant as this one?
But then people have to finish the dump (successfully! ie, without
hitting any corruption or other error partway through) before they see
that "FYI, the data you've just dumped isn't self-contained" warning.
Reporting error conditions as early as possible seems preferable.
Re: [PATCH] Don't repeatedly notify the same warning
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 08/28/2011 09:08 AM, Daniel Shahaf wrote:
> [[[
> Make 'svnadmin dump' print the following warning:
>
> WARNING 0x0001: Mergeinfo referencing revision(s) prior to the
> oldest dumped revision (4). Loading this dump may result in invalid
> mergeinfo.
>
> only once per dump operation, rather once per affected node. The
> repetition is deemed more noisy than useful. (There is already
> a warning at the end pointing the existence of warnings throughout
> the dump.)
>
> * subversion/libsvn_repos/dump.c:
> (TBD):
> Store the 'saw it' bit, not per-revision (in the edit baton) but per
> dump operation (in stack storage pointed to by the edit baton).
> (dump_node):
> Check the bit before generating the notification/warning.
> ]]]
Would it make more sense to just replace the warning at the end with a
single instance of each of the various mid-stream warnings? Or are all such
warnings not quite as redundant as this one?
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand