Hi,
the attached patch fixes both issue 4856 and the (different) bug
we were seeing in one of our own repositories.
The problem boils down to the fact that with `svn log --xml --verbose`
a log-item is opened every time the PATH_CHANGE_RECEIVER callback is
used. But if the revision is later declared "unimportant", the
corresponding log-item closure is never sent via the REVISION_RECEIVER
callback.
The patch fixes that by always using an intermediate callback routine
and recording the call of inner() in a new flag in
interesting_merge_baton_t. This flag is then used to decide if the
REVISION_RECEIVER callback has to be called to close the log-item.
With yet another flag in interesting_merge_baton_t record_inner_call()
could be merged into interesting_merge() if you prefer that.
Franz.
Index: subversion-1.14.x/subversion/libsvn_ra_serf/log.c
===================================================================
--- subversion-1.14.x/subversion/libsvn_ra_serf/log.c (revision 1921239)
+++ subversion-1.14.x/subversion/libsvn_ra_serf/log.c (working copy)
@@ -299,6 +299,7 @@
"subtractive-merge",
FALSE);
+ SVN_ERR_ASSERT(attrs != NULL);
rev_str = svn_hash_gets(attrs, "revision");
if (rev_str)
{
Index: subversion-1.14.x/subversion/libsvn_repos/log.c
===================================================================
--- subversion-1.14.x/subversion/libsvn_repos/log.c (revision 1921239)
+++ subversion-1.14.x/subversion/libsvn_repos/log.c (working copy)
@@ -1216,6 +1216,9 @@
/* Set to TRUE if we found it. */
svn_boolean_t found_rev_of_interest;
+ /* Set to TRUE if we called PATH_CHANGE_RECEIVER via INNER. */
+ svn_boolean_t inner_got_used;
+
/* We need to invoke this user-provided callback if not NULL. */
svn_repos_path_change_receiver_t inner;
void *inner_baton;
@@ -1236,7 +1239,10 @@
apr_hash_index_t *hi;
if (b->inner)
- SVN_ERR(b->inner(b->inner_baton, change, scratch_pool));
+ {
+ SVN_ERR(b->inner(b->inner_baton, change, scratch_pool));
+ b->inner_got_used = TRUE;
+ }
if (b->found_rev_of_interest)
return SVN_NO_ERROR;
@@ -1270,6 +1276,22 @@
return SVN_NO_ERROR;
}
+static svn_error_t *
+record_inner_call(void *baton,
+ svn_repos_path_change_t *change,
+ apr_pool_t *scratch_pool)
+{
+ interesting_merge_baton_t *b = baton;
+
+ if (b->inner)
+ {
+ SVN_ERR(b->inner(b->inner_baton, change, scratch_pool));
+ b->inner_got_used = TRUE;
+ }
+
+ return SVN_NO_ERROR;
+}
+
/* Send a log message for REV to the CALLBACKS.
FS is used with REV to fetch the interesting history information,
@@ -1310,6 +1332,7 @@
{
svn_repos_log_entry_t log_entry = { 0 };
log_callbacks_t my_callbacks = *callbacks;
+ apr_pool_t *scratch_pool;
interesting_merge_baton_t baton;
@@ -1323,6 +1346,7 @@
&& apr_hash_count(log_target_history_as_mergeinfo))
{
baton.found_rev_of_interest = FALSE;
+ baton.inner_got_used = FALSE;
baton.rev = rev;
baton.log_target_history_as_mergeinfo = log_target_history_as_mergeinfo;
baton.inner = callbacks->path_change_receiver;
@@ -1335,6 +1359,12 @@
else
{
baton.found_rev_of_interest = TRUE;
+ baton.inner_got_used = FALSE;
+ baton.inner = callbacks->path_change_receiver;
+ baton.inner_baton = callbacks->path_change_receiver_baton;
+ my_callbacks.path_change_receiver = record_inner_call;
+ my_callbacks.path_change_receiver_baton = &baton;
+ callbacks = &my_callbacks;
}
SVN_ERR(fill_log_entry(&log_entry, rev, fs, revprops, callbacks, pool));
@@ -1345,15 +1375,14 @@
revision. */
if (baton.found_rev_of_interest)
{
- apr_pool_t *scratch_pool;
-
/* Is REV a merged revision we've already sent? */
if (nested_merges && handling_merged_revision)
{
if (svn_bit_array__get(nested_merges, rev))
{
- /* We already sent REV. */
- return SVN_NO_ERROR;
+ if (!baton.inner_got_used)
+ /* We already sent REV and no log was streamed yet. */
+ return SVN_NO_ERROR;
}
else
{
@@ -1371,7 +1400,20 @@
&log_entry, scratch_pool));
svn_pool_destroy(scratch_pool);
}
+ else if (baton.inner_got_used)
+ {
+ svn_repos_log_entry_t empty_log_entry = { 0 };
+ /* Send the empty revision. */
+ empty_log_entry.revision = SVN_INVALID_REVNUM;
+ /* Pass a scratch pool to ensure no temporary state stored
+ by the receiver callback persists. */
+ scratch_pool = svn_pool_create(pool);
+ SVN_ERR(callbacks->revision_receiver(callbacks->revision_receiver_baton,
+ &empty_log_entry, scratch_pool));
+ svn_pool_destroy(scratch_pool);
+ }
+
return SVN_NO_ERROR;
}
@@ -1719,8 +1761,8 @@
int limit,
svn_boolean_t strict_node_history,
svn_boolean_t include_merged_revisions,
+ svn_boolean_t subtractive_merge,
svn_boolean_t handling_merged_revisions,
- svn_boolean_t subtractive_merge,
svn_boolean_t ignore_missing_locations,
const apr_array_header_t *revprops,
svn_boolean_t descending_order,
