Den tors 19 aug. 2021 kl 14:51 skrev Daniel Shahaf <d...@daniel.shahaf.name>:
> Daniel Sahlberg wrote on Wed, Aug 18, 2021 at 07:54:47 +0200:
> > Den tis 17 aug. 2021 kl 05:36 skrev Daniel Shahaf <
> d...@daniel.shahaf.name>:
> > > subversion/svn/schema/log.rnc doesn't mention reverse-merge at all.
> > > (I was looking to see whether the attribute was declared mandatory or
> not.)
> > >
> >
> > So, let's add it. Since it is not present in the case where there are no
> > merges, it should be optional. See if I got this right (first time
> touching
> > XML RELAX NG schemas):
> > [[[
> > Index: subversion/svn/schema/log.rnc
> > ===================================================================
> > --- subversion/svn/schema/log.rnc (revision 1892122)
> > +++ subversion/svn/schema/log.rnc (working copy)
> > @@ -28,7 +28,8 @@
> > logentry =
> > element logentry { attlist.logentry, author?, date?, paths?, msg?,
> revprops?, logentry* }
> > attlist.logentry &=
> > - attribute revision { revnum.type }
> > + attribute revision { revnum.type },
> > + attribute reverse-merge { "true" | "false" }?
>
> I don't know. I've not worked with .rnc schemata before. Do you have
> a more specific question about this change? Any particular aspect of it
> you aren't sure about?
>
No, not really. FWIW, I've tested log.rnc using trang and it seems to
validate and also produces a reasonable xsd schema.
I have committed as r1892455.
>
> > ## Changed paths information.
> > paths = element paths { path+ }
> > ]]]
> >
> >
> > >
> > > Could adding the attribute to r5 break anything?
> > >
> >
> > The attribute is already known (to those observing the existing output of
> > -g). Most XML parsers (that I have worked with) ignore unknown or
> > unexpected attributes so I think it wouldn't be a problem - in any case
> > they will have trouble with r4 (see my example two mails upthread) where
> it
> > WILL be present.
> >
> > I also don't think it will be a problem NOT to have it, since it doesn't
> > exist in non -g outputs.
> >
> > The larger question is "do we want it?". In other words: Is it a property
> > of r5 (which was filtered out, so it should be filtered out as well) or
> is
> > it a property necessary to reconstruct the path to r4.
>
> I think we should include it, because:
>
> - The API provides it. The CLI be to the API as the plain log output is
> to the XML output.
>
> - It adds information: absence does not imply the value would be "true",
> nor that it would be "false".
>
> > > > We could change the merge_stack to store this as well if needed, but
> I
> > > > don't have a clear picture about the merge/reverse merge flow.
> > >
> > > That's a bit of a large question. Handwavingly, merging rN from /foo
> to
> > > /bar "adds some diff" to /bar and records that in svn:mergeinfo;
> > > thereafter, reverse-merging the same revision from the same path will
> > > reverse both of these changes. Note: if the reverse merge was
> committed
> > > as rM, _that_ revision can be (forward) merged to a third branch. That
> > > commit would then add /bar:M and remove /foo:N from the third branch's
> > > mergeinfo.
> > >
> >
> > Out of time right now but I intend to come back to this and try to
> > construct a tree with this sequence.
>
> *nod*
>
When checking this closer, I agree with danielsh's conclusion in a separate
message (at 15:42 UTC) that the reverse-merge is a property of the parent
revision (r7 below, r6 in the previous example) and that it is necessary to
have it to properly reconstruct the path to r4. Thus we should not elide it
(even if we elide all the other properties of revisions that doesn't match
the search pattern).
Current output. Again XML indented for readability. It is the same WC as
before, but with an additional revision 7 where I reverse merge what was
merged in r6:
[[[
dsahlberg@daniel-2020:~/temp/wc/c$ ~/svn_trunk_4711/subversion/svn/svn log
-g .
------------------------------------------------------------------------
r7 | dsahlberg | 2021-08-19 22:08:21 +0200 (Thu, 19 Aug 2021) | 1 line
reverse merge
------------------------------------------------------------------------
r5 | dsahlberg | 2021-08-19 21:58:29 +0200 (Thu, 19 Aug 2021) | 1 line
Reverse merged via: r7
merge a to b
------------------------------------------------------------------------
r4 | dsahlberg | 2021-08-19 21:58:28 +0200 (Thu, 19 Aug 2021) | 1 line
Merged via: r7, r5
]]]
As can be seen here, it is not visible in the plain text output that r7 was
actually a reverse merge.
[[[
add a/foo
------------------------------------------------------------------------
r6 | dsahlberg | 2021-08-19 21:58:29 +0200 (Thu, 19 Aug 2021) | 1 line
merge b to c
------------------------------------------------------------------------
r3 | dsahlberg | 2021-08-19 21:58:28 +0200 (Thu, 19 Aug 2021) | 1 line
copy b to c
------------------------------------------------------------------------
r2 | dsahlberg | 2021-08-19 21:58:28 +0200 (Thu, 19 Aug 2021) | 1 line
copy a to b
------------------------------------------------------------------------
r1 | dsahlberg | 2021-08-19 21:58:28 +0200 (Thu, 19 Aug 2021) | 1 line
add a
------------------------------------------------------------------------
dsahlberg@daniel-2020:~/temp/wc/c$ svn log -g --xml .
<?xml version="1.0" encoding="UTF-8"?>
<log>
<logentry revision="7">
<author>dsahlberg</author>
<date>2021-08-19T20:08:21.276818Z</date>
<msg>reverse merge</msg>
<logentry reverse-merge="true" revision="5">
<author>dsahlberg</author>
<date>2021-08-19T19:58:29.079067Z</date>
<msg>merge a to b</msg>
<logentry reverse-merge="false" revision="4">
<author>dsahlberg</author>
<date>2021-08-19T19:58:28.948418Z</date>
<msg>add a/foo</msg>
</logentry>
</logentry>
</logentry>
<logentry revision="6">
<author>dsahlberg</author>
<date>2021-08-19T19:58:29.210735Z</date>
<msg>merge b to c</msg>
</logentry>
<logentry revision="3">
<author>dsahlberg</author>
<date>2021-08-19T19:58:28.845491Z</date>
<msg>copy b to c</msg>
</logentry>
<logentry revision="2">
<author>dsahlberg</author>
<date>2021-08-19T19:58:28.800706Z</date>
<msg>copy a to b</msg>
</logentry>
<logentry revision="1">
<author>dsahlberg</author>
<date>2021-08-19T19:58:28.742409Z</date>
<msg>add a</msg>
</logentry>
</log>
dsahlberg@daniel-2020:~/temp/wc/c$ ~/svn_trunk_4711/subversion/svn/svn log
-g --search foo .
------------------------------------------------------------------------
r4 | dsahlberg | 2021-08-19 21:58:28 +0200 (Thu, 19 Aug 2021) | 1 line
Merged via: r7, r5
add a/foo
------------------------------------------------------------------------
dsahlberg@daniel-2020:~/temp/wc/c$ ~/svn_trunk_4711/subversion/svn/svn log
-g --search foo --xml .
<?xml version="1.0" encoding="UTF-8"?>
<log>
<logentry revision="7">
<logentry reverse-merge="true" revision="5">
<logentry reverse-merge="false" revision="4">
<author>dsahlberg</author>
<date>2021-08-19T19:58:28.948418Z</date>
<msg>add a/foo</msg>
</logentry>
</logentry>
</logentry>
</log>
dsahlberg@daniel-2020:~/temp/wc/c$
]]]
Current patch:
[[[
Index: subversion/svn/log-cmd.c
===================================================================
--- subversion/svn/log-cmd.c (revision 1892122)
+++ subversion/svn/log-cmd.c (working copy)
@@ -45,6 +45,12 @@
#include "svn_private_config.h"
+typedef struct merge_stack {
+ svn_revnum_t rev;
+ svn_boolean_t emitted;
+ svn_boolean_t subtractive_merge;
+} merge_stack_t;
+
/*** Code. ***/
@@ -355,10 +361,15 @@
{
if (log_entry->has_children)
{
+ merge_stack_t ms;
+
if (! lb->merge_stack)
- lb->merge_stack = apr_array_make(lb->pool, 1,
sizeof(svn_revnum_t));
+ lb->merge_stack = apr_array_make(lb->pool, 1,
sizeof(merge_stack_t));
- APR_ARRAY_PUSH(lb->merge_stack, svn_revnum_t) =
log_entry->revision;
+ ms.rev = log_entry->revision;
+ ms.emitted = FALSE;
+ ms.subtractive_merge = log_entry->subtractive_merge;
+ APR_ARRAY_PUSH(lb->merge_stack, merge_stack_t) = ms;
}
return SVN_NO_ERROR;
@@ -435,10 +446,10 @@
iterpool = svn_pool_create(pool);
for (i = 0; i < lb->merge_stack->nelts; i++)
{
- svn_revnum_t rev = APR_ARRAY_IDX(lb->merge_stack, i,
svn_revnum_t);
+ merge_stack_t ms = APR_ARRAY_IDX(lb->merge_stack, i,
merge_stack_t);
svn_pool_clear(iterpool);
- SVN_ERR(svn_cmdline_printf(iterpool, " r%ld%c", rev,
+ SVN_ERR(svn_cmdline_printf(iterpool, " r%ld%c", ms.rev,
i == lb->merge_stack->nelts - 1 ?
'\n' :
','));
}
@@ -475,10 +486,15 @@
if (log_entry->has_children)
{
+ merge_stack_t ms;
+
if (! lb->merge_stack)
- lb->merge_stack = apr_array_make(lb->pool, 1,
sizeof(svn_revnum_t));
+ lb->merge_stack = apr_array_make(lb->pool, 1,
sizeof(merge_stack_t));
- APR_ARRAY_PUSH(lb->merge_stack, svn_revnum_t) = log_entry->revision;
+ ms.rev = log_entry->revision;
+ ms.emitted = FALSE;
+ ms.subtractive_merge = log_entry->subtractive_merge;
+ APR_ARRAY_PUSH(lb->merge_stack, merge_stack_t) = ms;
}
return SVN_NO_ERROR;
@@ -533,6 +549,7 @@
const char *author;
const char *date;
const char *message;
+ int i;
if (lb->ctx->cancel_func)
SVN_ERR(lb->ctx->cancel_func(lb->ctx->cancel_baton));
@@ -544,11 +561,15 @@
if (! SVN_IS_VALID_REVNUM(log_entry->revision))
{
- svn_xml_make_close_tag(&sb, pool, "logentry");
- SVN_ERR(svn_cl__error_checked_fputs(sb->data, stdout));
if (lb->merge_stack)
- apr_array_pop(lb->merge_stack);
-
+ {
+ if (lb->merge_stack->nelts > 0 &&
(APR_ARRAY_IDX(lb->merge_stack, lb->merge_stack->nelts-1,
merge_stack_t).emitted))
+ {
+ svn_xml_make_close_tag(&sb, pool, "logentry");
+ SVN_ERR(svn_cl__error_checked_fputs(sb->data, stdout));
+ }
+ apr_array_pop(lb->merge_stack);
+ }
return SVN_NO_ERROR;
}
@@ -559,15 +580,44 @@
{
if (log_entry->has_children)
{
+ merge_stack_t ms;
if (! lb->merge_stack)
- lb->merge_stack = apr_array_make(lb->pool, 1,
sizeof(svn_revnum_t));
+ lb->merge_stack = apr_array_make(lb->pool, 1,
sizeof(merge_stack_t));
- APR_ARRAY_PUSH(lb->merge_stack, svn_revnum_t) =
log_entry->revision;
+ ms.rev = log_entry->revision;
+ ms.emitted = FALSE;
+ ms.subtractive_merge = log_entry->subtractive_merge;
+ APR_ARRAY_PUSH(lb->merge_stack, merge_stack_t) = ms;
}
return SVN_NO_ERROR;
}
-
+ else if (lb->search_patterns && lb->merge_stack)
+ {
+ /* match_search_patterns returned true */
+ for (i = 0; i < lb->merge_stack->nelts; i++)
+ {
+ merge_stack_t ms = APR_ARRAY_IDX(lb->merge_stack, i,
merge_stack_t);
+ if (!ms.emitted)
+ {
+ revstr = apr_psprintf(pool, "%ld", ms.rev);
+ if (i == 0)
+ {
+ svn_xml_make_open_tag(&sb, pool, svn_xml_normal,
"logentry",
+ "revision", revstr, SVN_VA_NULL);
+ }
+ else
+ {
+ svn_xml_make_open_tag(&sb, pool, svn_xml_normal,
"logentry",
+ "revision", revstr, "reverse-merge",
+ ms.subtractive_merge ? "true" :
"false",
+ SVN_VA_NULL);
+ }
+ APR_ARRAY_IDX(lb->merge_stack, i, merge_stack_t).emitted =
TRUE;
+ }
+ }
+ }
+
if (author)
author = svn_xml_fuzzy_escape(author, pool);
if (date)
@@ -606,7 +656,6 @@
if (log_entry->changed_paths2)
{
apr_array_header_t *sorted_paths;
- int i;
/* <paths> */
svn_xml_make_open_tag(&sb, pool, svn_xml_normal, "paths",
@@ -684,10 +733,15 @@
if (log_entry->has_children)
{
+ merge_stack_t ms;
+
if (! lb->merge_stack)
- lb->merge_stack = apr_array_make(lb->pool, 1,
sizeof(svn_revnum_t));
+ lb->merge_stack = apr_array_make(lb->pool, 1,
sizeof(merge_stack_t));
- APR_ARRAY_PUSH(lb->merge_stack, svn_revnum_t) = log_entry->revision;
+ ms.rev = log_entry->revision;
+ ms.emitted = TRUE;
+ ms.subtractive_merge = log_entry->subtractive_merge;
+ APR_ARRAY_PUSH(lb->merge_stack, merge_stack_t) = ms;
}
else
svn_xml_make_close_tag(&sb, pool, "logentry");
Index: subversion/tests/cmdline/log_tests.py
===================================================================
--- subversion/tests/cmdline/log_tests.py (revision 1892456)
+++ subversion/tests/cmdline/log_tests.py (working copy)
@@ -2779,7 +2779,6 @@
'',
'-q', '-c', '1-2')
-@XFail()
@Issue(4711)
def log_with_merge_history_and_search(sbox):
"log --use-merge-history --search"
]]]
Kind regards,
Daniel
