On Tue, Oct 1, 2024 at 10:03 AM Franz Sirl
<franz.sirl-ker...@lauterbach.com> wrote:
>
> Hi,
>
> recently this svn log command started to fail like that:
>
> ```
> > svn log --xml --verbose --search @ --use-merge-history --incremental
> --revision 172342:{2024-09-30}
> http://svn/svn/project/branches/cd/2024_09 >/dev/null
> svn: E175009: The XML response contains invalid XML
> svn: E130003: Malformed XML: mismatched tag at line 139122
> ```
>
> Removing the --verbose lets the command succeed, so it seems related to
> the sending of the changed pathes. Also running the command with a
> file:// URL directly on the server works fine.
>
> Server and client are 1.14.3+issue4711-fix (BTW, when will there be an
> official release with the fix for issue 4711?) running on x64-linux.
> Actually the failure mode looks quite similar to issue 4711, but this
> time the problem happens on the server side.
I thought I remembered fix(es) for unbalanced XML closing tags being
implemented, committed, and backported to the 1.14.x branch!! But
looking through the history, I only see two regression tests that were
committed and both are @XFail() (meaning, expected FAILs).
Those tests were committed to trunk in r1877310 and r1897133.
Now, it seems there are two issues in the issue tracker related to
this: Issue 4711 [1] (which you referenced) and Issue 4856 [2]. I'm
adding a note to both...
> Looking at the transferred data with wireshark shows this at the end:
> ```
> <S:log-item>
> <S:modified-path node-kind="file" text-mods="true"
> prop-mods="false">...</S:modified-path>
> ... (>40000 lines of modified-path/added-path redacted)
> <S:modified-path node-kind="file" text-mods="true"
> prop-mods="false">...</S:modified-path>
> </S:log-report>
> ```
>
> So the closing tag `</S:log-item>` is missing. This closing tag is only
> sent by log_revision_receiver() in subversion/mod_dav_svn/reports/log.c
> and is used as a callback in
> subversion/libsvn_repos/log.c.
> Looking at the code the only failure mode I could imagine so far is that
> if the last revision processed for this transaction ends up with
> `baton.found_rev_of_interest = FALSE`.
> In this case send_log() doesn't call callbacks->revision_receiver() and
> so the log-item tag never gets closed?
Thank you for the analysis.
Would you say that the Issue 4711 patch in the issue tracker [3]
improves the situation, even if it's still missing something?
If we suspect that baton.found_rev_of_interest being FALSE for the
last processed revision causes the missing tag, we'll need to come up
with a reproduction recipe that triggers that. Could you rerun the
command but with a revision range where the last revision *is* of
interest, and tell us if the XML is formed correctly?
> Hope this helps to track down the bug,
> Franz
>
> PS: While investigating that I found a small cosmetic bug in the
> prototype (fortunately the code follows the definition) for do_logs() in
> subversion/libsvn_repos/log.c:
> ```
> Index: subversion/libsvn_repos/log.c
> ===================================================================
> --- subversion/libsvn_repos/log.c (revision 1921065)
> +++ subversion/libsvn_repos/log.c (working copy)
> @@ -1719,8 +1719,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,
> ```
Good catch! The 11th and 12th arguments are swapped in the forward
declaration. I'll commit this fix in a bit, but before I do that, I
want to go over the call sites and verify they do not have the
arguments swapped due to copying-and-pasting of the (wrong) forward
declaration. Hopefully as you say the code follows the definition (and
not the forward declaration).
Thanks again,
Nathan
