[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

now with more agressive elision (was: [PATCH] svn mergeinfo --elide)

From: Stefan Sperling <stsp_at_elego.de>
Date: Sat, 10 Sep 2011 11:43:32 +0200

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 *
Received on 2011-09-10 11:44:26 CEST

This is an archived mail posted to the Subversion Dev mailing list.