You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2011/09/10 03:43:36 UTC
[PATCH] svn mergeinfo --elide
Users with lots of subtree mergeinfo may want to clean it up by
eliding subtree mergeinfo without actually performing a merge.
There seem to be scripts floating around that perform this task.
But there is no "official" solution yet, it seems.
Would it be useful to allow mergeinfo elision to take place outside
the context of a merge? The patch below makes it possible to trigger
mergeinfo elision by running 'svn mergeinfo --elide'.
It seems that the elision algorithm is very basic at the moment,
which prevents this from being really useful. Subversion only elides
mergeinfo which exactly matches the mergeinfo of a parent.
It doesn't perform elision if mergeinfo on the parent covers a
greater range of revisions than the mergeinfo on the child.
Is this deliberate?
For instance:
r3 changed '/branch/gamma'.
This is merged into '/trunk', as follows:
$ svn merge --reintegrate ^/branch/gamma gamma
--- Merging differences between repository URLs into 'gamma':
U gamma/delta
--- Recording mergeinfo for merge between repository URLs into 'gamma':
U gamma
$
Nothing further happens on the branch.
Later, we merge the entire branch (a no-op merge):
$ svn merge --reintegrate ^/branch
--- Recording mergeinfo for merge between repository URLs into '.':
U .
$
Note that no elision took place.
Now we have the following mergeinfo which will not elide:
Properties on '.':
svn:mergeinfo
/branch:2-4
Properties on 'gamma':
svn:mergeinfo
/branch:2-3
The following mergeinfo will elide:
Properties on '.':
svn:mergeinfo
/branch:2-4
Properties on 'gamma':
svn:mergeinfo
/branch:2-4
So the user is required to run no-op merges on subtrees in order
to elide mergeinfo from subtrees. This can be quite tedious.
It should be safe elide mergeinfo from 'gamma' to the root of the
working copy. The mergeinfo on the parent is inheritable and a
superset of the mergeinfo on 'gamma'.
If the elision algorithm supported elision of mergeinfo which
is a proper subset of the mergeinfo in a parent, cleaning up
subtree mergeinfo would become a lot more easier than it is now.
Would it be a good idea to commit this patch and wait for the
elision algorithm to improve? Having 'svn mergeinfo --elide'
might make it a bit easier to perform ad-hoc testing while
improving the algorithm.
This thread on users@ inspired the idea:
http://svn.haxx.se/users/archive-2011-09/0101.shtml
[[[
Teach the 'svn mergeinfo' command to elide mergeinfo.
In some situations it can be useful to remove redundant mergeinfo
without actually running a merge.
Add a new option --elide to 'svn mergeinfo' which causes it
to elide mergeinfo instead of displaying it. The new --elide
option is mutually exclusive with the --show-revs option.
* subversion/include/svn_client.h
(svn_client_elide_subtree_mergeinfo): New.
* subversion/svn/cl.h
(svn_cl__opt_state_t): New option ELIDE_MERGEINFO.
* subversion/svn/mergeinfo-cmd.c
(svn_cl__mergeinfo): If the --elide option was given, elide
mergeinfo within the specified working copy path instead
of displaying mergeinfo, and default to depth 'infinity'.
If the working copy path was not given, default to the
current directory.
* subversion/svn/main.c
(svn_cl__longopt_t, svn_cl__options): New option --elide.
(svn_cl__cmd_table): Update help for 'svn mergeinfo'.
(main): Handle new --elide option. Disallow --elide and
--show-revs being used together.
* subversion/libsvn_client/mergeinfo.c
(elide_subtree_mergeinfo_baton, elide_subtree_mergeinfo_cb,
elide_subtree_mergeinfo_locked, svn_client_elide_subtree_mergeinfo): New.
]]]
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 1167379)
+++ subversion/include/svn_client.h (working copy)
@@ -3587,6 +3587,15 @@ svn_client_mergeinfo_log_eligible(const char *path
svn_client_ctx_t *ctx,
apr_pool_t *pool);
+/* Try to elide mergeinfo on working copy node @a local_abspath
+ * and children within the specified @a depth.
+ * Use @a scratch_pool for any temporary allocations. */
+svn_error_t *
+svn_client_elide_subtree_mergeinfo(const char *local_abspath,
+ svn_depth_t depth,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *scratch_pool);
+
/** @} */
/**
Index: subversion/svn/cl.h
===================================================================
--- subversion/svn/cl.h (revision 1167379)
+++ subversion/svn/cl.h (working copy)
@@ -230,6 +230,7 @@ typedef struct svn_cl__opt_state_t
svn_boolean_t internal_diff; /* override diff_cmd in config file */
svn_boolean_t use_git_diff_format; /* Use git's extended diff format */
svn_boolean_t allow_mixed_rev; /* Allow operation on mixed-revision WC */
+ svn_boolean_t elide_mergeinfo; /* Elide mergeinfo. */
} svn_cl__opt_state_t;
Index: subversion/svn/mergeinfo-cmd.c
===================================================================
--- subversion/svn/mergeinfo-cmd.c (revision 1167379)
+++ subversion/svn/mergeinfo-cmd.c (working copy)
@@ -30,6 +30,7 @@
#include "svn_pools.h"
#include "svn_client.h"
#include "svn_cmdline.h"
+#include "svn_dirent_uri.h"
#include "svn_path.h"
#include "svn_error.h"
#include "svn_error_codes.h"
@@ -66,14 +67,40 @@ svn_cl__mergeinfo(apr_getopt_t *os,
apr_array_header_t *targets;
const char *source, *target;
svn_opt_revision_t src_peg_revision, tgt_peg_revision;
- /* Default to depth empty. */
- svn_depth_t depth = opt_state->depth == svn_depth_unknown
- ? svn_depth_empty : opt_state->depth;
-
+ svn_depth_t depth;
+
SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
opt_state->targets,
ctx, FALSE, pool));
+ /* If we're eliding mergeinfo we don't display any. */
+ if (opt_state->elide_mergeinfo)
+ {
+ if (targets->nelts > 1)
+ return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("Too many arguments given"));
+
+ if (targets->nelts == 1)
+ target = APR_ARRAY_IDX(targets, 0, const char *);
+ else
+ target = "";
+
+ SVN_ERR(svn_dirent_get_absolute(&target, target, pool));
+
+ /* Default to depth infinity. */
+ depth = opt_state->depth == svn_depth_unknown
+ ? svn_depth_infinity : opt_state->depth;
+
+ SVN_ERR(svn_client_elide_subtree_mergeinfo(target, depth, ctx, pool));
+ return SVN_NO_ERROR;
+ }
+
+ /* Not eliding mergeinfo, so display mergeinfo. */
+
+ /* Default to depth empty. */
+ depth = opt_state->depth == svn_depth_unknown
+ ? svn_depth_empty : opt_state->depth;
+
/* We expect a single source URL followed by a single target --
nothing more, nothing less. */
if (targets->nelts < 1)
Index: subversion/svn/main.c
===================================================================
--- subversion/svn/main.c (revision 1167379)
+++ subversion/svn/main.c (working copy)
@@ -125,6 +125,7 @@ typedef enum svn_cl__longopt_t {
opt_internal_diff,
opt_use_git_diff_format,
opt_allow_mixed_revisions,
+ opt_elide,
} svn_cl__longopt_t;
@@ -342,6 +343,7 @@ const apr_getopt_option_t svn_cl__options[] =
"Use of this option is not recommended!\n"
" "
"Please run 'svn update' instead.")},
+ {"elide", opt_elide, 0, N_("elide mergeinfo")},
/* Long-opt Aliases
*
@@ -926,16 +928,21 @@ const svn_opt_subcommand_desc2_t svn_cl__cmd_table
opt_allow_mixed_revisions} },
{ "mergeinfo", svn_cl__mergeinfo, {0}, N_
- ("Display merge-related information.\n"
- "usage: mergeinfo SOURCE[@REV] [TARGET[@REV]]\n"
+ ("Display or eliminate redundancy in merge-related information.\n"
+ "usage: 1. mergeinfo SOURCE[@REV] [TARGET[@REV]]\n"
+ " 2. mergeinfo --elide [PATH]\n"
"\n"
- " Display information related to merges (or potential merges) between\n"
- " SOURCE and TARGET (default: '.'). Display the type of information\n"
- " specified by the --show-revs option. If --show-revs isn't passed,\n"
- " it defaults to --show-revs='merged'.\n"
+ " 1. Display information related to merges (or potential merges) between\n"
+ " SOURCE and TARGET (default: '.'). Display the type of information\n"
+ " specified by the --show-revs option. If --show-revs isn't passed,\n"
+ " it defaults to --show-revs='merged'.\n"
+ " The depth can be 'empty' or 'infinity'; the default is 'empty'.\n"
"\n"
- " The depth can be 'empty' or 'infinity'; the default is 'empty'.\n"),
- {'r', 'R', opt_depth, opt_show_revs} },
+ " 2. Attempt to elide merge information within the subtree rooted at PATH\n"
+ " (default '.'). This removes redundant svn:mergeinfo properties from\n"
+ " children whose parents record a superset of the same merge information.\n"
+ " The depth can be any valid depth value; the default is 'infinity'.\n"),
+ {'r', 'R', opt_depth, opt_show_revs, opt_elide} },
{ "mkdir", svn_cl__mkdir, {0}, N_
("Create a new directory under version control.\n"
@@ -1525,7 +1532,7 @@ main(int argc, const char *argv[])
opt_state.depth = svn_depth_unknown;
opt_state.set_depth = svn_depth_unknown;
opt_state.accept_which = svn_cl__accept_unspecified;
- opt_state.show_revs = svn_cl__show_revs_merged;
+ opt_state.show_revs = svn_cl__show_revs_invalid;
/* No args? Show usage. */
if (argc <= 1)
@@ -2030,6 +2037,9 @@ main(int argc, const char *argv[])
case opt_allow_mixed_revisions:
opt_state.allow_mixed_rev = TRUE;
break;
+ case opt_elide:
+ opt_state.elide_mergeinfo = TRUE;
+ break;
default:
/* Hmmm. Perhaps this would be a good place to squirrel away
opts that commands like svn diff might need. Hmmm indeed. */
@@ -2222,6 +2232,21 @@ main(int argc, const char *argv[])
return svn_cmdline_handle_exit_error(err, pool, "svn: ");
}
+ /* Disallow simultaneous use of both --show-revs and --elide. */
+ if (opt_state.elide_mergeinfo &&
+ opt_state.show_revs != svn_cl__show_revs_invalid)
+ {
+ err = svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL,
+ _("--show-revs and --elide are mutually "
+ "exclusive"));
+ return svn_cmdline_handle_exit_error(err, pool, "svn: ");
+ }
+ else if (opt_state.show_revs == svn_cl__show_revs_invalid)
+ {
+ /* Set the default value of --show-revs. */
+ opt_state.show_revs = svn_cl__show_revs_merged;
+ }
+
/* Ensure that 'revision_ranges' has at least one item, and make
'start_revision' and 'end_revision' match that item. */
if (opt_state.revision_ranges->nelts == 0)
Index: subversion/libsvn_client/mergeinfo.c
===================================================================
--- subversion/libsvn_client/mergeinfo.c (revision 1167379)
+++ subversion/libsvn_client/mergeinfo.c (working copy)
@@ -2255,3 +2255,64 @@ svn_client__mergeinfo_status(svn_boolean_t *mergei
return SVN_NO_ERROR;
}
+
+/* Baton for elide_subtree_mergeinfo_cb() and
+ * elide_subtree_mergeinfo_locked(). */
+struct elide_subtree_mergeinfo_baton {
+ const char *local_abspath;
+ svn_client_ctx_t *ctx;
+ svn_depth_t depth;
+};
+
+/* Implements svn_wc__proplist_receiver_t. */
+static svn_error_t *
+elide_subtree_mergeinfo_cb(void *baton,
+ const char *local_abspath,
+ apr_hash_t *props,
+ apr_pool_t *scratch_pool)
+{
+ struct elide_subtree_mergeinfo_baton *b = baton;
+
+ SVN_ERR(svn_client__elide_mergeinfo(local_abspath,
+ b->local_abspath,
+ b->ctx, scratch_pool));
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+elide_subtree_mergeinfo_locked(void *baton,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ struct elide_subtree_mergeinfo_baton *b = baton;
+
+ /* Use proplist to find nodes with mergeinfo, and try to elide
+ * mergeinfo on each. */
+ SVN_ERR(svn_wc__prop_list_recursive(b->ctx->wc_ctx, b->local_abspath,
+ SVN_PROP_MERGEINFO,
+ b->depth, FALSE, FALSE, NULL,
+ elide_subtree_mergeinfo_cb, baton,
+ b->ctx->cancel_func,
+ b->ctx->cancel_baton,
+ scratch_pool));
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_client_elide_subtree_mergeinfo(const char *local_abspath,
+ svn_depth_t depth,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *scratch_pool)
+{
+ struct elide_subtree_mergeinfo_baton baton;
+
+ baton.local_abspath = local_abspath;
+ baton.depth = depth;
+ baton.ctx = ctx;
+
+ SVN_ERR(svn_wc__call_with_write_lock(elide_subtree_mergeinfo_locked,
+ &baton, ctx->wc_ctx,
+ local_abspath, FALSE,
+ scratch_pool, scratch_pool));
+ return SVN_NO_ERROR;
+}
Re: now with more agressive elision (was: [PATCH] svn mergeinfo
--elide)
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Sep 13, 2011 at 11:15:39AM -0400, Bob Archer wrote:
> My question would be will this only elide merge info from child nodes
> that have been modified by the merge in the same way that in 1.7 child
> node merge info is only updated if that child node was a target of the
> merge?
Yes. The merge code already considers only modifiied subtrees during
elision. There is no intention to change that.
RE: now with more agressive elision (was: [PATCH] svn mergeinfo
--elide)
Posted by Bob Archer <Bo...@amsi.com>.
> On Sat, Sep 10, 2011 at 03:43:36AM +0200, Stefan Sperling wrote:
> > It seems that the elision algorithm is very basic at the moment, which
> > prevents this from being really useful. Subversion only elides
> > mergeinfo which exactly matches the mergeinfo of a parent.
> > It doesn't perform elision if mergeinfo on the parent covers a greater
> > range of revisions than the mergeinfo on the child.
> >
> > Is this deliberate?
> >
> > For instance:
> >
> > r3 changed '/branch/gamma'.
> > This is merged into '/trunk', as follows:
> >
> > $ svn merge --reintegrate ^/branch/gamma gamma
> > --- Merging differences between repository URLs into 'gamma':
> > U gamma/delta
> > --- Recording mergeinfo for merge between repository URLs into 'gamma':
> > U gamma
> > $
> >
> > Nothing further happens on the branch.
> > Later, we merge the entire branch (a no-op merge):
> >
> > $ svn merge --reintegrate ^/branch
> > --- Recording mergeinfo for merge between repository URLs into '.':
> > U .
> > $
> >
> > Note that no elision took place.
> > Now we have the following mergeinfo which will not elide:
> >
> > Properties on '.':
> > svn:mergeinfo
> > /branch:2-4
> > Properties on 'gamma':
> > svn:mergeinfo
> > /branch:2-3
> >
> > The following mergeinfo will elide:
> >
> > Properties on '.':
> > svn:mergeinfo
> > /branch:2-4
> > Properties on 'gamma':
> > svn:mergeinfo
> > /branch:2-4
> >
> > So the user is required to run no-op merges on subtrees in order to
> > elide mergeinfo from subtrees. This can be quite tedious.
>
> The updated patch below allows mergeinfo to elide if the child mergeinfo is a
> subset of the mergeinfo inherited from a parent.
>
> This seems to work fine in ad-hoc testing.
> There are some test failures with this patch, but before looking into fixing
> those I'd like to ask merge-tracking gurus if I am being naive and overlooking
> some obvious problem?
>
> I would commit this change in two steps. One that elides mergeinfo more
> aggressively, and one for the mergeinfo --elide parts.
> However, this combined patch is more useful for testing and review.
My question would be will this only elide merge info from child nodes that have been modified by the merge in the same way that in 1.7 child node merge info is only updated if that child node was a target of the merge?
I don't think you want to re-introduce the problem of mergeinfo being modified on child nodes that the merge didn't affect after making the enhancement to merge in 1.7.
BOb
Re: now with more agressive elision (was: [PATCH] svn mergeinfo --elide)
Posted by Paul Burba <pt...@gmail.com>.
On Sat, Sep 10, 2011 at 1:11 PM, Stefan Sperling <st...@elego.de> wrote:
> On Sat, Sep 10, 2011 at 07:07:40PM +0200, Stefan Sperling wrote:
>> On Sat, Sep 10, 2011 at 12:45:50PM -0400, Paul Burba wrote:
>> > ### This leaves us with a situation where r4 hasn't been merged
>> > ### to the branch:
>> >
>> > trunk@1167503>svn st
>> > M branch
>> > M branch\B
>> > M branch\D\gamma
>> >
>> > trunk@1167503>svn pl -vR
>> > Properties on 'branch':
>> > svn:mergeinfo
>> > /A:2-4
>> > Properties on 'branch\B':
>> > svn:mergeinfo
>> > /A/B:2-3
>>
>> > But your patch would elide[2] the mergeinfo on ^/branch/B and make it
>> > appear r4 was fully merged to ^/branch, which is clearly not the case.
>>
>> Quite right, it would only leave /A:2-4 on the 'branch' directory.
>> Thanks.
>
> Is the key problem here that the elision algorithm doesn't know (or
> care) whether r4 is operative on 'branch\B'?
Yes.
> It seems that if r4 is in-operative on 'branch\B' we could elide.
> And if it is, we cannot elide. Is this correct?
Yes again.
Paul
Re: now with more agressive elision (was: [PATCH] svn mergeinfo
--elide)
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Sep 10, 2011 at 07:07:40PM +0200, Stefan Sperling wrote:
> On Sat, Sep 10, 2011 at 12:45:50PM -0400, Paul Burba wrote:
> > ### This leaves us with a situation where r4 hasn't been merged
> > ### to the branch:
> >
> > trunk@1167503>svn st
> > M branch
> > M branch\B
> > M branch\D\gamma
> >
> > trunk@1167503>svn pl -vR
> > Properties on 'branch':
> > svn:mergeinfo
> > /A:2-4
> > Properties on 'branch\B':
> > svn:mergeinfo
> > /A/B:2-3
>
> > But your patch would elide[2] the mergeinfo on ^/branch/B and make it
> > appear r4 was fully merged to ^/branch, which is clearly not the case.
>
> Quite right, it would only leave /A:2-4 on the 'branch' directory.
> Thanks.
Is the key problem here that the elision algorithm doesn't know (or
care) whether r4 is operative on 'branch\B'?
It seems that if r4 is in-operative on 'branch\B' we could elide.
And if it is, we cannot elide. Is this correct?
Re: now with more agressive elision (was: [PATCH] svn mergeinfo
--elide)
Posted by Peter Samuelson <pe...@p12n.org>.
[Paul Burba]
> Given:
>
> A path P and some path-wise child of P called C:
> Assume both P and C have explicit mergeinfo.
> Assume there is no intermediate patch between P and C with explicit mergeinfo.
> Assume P and C are at the same revision as are any intermediate
> paths between them.
>
> A) Common mergeinfo (that differs only by the path-wise difference
> between P and C) - This can elide.
>
> B) Mergeinfo exclusive to P - If *all* of this describes merges that
> are inoperative on C, then this can elide.
>
> C) Mergeinfo exclusive to C - If all of this describes merges that are
> operative only on C and/or on subtrees which are not part of P, then
> this can elide.
Meaning, I guess, set (C) is added to path P, _then_ C can elide.
Also, obviously, this is only possible if the paths in set (C) end in
the same suffix as {C relative to P}.
Note also that there's a fourth case, similar to B), in which P has
non-inheritable MI that is operative only on P and/or those of its
children that have explicit MI. (This can easily arise from merging
into a sparse wc, whose missing subtrees would have been inoperative.)
Such non-inheritable MI can be converted to inheritable, which of
course makes other elision cases possible.
--
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/
Re: now with more agressive elision (was: [PATCH] svn mergeinfo
--elide)
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Sep 10, 2011 at 02:08:38PM -0400, Paul Burba wrote:
> Given:
>
> A path P and some path-wise child of P called C:
> Assume both P and C have explicit mergeinfo.
> Assume there is no intermediate patch between P and C with explicit mergeinfo.
> Assume P and C are at the same revision as are any intermediate
> paths between them.
>
Ah, right. I didn't think at all about mixed-revisions...
> Question:
>
> Can the mergeinfo on C elide to P?
>
> Looking at the intersection of the P and C's mergeinfo we have:
>
> A) Common mergeinfo (that differs only by the path-wise difference
> between P and C) - This can elide.
>
> B) Mergeinfo exclusive to P - If *all* of this describes merges that
> are inoperative on C, then this can elide.
>
> C) Mergeinfo exclusive to C - If all of this describes merges that are
> operative only on C and/or on subtrees which are not part of P, then
> this can elide.
>
> If elision occurs in *all* three cases then the mergeinfo on C can elide to P.
Nice. We should try to implement that.
> > Do you think there is value in adding just 'svn mergeinfo --elide'
> > and keep using the present elision code?
>
> Yes, I think the basic idea is sound: Some way to trigger the elision
> code outside of a merge proper. Admittedly today it has rather
> limited usefulness, but only because elision itself is so "dumb".
> Once elision gets smarter it will be more useful. But there's no
> reason no to add it now.
Ok, I'll add the --elide option so. Thanks Paul!
Re: now with more agressive elision (was: [PATCH] svn mergeinfo --elide)
Posted by Paul Burba <pt...@gmail.com>.
On Sat, Sep 10, 2011 at 1:07 PM, Stefan Sperling <st...@elego.de> wrote:
> On Sat, Sep 10, 2011 at 12:45:50PM -0400, Paul Burba wrote:
>> ### This leaves us with a situation where r4 hasn't been merged
>> ### to the branch:
>>
>> trunk@1167503>svn st
>> M branch
>> M branch\B
>> M branch\D\gamma
>>
>> trunk@1167503>svn pl -vR
>> Properties on 'branch':
>> svn:mergeinfo
>> /A:2-4
>> Properties on 'branch\B':
>> svn:mergeinfo
>> /A/B:2-3
>
>> But your patch would elide[2] the mergeinfo on ^/branch/B and make it
>> appear r4 was fully merged to ^/branch, which is clearly not the case.
>
> Quite right, it would only leave /A:2-4 on the 'branch' directory.
> Thanks.
>
>> [1] Of course a path with explicit mergeinfo never inherits mergeinfo,
>> what I mean here is simply the mergeinfo it would inherit *if* it had
>> no explicit mergeinfo.
>
> So non-inheritable ranges only apply to nodes which do not have their
> own svn:mergeinfo.
No, rather, "non-inheritable ranges only apply to nodes which *DO*
have their own svn:mergeinfo."
Say a path src/branches/b1 has the mergeinfo:
'/src/trunk:556-1001:1200*
r1200 applies only to src/branches/b1, the path on which it is
explicitly set. All of b1's children inherit only r556-1001.
I'm pretty sure you already know this, it's just that we keep using
different words to describe the same thing ;-)
> And any subtree mergeinfo overrides the mergeinfo
> of its ancestors.
Yes, that is all I was trying to say. If a path has explicit
mergeinfo that mergeinfo is a complete description of what has been
merged to that path, we don't care about any of the path's parents.
>Correct?
> In which ways do you think automatic elision needs to be improved?
Given:
A path P and some path-wise child of P called C:
Assume both P and C have explicit mergeinfo.
Assume there is no intermediate patch between P and C with explicit mergeinfo.
Assume P and C are at the same revision as are any intermediate
paths between them.
Question:
Can the mergeinfo on C elide to P?
Looking at the intersection of the P and C's mergeinfo we have:
A) Common mergeinfo (that differs only by the path-wise difference
between P and C) - This can elide.
B) Mergeinfo exclusive to P - If *all* of this describes merges that
are inoperative on C, then this can elide.
C) Mergeinfo exclusive to C - If all of this describes merges that are
operative only on C and/or on subtrees which are not part of P, then
this can elide.
If elision occurs in *all* three cases then the mergeinfo on C can elide to P.
Today only 'A' is supported when 'B' and 'C' are empty sets.
> Do you think there is value in adding just 'svn mergeinfo --elide'
> and keep using the present elision code?
Yes, I think the basic idea is sound: Some way to trigger the elision
code outside of a merge proper. Admittedly today it has rather
limited usefulness, but only because elision itself is so "dumb".
Once elision gets smarter it will be more useful. But there's no
reason no to add it now.
Paul
Re: now with more agressive elision (was: [PATCH] svn mergeinfo
--elide)
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Sep 10, 2011 at 12:45:50PM -0400, Paul Burba wrote:
> ### This leaves us with a situation where r4 hasn't been merged
> ### to the branch:
>
> trunk@1167503>svn st
> M branch
> M branch\B
> M branch\D\gamma
>
> trunk@1167503>svn pl -vR
> Properties on 'branch':
> svn:mergeinfo
> /A:2-4
> Properties on 'branch\B':
> svn:mergeinfo
> /A/B:2-3
> But your patch would elide[2] the mergeinfo on ^/branch/B and make it
> appear r4 was fully merged to ^/branch, which is clearly not the case.
Quite right, it would only leave /A:2-4 on the 'branch' directory.
Thanks.
> [1] Of course a path with explicit mergeinfo never inherits mergeinfo,
> what I mean here is simply the mergeinfo it would inherit *if* it had
> no explicit mergeinfo.
So non-inheritable ranges only apply to nodes which do not have their
own svn:mergeinfo. And any subtree mergeinfo overrides the mergeinfo
of its ancestors. Correct?
In which ways do you think automatic elision needs to be improved?
Do you think there is value in adding just 'svn mergeinfo --elide'
and keep using the present elision code?
Re: now with more agressive elision (was: [PATCH] svn mergeinfo --elide)
Posted by Paul Burba <pt...@gmail.com>.
On Sat, Sep 10, 2011 at 5:43 AM, Stefan Sperling <st...@elego.de> wrote:
> On Sat, Sep 10, 2011 at 03:43:36AM +0200, Stefan Sperling wrote:
>> It seems that the elision algorithm is very basic at the moment,
>> which prevents this from being really useful. Subversion only elides
>> mergeinfo which exactly matches the mergeinfo of a parent.
>> It doesn't perform elision if mergeinfo on the parent covers a
>> greater range of revisions than the mergeinfo on the child.
>>
>> Is this deliberate?
Hi Stefan,
Yes it is, see below...
>> For instance:
>>
>> r3 changed '/branch/gamma'.
>> This is merged into '/trunk', as follows:
>>
>> $ svn merge --reintegrate ^/branch/gamma gamma
>> --- Merging differences between repository URLs into 'gamma':
>> U gamma/delta
>> --- Recording mergeinfo for merge between repository URLs into 'gamma':
>> U gamma
>> $
>>
>> Nothing further happens on the branch.
>> Later, we merge the entire branch (a no-op merge):
>>
>> $ svn merge --reintegrate ^/branch
>> --- Recording mergeinfo for merge between repository URLs into '.':
>> U .
>> $
>>
>> Note that no elision took place.
>> Now we have the following mergeinfo which will not elide:
>>
>> Properties on '.':
>> svn:mergeinfo
>> /branch:2-4
>> Properties on 'gamma':
>> svn:mergeinfo
>> /branch:2-3
>>
>> The following mergeinfo will elide:
>>
>> Properties on '.':
>> svn:mergeinfo
>> /branch:2-4
>> Properties on 'gamma':
>> svn:mergeinfo
>> /branch:2-4
>>
>> So the user is required to run no-op merges on subtrees in order
>> to elide mergeinfo from subtrees. This can be quite tedious.
>
> The updated patch below allows mergeinfo to elide if the child
> mergeinfo is a subset of the mergeinfo inherited from a parent.
>
> This seems to work fine in ad-hoc testing.
> There are some test failures with this patch, but before looking
> into fixing those I'd like to ask merge-tracking gurus if I am being
> naive and overlooking some obvious problem?
You are quite right that elision needs to be smarter, particularly in
the 1.7 world where we no longer set mergeinfo unconditionally on all
subtrees. As it stands now elision will only work in the simplest of
cases; we need a more aggressive approach. Unfortunately yours is a
bit too simplistic. Simply because a subtree's explicit mergeinfo is
a subset of its inherited[1] mergeinfo doesn't allow us to elide the
subtree mergeinfo.
One way your approach is broken is with reverse subtree merges. A
quick demonstration of what I mean, starting with one of our vanilla
Greek trees:
### Make a simple branch, then make a couple of edits on the "trunk"
### in r3 and r4:
trunk@1167503>svn copy ^^/A ^^/branch -m "Make a branch"
Committed revision 2.
trunk@1167503>echo trunk edit> A\D\gamma
trunk@1167503>svn ci -m "trunk edit"
Sending A\D\gamma
Transmitting file data .
Committed revision 3.
trunk@1167503>echo trunk edit> A\B\lambda
trunk@1167503>svn ci -m "trunk edit"
Sending A\B\lambda
Transmitting file data .
Committed revision 4.
trunk@1167503>svn up -q
### Sync merge the root of the branch with "trunk".
### This applies r3 and r4 to the branch:
trunk@1167503>svn merge ^^/A branch
--- Merging r2 through r4 into 'branch':
U branch\B\lambda
U branch\D\gamma
--- Recording mergeinfo for merge of r2 through r4 into 'branch':
U branch
trunk@1167503>svn pl -vR
Properties on 'branch':
svn:mergeinfo
/A:2-4
### Now reverse merge r4 from a subtree:
trunk@1167503>svn merge ^^/A/B branch\B -c-4
--- Reverse-merging r4 into 'branch\B':
G branch\B\lambda
--- Recording mergeinfo for reverse merge of r4 into 'branch\B':
G branch\B
### This leaves us with a situation where r4 hasn't been merged
### to the branch:
trunk@1167503>svn st
M branch
M branch\B
M branch\D\gamma
trunk@1167503>svn pl -vR
Properties on 'branch':
svn:mergeinfo
/A:2-4
Properties on 'branch\B':
svn:mergeinfo
/A/B:2-3
trunk@1167503>svn ci -m "partial sync merge"
Sending branch
Sending branch\B
Sending branch\D\gamma
Transmitting file data .
Committed revision 5.
But your patch would elide[2] the mergeinfo on ^/branch/B and make it
appear r4 was fully merged to ^/branch, which is clearly not the case.
Paul
[1] Of course a path with explicit mergeinfo never inherits mergeinfo,
what I mean here is simply the mergeinfo it would inherit *if* it had
no explicit mergeinfo.
[2] IIUC, I cannot apply your patch to my windows WC with either TSVN
or svn patch...some line ending problems.
> I would commit this change in two steps. One that elides mergeinfo
> more aggressively, and one for the mergeinfo --elide parts.
> However, this combined patch is more useful for testing and review.
>
> [[[
> Elide mergeinfo if mergeinfo inherited from a parent is a superset
> of existing mergeinfo.
>
> In some situations it can be useful to remove redundant mergeinfo
> without actually running a merge. For this purpose, add a new option
> --elide to 'svn mergeinfo' which causes it to elide mergeinfo instead
> of displaying it. The new --elide option is mutually exclusive with
> the --show-revs option.
>
> * subversion/include/svn_client.h
> (svn_client_elide_subtree_mergeinfo): New.
>
> * subversion/svn/cl.h
> (svn_cl__opt_state_t): New option ELIDE_MERGEINFO.
>
> * subversion/svn/mergeinfo-cmd.c
> (svn_cl__mergeinfo): If the --elide option was given, elide
> mergeinfo within the specified working copy path instead
> of displaying mergeinfo, and default to depth 'infinity'.
> If the working copy path was not given, default to the
> current directory.
>
> * subversion/svn/main.c
> (svn_cl__longopt_t, svn_cl__options): New option --elide.
> (svn_cl__cmd_table): Update help for 'svn mergeinfo'.
> (main): Handle new --elide option. Disallow --elide and
> --show-revs being used together.
>
> * subversion/libsvn_client/mergeinfo.c
> (should_elide_mergeinfo): Instead of checking the path-adjusted
> parent mergeinfo for equality with the child's mergeinfo, remove
> inheritable, and path-adjusted, parent mergeinfo from the child,
> and allow elision if the resulting child mergeinfo is empty.
> (elide_subtree_mergeinfo_baton, elide_subtree_mergeinfo_cb,
> elide_subtree_mergeinfo_locked, svn_client_elide_subtree_mergeinfo): New.
>
> * subversion/libsvn_client/mergeinfo.h
> (svn_client__elide_mergeinfo): Update documentation of elision rules.
> ]]]
>
> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h (revision 1167379)
> +++ subversion/include/svn_client.h (working copy)
> @@ -3587,6 +3587,17 @@ svn_client_mergeinfo_log_eligible(const char *path
> svn_client_ctx_t *ctx,
> apr_pool_t *pool);
>
> +/* Try to elide mergeinfo on working copy node @a local_abspath
> + * and children within the specified @a depth.
> + * Use @a scratch_pool for any temporary allocations.
> + * @see svn_client__elide_mergeinfo() for elision rules
> + * @since New in 1.8. */
> +svn_error_t *
> +svn_client_elide_subtree_mergeinfo(const char *local_abspath,
> + svn_depth_t depth,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *scratch_pool);
> +
> /** @} */
>
> /**
> Index: subversion/svn/cl.h
> ===================================================================
> --- subversion/svn/cl.h (revision 1167379)
> +++ subversion/svn/cl.h (working copy)
> @@ -230,6 +230,7 @@ typedef struct svn_cl__opt_state_t
> svn_boolean_t internal_diff; /* override diff_cmd in config file */
> svn_boolean_t use_git_diff_format; /* Use git's extended diff format */
> svn_boolean_t allow_mixed_rev; /* Allow operation on mixed-revision WC */
> + svn_boolean_t elide_mergeinfo; /* Elide mergeinfo. */
> } svn_cl__opt_state_t;
>
>
> Index: subversion/svn/mergeinfo-cmd.c
> ===================================================================
> --- subversion/svn/mergeinfo-cmd.c (revision 1167379)
> +++ subversion/svn/mergeinfo-cmd.c (working copy)
> @@ -30,6 +30,7 @@
> #include "svn_pools.h"
> #include "svn_client.h"
> #include "svn_cmdline.h"
> +#include "svn_dirent_uri.h"
> #include "svn_path.h"
> #include "svn_error.h"
> #include "svn_error_codes.h"
> @@ -66,14 +67,40 @@ svn_cl__mergeinfo(apr_getopt_t *os,
> apr_array_header_t *targets;
> const char *source, *target;
> svn_opt_revision_t src_peg_revision, tgt_peg_revision;
> - /* Default to depth empty. */
> - svn_depth_t depth = opt_state->depth == svn_depth_unknown
> - ? svn_depth_empty : opt_state->depth;
> -
> + svn_depth_t depth;
> +
> SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
> opt_state->targets,
> ctx, FALSE, pool));
>
> + /* If we're eliding mergeinfo we don't display any. */
> + if (opt_state->elide_mergeinfo)
> + {
> + if (targets->nelts > 1)
> + return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
> + _("Too many arguments given"));
> +
> + if (targets->nelts == 1)
> + target = APR_ARRAY_IDX(targets, 0, const char *);
> + else
> + target = "";
> +
> + SVN_ERR(svn_dirent_get_absolute(&target, target, pool));
> +
> + /* Default to depth infinity. */
> + depth = opt_state->depth == svn_depth_unknown
> + ? svn_depth_infinity : opt_state->depth;
> +
> + SVN_ERR(svn_client_elide_subtree_mergeinfo(target, depth, ctx, pool));
> + return SVN_NO_ERROR;
> + }
> +
> + /* Not eliding mergeinfo, so display mergeinfo. */
> +
> + /* Default to depth empty. */
> + depth = opt_state->depth == svn_depth_unknown
> + ? svn_depth_empty : opt_state->depth;
> +
> /* We expect a single source URL followed by a single target --
> nothing more, nothing less. */
> if (targets->nelts < 1)
> Index: subversion/svn/main.c
> ===================================================================
> --- subversion/svn/main.c (revision 1167379)
> +++ subversion/svn/main.c (working copy)
> @@ -125,6 +125,7 @@ typedef enum svn_cl__longopt_t {
> opt_internal_diff,
> opt_use_git_diff_format,
> opt_allow_mixed_revisions,
> + opt_elide,
> } svn_cl__longopt_t;
>
>
> @@ -342,6 +343,7 @@ const apr_getopt_option_t svn_cl__options[] =
> "Use of this option is not recommended!\n"
> " "
> "Please run 'svn update' instead.")},
> + {"elide", opt_elide, 0, N_("elide mergeinfo")},
>
> /* Long-opt Aliases
> *
> @@ -926,16 +928,21 @@ const svn_opt_subcommand_desc2_t svn_cl__cmd_table
> opt_allow_mixed_revisions} },
>
> { "mergeinfo", svn_cl__mergeinfo, {0}, N_
> - ("Display merge-related information.\n"
> - "usage: mergeinfo SOURCE[@REV] [TARGET[@REV]]\n"
> + ("Display or eliminate redundancy in merge-related information.\n"
> + "usage: 1. mergeinfo SOURCE[@REV] [TARGET[@REV]]\n"
> + " 2. mergeinfo --elide [PATH]\n"
> "\n"
> - " Display information related to merges (or potential merges) between\n"
> - " SOURCE and TARGET (default: '.'). Display the type of information\n"
> - " specified by the --show-revs option. If --show-revs isn't passed,\n"
> - " it defaults to --show-revs='merged'.\n"
> + " 1. Display information related to merges (or potential merges) between\n"
> + " SOURCE and TARGET (default: '.'). Display the type of information\n"
> + " specified by the --show-revs option. If --show-revs isn't passed,\n"
> + " it defaults to --show-revs='merged'.\n"
> + " The depth can be 'empty' or 'infinity'; the default is 'empty'.\n"
> "\n"
> - " The depth can be 'empty' or 'infinity'; the default is 'empty'.\n"),
> - {'r', 'R', opt_depth, opt_show_revs} },
> + " 2. Attempt to elide merge information within the subtree rooted at PATH\n"
> + " (default '.'). This removes redundant svn:mergeinfo properties from\n"
> + " children whose parents record a superset of the same merge information.\n"
> + " The depth can be any valid depth value; the default is 'infinity'.\n"),
> + {'r', 'R', opt_depth, opt_show_revs, opt_elide} },
>
> { "mkdir", svn_cl__mkdir, {0}, N_
> ("Create a new directory under version control.\n"
> @@ -1525,7 +1532,7 @@ main(int argc, const char *argv[])
> opt_state.depth = svn_depth_unknown;
> opt_state.set_depth = svn_depth_unknown;
> opt_state.accept_which = svn_cl__accept_unspecified;
> - opt_state.show_revs = svn_cl__show_revs_merged;
> + opt_state.show_revs = svn_cl__show_revs_invalid;
>
> /* No args? Show usage. */
> if (argc <= 1)
> @@ -2030,6 +2037,9 @@ main(int argc, const char *argv[])
> case opt_allow_mixed_revisions:
> opt_state.allow_mixed_rev = TRUE;
> break;
> + case opt_elide:
> + opt_state.elide_mergeinfo = TRUE;
> + break;
> default:
> /* Hmmm. Perhaps this would be a good place to squirrel away
> opts that commands like svn diff might need. Hmmm indeed. */
> @@ -2222,6 +2232,21 @@ main(int argc, const char *argv[])
> return svn_cmdline_handle_exit_error(err, pool, "svn: ");
> }
>
> + /* Disallow simultaneous use of both --show-revs and --elide. */
> + if (opt_state.elide_mergeinfo &&
> + opt_state.show_revs != svn_cl__show_revs_invalid)
> + {
> + err = svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL,
> + _("--show-revs and --elide are mutually "
> + "exclusive"));
> + return svn_cmdline_handle_exit_error(err, pool, "svn: ");
> + }
> + else if (opt_state.show_revs == svn_cl__show_revs_invalid)
> + {
> + /* Set the default value of --show-revs. */
> + opt_state.show_revs = svn_cl__show_revs_merged;
> + }
> +
> /* Ensure that 'revision_ranges' has at least one item, and make
> 'start_revision' and 'end_revision' match that item. */
> if (opt_state.revision_ranges->nelts == 0)
> Index: subversion/libsvn_client/mergeinfo.c
> ===================================================================
> --- subversion/libsvn_client/mergeinfo.c (revision 1167379)
> +++ subversion/libsvn_client/mergeinfo.c (working copy)
> @@ -840,9 +840,9 @@ should_elide_mergeinfo(svn_boolean_t *elides,
> else
> path_tweaked_parent_mergeinfo = parent_mergeinfo;
>
> - SVN_ERR(svn_mergeinfo__equals(elides,
> - path_tweaked_parent_mergeinfo,
> - child_mergeinfo, TRUE, subpool));
> + SVN_ERR(svn_mergeinfo_remove2(&child_mergeinfo, parent_mergeinfo,
> + child_mergeinfo, TRUE, subpool, subpool));
> + *elides = (apr_hash_count(child_mergeinfo) == 0);
> svn_pool_destroy(subpool);
> }
>
> @@ -2255,3 +2255,64 @@ svn_client__mergeinfo_status(svn_boolean_t *mergei
>
> return SVN_NO_ERROR;
> }
> +
> +/* Baton for elide_subtree_mergeinfo_cb() and
> + * elide_subtree_mergeinfo_locked(). */
> +struct elide_subtree_mergeinfo_baton {
> + const char *local_abspath;
> + svn_client_ctx_t *ctx;
> + svn_depth_t depth;
> +};
> +
> +/* Implements svn_wc__proplist_receiver_t. */
> +static svn_error_t *
> +elide_subtree_mergeinfo_cb(void *baton,
> + const char *local_abspath,
> + apr_hash_t *props,
> + apr_pool_t *scratch_pool)
> +{
> + struct elide_subtree_mergeinfo_baton *b = baton;
> +
> + SVN_ERR(svn_client__elide_mergeinfo(local_abspath,
> + b->local_abspath,
> + b->ctx, scratch_pool));
> + return SVN_NO_ERROR;
> +}
> +
> +static svn_error_t *
> +elide_subtree_mergeinfo_locked(void *baton,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool)
> +{
> + struct elide_subtree_mergeinfo_baton *b = baton;
> +
> + /* Use proplist to find nodes with mergeinfo, and try to elide
> + * mergeinfo on each. */
> + SVN_ERR(svn_wc__prop_list_recursive(b->ctx->wc_ctx, b->local_abspath,
> + SVN_PROP_MERGEINFO,
> + b->depth, FALSE, FALSE, NULL,
> + elide_subtree_mergeinfo_cb, baton,
> + b->ctx->cancel_func,
> + b->ctx->cancel_baton,
> + scratch_pool));
> + return SVN_NO_ERROR;
> +}
> +
> +svn_error_t *
> +svn_client_elide_subtree_mergeinfo(const char *local_abspath,
> + svn_depth_t depth,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *scratch_pool)
> +{
> + struct elide_subtree_mergeinfo_baton baton;
> +
> + baton.local_abspath = local_abspath;
> + baton.depth = depth;
> + baton.ctx = ctx;
> +
> + SVN_ERR(svn_wc__call_with_write_lock(elide_subtree_mergeinfo_locked,
> + &baton, ctx->wc_ctx,
> + local_abspath, FALSE,
> + scratch_pool, scratch_pool));
> + return SVN_NO_ERROR;
> +}
> Index: subversion/libsvn_client/mergeinfo.h
> ===================================================================
> --- subversion/libsvn_client/mergeinfo.h (revision 1167379)
> +++ subversion/libsvn_client/mergeinfo.h (working copy)
> @@ -347,7 +347,7 @@ svn_client__record_wc_mergeinfo(const char *local_
> B) TARGET_WCPATH has empty mergeinfo and its nearest parent also
> has empty mergeinfo.
>
> - C) TARGET_WCPATH has the same mergeinfo as its nearest parent
> + C) TARGET_WCPATH has a subset of the mergeinfo as its nearest parent
> when that parent's mergeinfo is adjusted for the path
> difference between the two, e.g.:
>
> @@ -355,9 +355,14 @@ svn_client__record_wc_mergeinfo(const char *local_
> TARGET_WCPATH's mergeinfo = '/A/D/H:3'
> TARGET_WCPATH nearest parent = A_COPY
> Parent's mergeinfo = '/A:3'
> - Path differece = 'D/H'
> + Path difference = 'D/H'
> Parent's adjusted mergeinfo = '/A/D/H:3'
>
> + TARGET_WCPATH's mergeinfo is a subset of the nearest parent
> + mergeinfo if removing mergeinfo inherited from the parent,
> + with adjusted paths, from TARGET_WCPATH's mergeinfo results
> + in empty mergeinfo. See svn_mergeinfo_remove2().
> +
> If Elision occurs remove the svn:mergeinfo property from
> TARGET_WCPATH. */
> svn_error_t *
>
now with more agressive elision (was: [PATCH] svn mergeinfo --elide)
Posted by Stefan Sperling <st...@elego.de>.
On Sat, Sep 10, 2011 at 03:43:36AM +0200, Stefan Sperling wrote:
> It seems that the elision algorithm is very basic at the moment,
> which prevents this from being really useful. Subversion only elides
> mergeinfo which exactly matches the mergeinfo of a parent.
> It doesn't perform elision if mergeinfo on the parent covers a
> greater range of revisions than the mergeinfo on the child.
>
> Is this deliberate?
>
> For instance:
>
> r3 changed '/branch/gamma'.
> This is merged into '/trunk', as follows:
>
> $ svn merge --reintegrate ^/branch/gamma gamma
> --- Merging differences between repository URLs into 'gamma':
> U gamma/delta
> --- Recording mergeinfo for merge between repository URLs into 'gamma':
> U gamma
> $
>
> Nothing further happens on the branch.
> Later, we merge the entire branch (a no-op merge):
>
> $ svn merge --reintegrate ^/branch
> --- Recording mergeinfo for merge between repository URLs into '.':
> U .
> $
>
> Note that no elision took place.
> Now we have the following mergeinfo which will not elide:
>
> Properties on '.':
> svn:mergeinfo
> /branch:2-4
> Properties on 'gamma':
> svn:mergeinfo
> /branch:2-3
>
> The following mergeinfo will elide:
>
> Properties on '.':
> svn:mergeinfo
> /branch:2-4
> Properties on 'gamma':
> svn:mergeinfo
> /branch:2-4
>
> So the user is required to run no-op merges on subtrees in order
> to elide mergeinfo from subtrees. This can be quite tedious.
The updated patch below allows mergeinfo to elide if the child
mergeinfo is a subset of the mergeinfo inherited from a parent.
This seems to work fine in ad-hoc testing.
There are some test failures with this patch, but before looking
into fixing those I'd like to ask merge-tracking gurus if I am being
naive and overlooking some obvious problem?
I would commit this change in two steps. One that elides mergeinfo
more aggressively, and one for the mergeinfo --elide parts.
However, this combined patch is more useful for testing and review.
[[[
Elide mergeinfo if mergeinfo inherited from a parent is a superset
of existing mergeinfo.
In some situations it can be useful to remove redundant mergeinfo
without actually running a merge. For this purpose, add a new option
--elide to 'svn mergeinfo' which causes it to elide mergeinfo instead
of displaying it. The new --elide option is mutually exclusive with
the --show-revs option.
* subversion/include/svn_client.h
(svn_client_elide_subtree_mergeinfo): New.
* subversion/svn/cl.h
(svn_cl__opt_state_t): New option ELIDE_MERGEINFO.
* subversion/svn/mergeinfo-cmd.c
(svn_cl__mergeinfo): If the --elide option was given, elide
mergeinfo within the specified working copy path instead
of displaying mergeinfo, and default to depth 'infinity'.
If the working copy path was not given, default to the
current directory.
* subversion/svn/main.c
(svn_cl__longopt_t, svn_cl__options): New option --elide.
(svn_cl__cmd_table): Update help for 'svn mergeinfo'.
(main): Handle new --elide option. Disallow --elide and
--show-revs being used together.
* subversion/libsvn_client/mergeinfo.c
(should_elide_mergeinfo): Instead of checking the path-adjusted
parent mergeinfo for equality with the child's mergeinfo, remove
inheritable, and path-adjusted, parent mergeinfo from the child,
and allow elision if the resulting child mergeinfo is empty.
(elide_subtree_mergeinfo_baton, elide_subtree_mergeinfo_cb,
elide_subtree_mergeinfo_locked, svn_client_elide_subtree_mergeinfo): New.
* subversion/libsvn_client/mergeinfo.h
(svn_client__elide_mergeinfo): Update documentation of elision rules.
]]]
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 1167379)
+++ subversion/include/svn_client.h (working copy)
@@ -3587,6 +3587,17 @@ svn_client_mergeinfo_log_eligible(const char *path
svn_client_ctx_t *ctx,
apr_pool_t *pool);
+/* Try to elide mergeinfo on working copy node @a local_abspath
+ * and children within the specified @a depth.
+ * Use @a scratch_pool for any temporary allocations.
+ * @see svn_client__elide_mergeinfo() for elision rules
+ * @since New in 1.8. */
+svn_error_t *
+svn_client_elide_subtree_mergeinfo(const char *local_abspath,
+ svn_depth_t depth,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *scratch_pool);
+
/** @} */
/**
Index: subversion/svn/cl.h
===================================================================
--- subversion/svn/cl.h (revision 1167379)
+++ subversion/svn/cl.h (working copy)
@@ -230,6 +230,7 @@ typedef struct svn_cl__opt_state_t
svn_boolean_t internal_diff; /* override diff_cmd in config file */
svn_boolean_t use_git_diff_format; /* Use git's extended diff format */
svn_boolean_t allow_mixed_rev; /* Allow operation on mixed-revision WC */
+ svn_boolean_t elide_mergeinfo; /* Elide mergeinfo. */
} svn_cl__opt_state_t;
Index: subversion/svn/mergeinfo-cmd.c
===================================================================
--- subversion/svn/mergeinfo-cmd.c (revision 1167379)
+++ subversion/svn/mergeinfo-cmd.c (working copy)
@@ -30,6 +30,7 @@
#include "svn_pools.h"
#include "svn_client.h"
#include "svn_cmdline.h"
+#include "svn_dirent_uri.h"
#include "svn_path.h"
#include "svn_error.h"
#include "svn_error_codes.h"
@@ -66,14 +67,40 @@ svn_cl__mergeinfo(apr_getopt_t *os,
apr_array_header_t *targets;
const char *source, *target;
svn_opt_revision_t src_peg_revision, tgt_peg_revision;
- /* Default to depth empty. */
- svn_depth_t depth = opt_state->depth == svn_depth_unknown
- ? svn_depth_empty : opt_state->depth;
-
+ svn_depth_t depth;
+
SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
opt_state->targets,
ctx, FALSE, pool));
+ /* If we're eliding mergeinfo we don't display any. */
+ if (opt_state->elide_mergeinfo)
+ {
+ if (targets->nelts > 1)
+ return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+ _("Too many arguments given"));
+
+ if (targets->nelts == 1)
+ target = APR_ARRAY_IDX(targets, 0, const char *);
+ else
+ target = "";
+
+ SVN_ERR(svn_dirent_get_absolute(&target, target, pool));
+
+ /* Default to depth infinity. */
+ depth = opt_state->depth == svn_depth_unknown
+ ? svn_depth_infinity : opt_state->depth;
+
+ SVN_ERR(svn_client_elide_subtree_mergeinfo(target, depth, ctx, pool));
+ return SVN_NO_ERROR;
+ }
+
+ /* Not eliding mergeinfo, so display mergeinfo. */
+
+ /* Default to depth empty. */
+ depth = opt_state->depth == svn_depth_unknown
+ ? svn_depth_empty : opt_state->depth;
+
/* We expect a single source URL followed by a single target --
nothing more, nothing less. */
if (targets->nelts < 1)
Index: subversion/svn/main.c
===================================================================
--- subversion/svn/main.c (revision 1167379)
+++ subversion/svn/main.c (working copy)
@@ -125,6 +125,7 @@ typedef enum svn_cl__longopt_t {
opt_internal_diff,
opt_use_git_diff_format,
opt_allow_mixed_revisions,
+ opt_elide,
} svn_cl__longopt_t;
@@ -342,6 +343,7 @@ const apr_getopt_option_t svn_cl__options[] =
"Use of this option is not recommended!\n"
" "
"Please run 'svn update' instead.")},
+ {"elide", opt_elide, 0, N_("elide mergeinfo")},
/* Long-opt Aliases
*
@@ -926,16 +928,21 @@ const svn_opt_subcommand_desc2_t svn_cl__cmd_table
opt_allow_mixed_revisions} },
{ "mergeinfo", svn_cl__mergeinfo, {0}, N_
- ("Display merge-related information.\n"
- "usage: mergeinfo SOURCE[@REV] [TARGET[@REV]]\n"
+ ("Display or eliminate redundancy in merge-related information.\n"
+ "usage: 1. mergeinfo SOURCE[@REV] [TARGET[@REV]]\n"
+ " 2. mergeinfo --elide [PATH]\n"
"\n"
- " Display information related to merges (or potential merges) between\n"
- " SOURCE and TARGET (default: '.'). Display the type of information\n"
- " specified by the --show-revs option. If --show-revs isn't passed,\n"
- " it defaults to --show-revs='merged'.\n"
+ " 1. Display information related to merges (or potential merges) between\n"
+ " SOURCE and TARGET (default: '.'). Display the type of information\n"
+ " specified by the --show-revs option. If --show-revs isn't passed,\n"
+ " it defaults to --show-revs='merged'.\n"
+ " The depth can be 'empty' or 'infinity'; the default is 'empty'.\n"
"\n"
- " The depth can be 'empty' or 'infinity'; the default is 'empty'.\n"),
- {'r', 'R', opt_depth, opt_show_revs} },
+ " 2. Attempt to elide merge information within the subtree rooted at PATH\n"
+ " (default '.'). This removes redundant svn:mergeinfo properties from\n"
+ " children whose parents record a superset of the same merge information.\n"
+ " The depth can be any valid depth value; the default is 'infinity'.\n"),
+ {'r', 'R', opt_depth, opt_show_revs, opt_elide} },
{ "mkdir", svn_cl__mkdir, {0}, N_
("Create a new directory under version control.\n"
@@ -1525,7 +1532,7 @@ main(int argc, const char *argv[])
opt_state.depth = svn_depth_unknown;
opt_state.set_depth = svn_depth_unknown;
opt_state.accept_which = svn_cl__accept_unspecified;
- opt_state.show_revs = svn_cl__show_revs_merged;
+ opt_state.show_revs = svn_cl__show_revs_invalid;
/* No args? Show usage. */
if (argc <= 1)
@@ -2030,6 +2037,9 @@ main(int argc, const char *argv[])
case opt_allow_mixed_revisions:
opt_state.allow_mixed_rev = TRUE;
break;
+ case opt_elide:
+ opt_state.elide_mergeinfo = TRUE;
+ break;
default:
/* Hmmm. Perhaps this would be a good place to squirrel away
opts that commands like svn diff might need. Hmmm indeed. */
@@ -2222,6 +2232,21 @@ main(int argc, const char *argv[])
return svn_cmdline_handle_exit_error(err, pool, "svn: ");
}
+ /* Disallow simultaneous use of both --show-revs and --elide. */
+ if (opt_state.elide_mergeinfo &&
+ opt_state.show_revs != svn_cl__show_revs_invalid)
+ {
+ err = svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL,
+ _("--show-revs and --elide are mutually "
+ "exclusive"));
+ return svn_cmdline_handle_exit_error(err, pool, "svn: ");
+ }
+ else if (opt_state.show_revs == svn_cl__show_revs_invalid)
+ {
+ /* Set the default value of --show-revs. */
+ opt_state.show_revs = svn_cl__show_revs_merged;
+ }
+
/* Ensure that 'revision_ranges' has at least one item, and make
'start_revision' and 'end_revision' match that item. */
if (opt_state.revision_ranges->nelts == 0)
Index: subversion/libsvn_client/mergeinfo.c
===================================================================
--- subversion/libsvn_client/mergeinfo.c (revision 1167379)
+++ subversion/libsvn_client/mergeinfo.c (working copy)
@@ -840,9 +840,9 @@ should_elide_mergeinfo(svn_boolean_t *elides,
else
path_tweaked_parent_mergeinfo = parent_mergeinfo;
- SVN_ERR(svn_mergeinfo__equals(elides,
- path_tweaked_parent_mergeinfo,
- child_mergeinfo, TRUE, subpool));
+ SVN_ERR(svn_mergeinfo_remove2(&child_mergeinfo, parent_mergeinfo,
+ child_mergeinfo, TRUE, subpool, subpool));
+ *elides = (apr_hash_count(child_mergeinfo) == 0);
svn_pool_destroy(subpool);
}
@@ -2255,3 +2255,64 @@ svn_client__mergeinfo_status(svn_boolean_t *mergei
return SVN_NO_ERROR;
}
+
+/* Baton for elide_subtree_mergeinfo_cb() and
+ * elide_subtree_mergeinfo_locked(). */
+struct elide_subtree_mergeinfo_baton {
+ const char *local_abspath;
+ svn_client_ctx_t *ctx;
+ svn_depth_t depth;
+};
+
+/* Implements svn_wc__proplist_receiver_t. */
+static svn_error_t *
+elide_subtree_mergeinfo_cb(void *baton,
+ const char *local_abspath,
+ apr_hash_t *props,
+ apr_pool_t *scratch_pool)
+{
+ struct elide_subtree_mergeinfo_baton *b = baton;
+
+ SVN_ERR(svn_client__elide_mergeinfo(local_abspath,
+ b->local_abspath,
+ b->ctx, scratch_pool));
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+elide_subtree_mergeinfo_locked(void *baton,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ struct elide_subtree_mergeinfo_baton *b = baton;
+
+ /* Use proplist to find nodes with mergeinfo, and try to elide
+ * mergeinfo on each. */
+ SVN_ERR(svn_wc__prop_list_recursive(b->ctx->wc_ctx, b->local_abspath,
+ SVN_PROP_MERGEINFO,
+ b->depth, FALSE, FALSE, NULL,
+ elide_subtree_mergeinfo_cb, baton,
+ b->ctx->cancel_func,
+ b->ctx->cancel_baton,
+ scratch_pool));
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_client_elide_subtree_mergeinfo(const char *local_abspath,
+ svn_depth_t depth,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *scratch_pool)
+{
+ struct elide_subtree_mergeinfo_baton baton;
+
+ baton.local_abspath = local_abspath;
+ baton.depth = depth;
+ baton.ctx = ctx;
+
+ SVN_ERR(svn_wc__call_with_write_lock(elide_subtree_mergeinfo_locked,
+ &baton, ctx->wc_ctx,
+ local_abspath, FALSE,
+ scratch_pool, scratch_pool));
+ return SVN_NO_ERROR;
+}
Index: subversion/libsvn_client/mergeinfo.h
===================================================================
--- subversion/libsvn_client/mergeinfo.h (revision 1167379)
+++ subversion/libsvn_client/mergeinfo.h (working copy)
@@ -347,7 +347,7 @@ svn_client__record_wc_mergeinfo(const char *local_
B) TARGET_WCPATH has empty mergeinfo and its nearest parent also
has empty mergeinfo.
- C) TARGET_WCPATH has the same mergeinfo as its nearest parent
+ C) TARGET_WCPATH has a subset of the mergeinfo as its nearest parent
when that parent's mergeinfo is adjusted for the path
difference between the two, e.g.:
@@ -355,9 +355,14 @@ svn_client__record_wc_mergeinfo(const char *local_
TARGET_WCPATH's mergeinfo = '/A/D/H:3'
TARGET_WCPATH nearest parent = A_COPY
Parent's mergeinfo = '/A:3'
- Path differece = 'D/H'
+ Path difference = 'D/H'
Parent's adjusted mergeinfo = '/A/D/H:3'
+ TARGET_WCPATH's mergeinfo is a subset of the nearest parent
+ mergeinfo if removing mergeinfo inherited from the parent,
+ with adjusted paths, from TARGET_WCPATH's mergeinfo results
+ in empty mergeinfo. See svn_mergeinfo_remove2().
+
If Elision occurs remove the svn:mergeinfo property from
TARGET_WCPATH. */
svn_error_t *