You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@subversion.apache.org by Franz Sirl <Fr...@lauterbach.com> on 2024/10/14 11:07:21 UTC
[PATCH] Fix missing log-item close tag in on-the-wire XML
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.
Re: [PATCH] Fix missing log-item close tag in on-the-wire XML
Posted by Franz Sirl <Fr...@lauterbach.com>.
Am 2024-10-15 um 12:23 schrieb Daniel Sahlberg:
> Den mån 14 okt. 2024 kl 13:07 skrev Franz Sirl <Franz.Sirl-
> kernel@lauterbach.com <ma...@lauterbach.com>>:
>
> 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.
>
>
> Thank you for the patch!
>
> In the discussion about issue 4177, Daniel Shahaf mentioned that there
> was an inconsistency about the number of times the receiver callback was
> called, so it seems likely you have found that issue.
>
> I'm going to test this further when time permits, hopefully this week.
Hello Daniel,
in the meantime I noticed that the previous patch streams out a lot of
duplicated revisions. I've come up with v2 of the patch, which probably
has a bit of unnecessary code because I'm not so sure about when and how
this code recurses.
The patch avoids the duplicates, but it also triggers a strange behavior
with --limit. If I add for example a `--limit 1` to the log command, the
XML generated by file: and http: start to differ a lot on the repository
I used for testing. I don't know what to make of that...
BTW, the SVN_ERR_ASSERT() in subversion/libsvn_ra_serf/log.c was just
helpful to get a better error report. It's not necessary for the
functionality of the patch.
Hope this helps,
Franz
Re: [PATCH] Fix missing log-item close tag in on-the-wire XML
Posted by Daniel Sahlberg <da...@gmail.com>.
Den mån 14 okt. 2024 kl 13:07 skrev Franz Sirl <
Franz.Sirl-kernel@lauterbach.com>:
> 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.
>
Thank you for the patch!
In the discussion about issue 4177, Daniel Shahaf mentioned that there was
an inconsistency about the number of times the receiver callback was
called, so it seems likely you have found that issue.
I'm going to test this further when time permits, hopefully this week.
Kind regards,
Daniel