You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ju...@apache.org on 2012/03/20 18:15:23 UTC

svn commit: r1303016 - in /subversion/trunk/subversion: include/private/svn_client_private.h libsvn_client/merge.c svn/cl.h svn/main.c svn/merge-cmd.c

Author: julianfoad
Date: Tue Mar 20 17:15:22 2012
New Revision: 1303016

URL: http://svn.apache.org/viewvc?rev=1303016&view=rev
Log:
Start implementing 'symmetric merge'.

* subversion/include/private/svn_client_private.h
  (SVN_WITH_SYMMETRIC_MERGE): New defined macro, used just to mark sections
    of code that belong to this feature while it's still new.
  (svn_client__symmetric_merge_t): New structure type.
  (svn_client__find_symmetric_merge, svn_client__do_symmetric_merge): New
    functions.

* subversion/libsvn_client/merge.c
  (svn_client__symmetric_merge_t, source_and_target_t): New structures.
  (open_source_and_target, close_source_and_target,
   find_base_on_source, find_base_on_target,
   find_symmetric_merge, svn_client__find_symmetric_merge,
   do_symmetric_merge_locked, svn_client__do_symmetric_merge): New functions.

* subversion/svn/cl.h
  (svn_cl__opt_state_t): Add a '--symmetric' option.

* subversion/svn/main.c
  (svn_cl__options, svn_cl__cmd_table, main): Define and parse the
    '--symmetric' option.

* subversion/svn/merge-cmd.c
  (symmetric_merge): New function.
  (svn_cl__merge): Call it if the '--symmetric' option is given.

Modified:
    subversion/trunk/subversion/include/private/svn_client_private.h
    subversion/trunk/subversion/libsvn_client/merge.c
    subversion/trunk/subversion/svn/cl.h
    subversion/trunk/subversion/svn/main.c
    subversion/trunk/subversion/svn/merge-cmd.c

Modified: subversion/trunk/subversion/include/private/svn_client_private.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_client_private.h?rev=1303016&r1=1303015&r2=1303016&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_client_private.h (original)
+++ subversion/trunk/subversion/include/private/svn_client_private.h Tue Mar 20 17:15:22 2012
@@ -100,6 +100,51 @@ svn_client__wc_node_get_origin(const cha
                                apr_pool_t *result_pool,
                                apr_pool_t *scratch_pool);
 
+/* A macro to mark sections of code that belong to the 'symmetric merge'
+ * feature while it's still new. */
+#define SVN_WITH_SYMMETRIC_MERGE
+
+#ifdef SVN_WITH_SYMMETRIC_MERGE
+
+/* Details of a symmetric merge. */
+typedef struct svn_client__symmetric_merge_t svn_client__symmetric_merge_t;
+
+/* Find the information needed to merge all unmerged changes from a source
+ * branch into a target branch.  The information is the locations of the
+ * youngest common ancestor, merge base, and such like.
+ *
+ * Set *MERGE to the information needed to merge all unmerged changes
+ * (up to SOURCE_REVISION) from the source branch SOURCE_PATH_OR_URL @
+ * SOURCE_REVISION into the target WC at TARGET_WCPATH.
+ */
+svn_error_t *
+svn_client__find_symmetric_merge(svn_client__symmetric_merge_t **merge,
+                                 const char *source_path_or_url,
+                                 const svn_opt_revision_t *source_revision,
+                                 const char *target_wcpath,
+                                 svn_boolean_t allow_mixed_rev,
+                                 svn_client_ctx_t *ctx,
+                                 apr_pool_t *result_pool,
+                                 apr_pool_t *scratch_pool);
+
+/* Perform a symmetric merge.
+ *
+ * Merge according to MERGE into the WC at TARGET_WCPATH.
+ */
+svn_error_t *
+svn_client__do_symmetric_merge(const svn_client__symmetric_merge_t *merge,
+                               const char *target_wcpath,
+                               svn_depth_t depth,
+                               svn_boolean_t ignore_ancestry,
+                               svn_boolean_t force,
+                               svn_boolean_t record_only,
+                               svn_boolean_t dry_run,
+                               const apr_array_header_t *merge_options,
+                               svn_client_ctx_t *ctx,
+                               apr_pool_t *scratch_pool);
+
+#endif /* SVN_WITH_SYMMETRIC_MERGE */
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

Modified: subversion/trunk/subversion/libsvn_client/merge.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1303016&r1=1303015&r2=1303016&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/merge.c (original)
+++ subversion/trunk/subversion/libsvn_client/merge.c Tue Mar 20 17:15:22 2012
@@ -10864,3 +10864,409 @@ svn_client_merge_peg4(const char *source
 
   return SVN_NO_ERROR;
 }
+
+#ifdef SVN_WITH_SYMMETRIC_MERGE
+
+/* Details of a merge. */
+struct svn_client__symmetric_merge_t
+{
+  repo_location_t *yca, *base, *mid, *right;
+};
+
+/* */
+typedef struct source_and_target_t
+{
+  repo_location_t *source;
+  svn_ra_session_t *source_ra_session;
+  merge_target_t *target;
+  svn_ra_session_t *target_ra_session;
+} source_and_target_t;
+
+/* "Open" the source and target branches of a merge.  That means:
+ *   - find out their exact repository locations (resolve WC paths and
+ *     non-numeric revision numbers),
+ *   - check the branches are suitably related,
+ *   - establish RA session(s) to the repo,
+ *   - check the WC for suitability (throw an error if unsuitable)
+ *
+ * Record this information and return it in a new "merge context" object.
+ */
+static svn_error_t *
+open_source_and_target(source_and_target_t **source_and_target,
+                       const char *source_path_or_url,
+                       const svn_opt_revision_t *source_peg_revision,
+                       const char *target_abspath,
+                       svn_boolean_t allow_mixed_rev,
+                       svn_client_ctx_t *ctx,
+                       apr_pool_t *session_pool,
+                       apr_pool_t *result_pool,
+                       apr_pool_t *scratch_pool)
+{
+  source_and_target_t *s_t = apr_palloc(result_pool, sizeof(*s_t));
+
+  /* Target */
+  SVN_ERR(open_target_wc(&s_t->target, target_abspath,
+                         allow_mixed_rev, TRUE, TRUE,
+                         ctx, result_pool, scratch_pool));
+  SVN_ERR(svn_client_open_ra_session(&s_t->target_ra_session,
+                                     s_t->target->loc.url,
+                                     ctx, session_pool));
+
+  /* Source */
+  SVN_ERR(open_source_session(&s_t->source, &s_t->source_ra_session,
+                              source_path_or_url, source_peg_revision,
+                              ctx, result_pool, scratch_pool));
+
+  *source_and_target = s_t;
+  return SVN_NO_ERROR;
+}
+
+/* "Close" any resources that were acquired in the S_T structure. */
+static svn_error_t *
+close_source_and_target(source_and_target_t *s_t,
+                        apr_pool_t *scratch_pool)
+{
+  /* close s_t->source_/target_ra_session */
+  return SVN_NO_ERROR;
+}
+
+/* Find a merge base location on the target branch, like in a sync
+ * merge.
+ *
+ *          (Source-left) (Source-right)
+ *                BASE        RIGHT
+ *          o-------o-----------o---
+ *         /         \           \
+ *   -----o     prev. \           \  this
+ *     YCA \    merge  \           \ merge
+ *          o-----------o-----------o
+ *                                TARGET
+ *
+ */
+static svn_error_t *
+find_base_on_source(repo_location_t **base_p,
+                    source_and_target_t *s_t,
+                    svn_client_ctx_t *ctx,
+                    apr_pool_t *result_pool,
+                    apr_pool_t *scratch_pool)
+{
+  *base_p = NULL;
+  return SVN_NO_ERROR;
+}
+
+/* Find a merge base location on the target branch, like in a reintegrate
+ * merge.
+ * 
+ *                     MID    RIGHT
+ *          o-----------o-------o---
+ *         /    prev.  /         \
+ *   -----o     merge /           \  this
+ *     YCA \         /             \ merge
+ *          o-------o---------------o
+ *                BASE            TARGET
+ *
+ * Set *BASE_P to the latest location on the history of S_T->target at
+ * which all revisions up to *BASE_P are recorded as merged into RIGHT
+ * (which is S_T->source).
+ * 
+ * ### TODO: Set *MID_P to the first location on the history of
+ * S_T->source at which all revisions up to BASE_P are recorded as merged.
+ */
+static svn_error_t *
+find_base_on_target(repo_location_t **base_p,
+                    repo_location_t **mid_p,
+                    source_and_target_t *s_t,
+                    svn_client_ctx_t *ctx,
+                    apr_pool_t *result_pool,
+                    apr_pool_t *scratch_pool)
+{
+  repo_location_t *base = apr_palloc(result_pool, sizeof(*base));
+  svn_mergeinfo_t unmerged_to_source_mergeinfo_catalog;
+  svn_mergeinfo_t merged_to_source_mergeinfo_catalog;
+  apr_hash_t *subtrees_with_mergeinfo;
+
+  /* Find all the subtrees in TARGET_WCPATH that have explicit mergeinfo. */
+  SVN_ERR(get_wc_explicit_mergeinfo_catalog(&subtrees_with_mergeinfo,
+                                            s_t->target->abspath,
+                                            svn_depth_infinity,
+                                            ctx, scratch_pool, scratch_pool));
+
+  SVN_ERR(calculate_left_hand_side(&base->url, &base->rev,
+                                   &merged_to_source_mergeinfo_catalog,
+                                   &unmerged_to_source_mergeinfo_catalog,
+                                   s_t->target,
+                                   subtrees_with_mergeinfo,
+                                   s_t->source,
+                                   s_t->source_ra_session,
+                                   s_t->target_ra_session,
+                                   ctx, result_pool, scratch_pool));
+
+  if (base->url)
+    {
+      *base_p = base;
+      *mid_p = s_t->source;  /* ### WRONG! This is quite difficult. */
+    }
+  else
+    {
+      *base_p = NULL;
+      *mid_p = NULL;
+    }
+
+  return SVN_NO_ERROR;
+}
+
+/* */
+static svn_error_t *
+find_symmetric_merge(repo_location_t **yca_p,
+                     repo_location_t **base_p,
+                     repo_location_t **mid_p,
+                     source_and_target_t *s_t,
+                     svn_client_ctx_t *ctx,
+                     apr_pool_t *result_pool,
+                     apr_pool_t *scratch_pool)
+{
+  repo_location_t *yca, *base_on_source, *base_on_target, *mid;
+
+  yca = apr_palloc(result_pool, sizeof(*yca));
+  SVN_ERR(svn_client__get_youngest_common_ancestor(
+            NULL, &yca->url, &yca->rev,
+            s_t->source->url, s_t->source->rev,
+            s_t->target->loc.url, s_t->target->loc.rev,
+            ctx, result_pool));
+  *yca_p = yca;
+
+  /* Find the latest revision of A synced to B and the latest
+   * revision of B synced to A.
+   *
+   *   base_on_source = youngest_complete_synced_point(source, target)
+   *   base_on_target = youngest_complete_synced_point(target, source)
+   */
+  SVN_ERR(find_base_on_source(&base_on_source, s_t,
+                              ctx, scratch_pool, scratch_pool));
+  SVN_ERR(find_base_on_target(&base_on_target, &mid, s_t,
+                              ctx, scratch_pool, scratch_pool));
+
+  if (base_on_source)
+    SVN_DBG(("base on source: %s@%ld\n", base_on_source->url, base_on_source->rev));
+  if (base_on_target)
+    SVN_DBG(("base on target: %s@%ld\n", base_on_target->url, base_on_target->rev));
+
+  /* Choose a base. */
+  if (base_on_source
+      && (! base_on_target || (base_on_source->rev > base_on_target->rev)))
+    {
+      *base_p = base_on_source;
+      *mid_p = NULL;
+    }
+  else if (base_on_target)
+    {
+      *base_p = base_on_target;
+      *mid_p = mid;
+    }
+  else
+    {
+      /* No previous merge was found, so this is the simple case where
+       * the base is the youngest common ancestor of the branches.  We'll
+       * set MID=NULL; in theory the end result should be the same if we
+       * set MID=YCA instead. */
+      *base_p = yca;
+      *mid_p = NULL;
+    }
+
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_client__find_symmetric_merge(svn_client__symmetric_merge_t **merge_p,
+                                 const char *source_path_or_url,
+                                 const svn_opt_revision_t *source_revision,
+                                 const char *target_wcpath,
+                                 svn_boolean_t allow_mixed_rev,
+                                 svn_client_ctx_t *ctx,
+                                 apr_pool_t *result_pool,
+                                 apr_pool_t *scratch_pool)
+{
+  const char *target_abspath;
+  source_and_target_t *s_t;
+  svn_client__symmetric_merge_t *merge = apr_palloc(result_pool, sizeof(*merge));
+
+  SVN_ERR(svn_dirent_get_absolute(&target_abspath, target_wcpath, scratch_pool));
+  SVN_ERR(open_source_and_target(&s_t, source_path_or_url, source_revision,
+                                 target_abspath, allow_mixed_rev,
+                                 ctx, result_pool, result_pool, scratch_pool));
+
+  /* Check source is in same repos as target. */
+  SVN_ERR(check_same_repos(s_t->source->repo, source_path_or_url,
+                           s_t->target->loc.repo, target_wcpath,
+                           TRUE /* strict_urls */, scratch_pool));
+
+  SVN_ERR(find_symmetric_merge(&merge->yca, &merge->base, &merge->mid, s_t,
+                               ctx, result_pool, scratch_pool));
+  merge->right = s_t->source;
+
+  /* Identify cherry-picks.
+   */
+
+  /* Break into 3-way merges, skipping the cherry-picks.
+   */
+
+  /* Mergeinfo addition.
+   */
+
+  /* Report a list of merge_souce_t merges.
+   * ### Just one, for now. */
+  /*
+  {
+    *merge_source = apr_palloc(result_pool, sizeof(**merge_source));
+    merge_source->url1 = base->url;
+    merge_source->rev1 = base->rev;
+    merge_source->url2 = s_t->source->url;
+    merge_source->rev2 = s_t->source->rev;
+  }
+  */
+
+  *merge_p = merge;
+
+  SVN_ERR(close_source_and_target(s_t, scratch_pool));
+
+  return SVN_NO_ERROR;
+}
+
+/* The body of svn_client__do_symmetric_merge(), which see.
+ *
+ * Five locations are inputs: YCA, BASE, MID, RIGHT, TARGET, as shown
+ * depending on whether the base is on the source branch or the target
+ * branch of this merge.
+ *
+ *                     MID    RIGHT
+ *          o-----------o-------o---
+ *         /    prev.  /         \
+ *   -----o     merge /           \  this
+ *     YCA \         /             \ merge
+ *          o-------o---------------o
+ *                BASE            TARGET
+ *
+ * or
+ *
+ *                BASE        RIGHT      (and MID=NULL)
+ *          o-------o-----------o---
+ *         /         \           \
+ *   -----o     prev. \           \  this
+ *     YCA \    merge  \           \ merge
+ *          o-----------o-----------o
+ *                                TARGET
+ *
+ *
+ */
+static svn_error_t *
+do_symmetric_merge_locked(const svn_client__symmetric_merge_t *merge,
+                          const char *target_abspath,
+                          svn_depth_t depth,
+                          svn_boolean_t ignore_ancestry,
+                          svn_boolean_t force,
+                          svn_boolean_t record_only,
+                          svn_boolean_t dry_run,
+                          const apr_array_header_t *merge_options,
+                          svn_client_ctx_t *ctx,
+                          apr_pool_t *scratch_pool)
+{
+  merge_target_t *target;
+  merge_source_t *source = apr_palloc(scratch_pool, sizeof(*source));
+  svn_boolean_t use_sleep = FALSE;
+  svn_error_t *err;
+
+  SVN_ERR(open_target_wc(&target, target_abspath, TRUE, TRUE, TRUE,
+                         ctx, scratch_pool, scratch_pool));
+
+  /* Do the real merge.  We know the merge sources are in the same repos
+   * as the target, and are related, but not necessarily 'ancestral'. */
+  /* ### Using DO_MERGE at the moment, just because it's convenient.  In
+   *     fact it's overkill because it does merge tracking -- eliminating
+   *     already-merged changes -- which we've already done and don't need
+   *     here. */
+  source->url1 = merge->base->url;
+  source->rev1 = merge->base->rev;
+  source->url2 = merge->right->url;
+  source->rev2 = merge->right->rev;
+  SVN_DBG(("yca   %s@%ld\n", merge->yca->url, merge->yca->rev));
+  SVN_DBG(("base  %s@%ld\n", merge->base->url, merge->base->rev));
+  if (merge->mid)
+    SVN_DBG(("mid   %s@%ld\n", merge->mid->url, merge->mid->rev));
+  SVN_DBG(("right %s@%ld\n", merge->right->url, merge->right->rev));
+
+  if (merge->mid)
+    {
+      svn_ra_session_t *ra_session = NULL;
+
+      SVN_ERR(ensure_ra_session_url(&ra_session, source->url1,
+                                    ctx, scratch_pool));
+
+      err = merge_cousins_and_supplement_mergeinfo(target,
+                                                   ra_session, ra_session,
+                                                   source, merge->yca->rev,
+                                                   TRUE /* same_repos */,
+                                                   svn_depth_infinity,
+                                                   FALSE /* ignore_ancestry */,
+                                                   FALSE /* force */,
+                                                   FALSE /* record_only */,
+                                                   dry_run,
+                                                   merge_options, &use_sleep,
+                                                   ctx, scratch_pool);
+
+    }
+  else
+    {
+      apr_array_header_t *merge_sources;
+
+      merge_sources = apr_array_make(scratch_pool, 1, sizeof(merge_source_t *));
+      APR_ARRAY_PUSH(merge_sources, const merge_source_t *) = source;
+
+      err = do_merge(NULL, NULL, merge_sources, target,
+                     TRUE /*sources_ancestral*/, TRUE /*related*/,
+                     TRUE /*same_repos*/, ignore_ancestry, force, dry_run,
+                     record_only, NULL, FALSE, FALSE, depth, merge_options,
+                     &use_sleep, ctx, scratch_pool, scratch_pool);
+    }
+
+  if (use_sleep)
+    svn_io_sleep_for_timestamps(target_abspath, scratch_pool);
+
+  SVN_ERR(err);
+
+  return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_client__do_symmetric_merge(const svn_client__symmetric_merge_t *merge,
+                               const char *target_wcpath,
+                               svn_depth_t depth,
+                               svn_boolean_t ignore_ancestry,
+                               svn_boolean_t force,
+                               svn_boolean_t record_only,
+                               svn_boolean_t dry_run,
+                               const apr_array_header_t *merge_options,
+                               svn_client_ctx_t *ctx,
+                               apr_pool_t *pool)
+{
+  const char *target_abspath, *lock_abspath;
+
+  SVN_ERR(get_target_and_lock_abspath(&target_abspath, &lock_abspath,
+                                      target_wcpath, ctx, pool));
+
+  if (!dry_run)
+    SVN_WC__CALL_WITH_WRITE_LOCK(
+      do_symmetric_merge_locked(merge,
+                                target_abspath, depth, ignore_ancestry,
+                                force, record_only, dry_run,
+                                merge_options, ctx, pool),
+      ctx->wc_ctx, lock_abspath, FALSE /* lock_anchor */, pool);
+  else
+    SVN_ERR(do_symmetric_merge_locked(merge,
+                                target_abspath, depth, ignore_ancestry,
+                                force, record_only, dry_run,
+                                merge_options, ctx, pool));
+
+  return SVN_NO_ERROR;
+}
+
+#endif /* SVN_WITH_SYMMETRIC_MERGE */

Modified: subversion/trunk/subversion/svn/cl.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/cl.h?rev=1303016&r1=1303015&r2=1303016&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/cl.h (original)
+++ subversion/trunk/subversion/svn/cl.h Tue Mar 20 17:15:22 2012
@@ -196,6 +196,7 @@ typedef struct svn_cl__opt_state_t
   const char *merge_cmd;         /* the external merge command to use */
   const char *editor_cmd;        /* the external editor command to use */
   svn_boolean_t record_only;     /* whether to record mergeinfo */
+  svn_boolean_t symmetric_merge; /* symmetric merge */
   const char *old_target;        /* diff target */
   const char *new_target;        /* diff target */
   svn_boolean_t relocate;        /* rewrite urls (svn switch) */

Modified: subversion/trunk/subversion/svn/main.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/main.c?rev=1303016&r1=1303015&r2=1303016&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/main.c (original)
+++ subversion/trunk/subversion/svn/main.c Tue Mar 20 17:15:22 2012
@@ -127,6 +127,7 @@ typedef enum svn_cl__longopt_t {
   opt_use_patch_diff_format,
   opt_allow_mixed_revisions,
   opt_include_externals,
+  opt_symmetric,
 } svn_cl__longopt_t;
 
 
@@ -366,6 +367,8 @@ const apr_getopt_option_t svn_cl__option
                        "recursion. This does not include externals with a\n"
                        "                             "
                        "fixed revision. (See the svn:externals property)")},
+  {"symmetric", opt_symmetric, 0,
+                       N_("Symmetric merge")},
 
   /* Long-opt Aliases
    *
@@ -1014,7 +1017,7 @@ const svn_opt_subcommand_desc2_t svn_cl_
 "  repositories.\n"),
     {'r', 'c', 'N', opt_depth, 'q', opt_force, opt_dry_run, opt_merge_cmd,
      opt_record_only, 'x', opt_ignore_ancestry, opt_accept, opt_reintegrate,
-     opt_allow_mixed_revisions} },
+     opt_allow_mixed_revisions, opt_symmetric} },
 
   { "mergeinfo", svn_cl__mergeinfo, {0}, N_
     ("Display merge-related information.\n"
@@ -1951,6 +1954,9 @@ main(int argc, const char *argv[])
       case opt_record_only:
         opt_state.record_only = TRUE;
         break;
+      case opt_symmetric:
+        opt_state.symmetric_merge = TRUE;
+        break;
       case opt_editor_cmd:
         opt_state.editor_cmd = apr_pstrdup(pool, opt_arg);
         break;

Modified: subversion/trunk/subversion/svn/merge-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/merge-cmd.c?rev=1303016&r1=1303015&r2=1303016&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/merge-cmd.c (original)
+++ subversion/trunk/subversion/svn/merge-cmd.c Tue Mar 20 17:15:22 2012
@@ -33,6 +33,7 @@
 #include "svn_error.h"
 #include "svn_types.h"
 #include "cl.h"
+#include "private/svn_client_private.h"
 
 #include "svn_private_config.h"
 
@@ -97,6 +98,40 @@ ensure_wc_path_has_repo_revision(const c
   return SVN_NO_ERROR;
 }
 
+#ifdef SVN_WITH_SYMMETRIC_MERGE
+/* Symmetric, merge-tracking merge, used for sync or reintegrate purposes. */
+static svn_error_t *
+symmetric_merge(const char *source_path_or_url,
+                const svn_opt_revision_t *source_revision,
+                const char *target_wcpath,
+                svn_depth_t depth,
+                svn_boolean_t ignore_ancestry,
+                svn_boolean_t force,
+                svn_boolean_t record_only,
+                svn_boolean_t dry_run,
+                svn_boolean_t allow_mixed_rev,
+                const apr_array_header_t *merge_options,
+                svn_client_ctx_t *ctx,
+                apr_pool_t *scratch_pool)
+{
+  svn_client__symmetric_merge_t *merge;
+
+  /* Find the 3-way merges needed (and check suitability of the WC). */
+  SVN_ERR(svn_client__find_symmetric_merge(&merge,
+                                           source_path_or_url, source_revision,
+                                           target_wcpath, allow_mixed_rev,
+                                           ctx, scratch_pool, scratch_pool));
+
+  /* Perform the 3-way merges */
+  SVN_ERR(svn_client__do_symmetric_merge(merge, target_wcpath, depth,
+                                         ignore_ancestry, force, record_only,
+                                         dry_run, merge_options,
+                                         ctx, scratch_pool));
+
+  return SVN_NO_ERROR;
+}
+#endif
+
 /* This implements the `svn_opt_subcommand_t' interface. */
 svn_error_t *
 svn_cl__merge(apr_getopt_t *os,
@@ -346,6 +381,29 @@ svn_cl__merge(apr_getopt_t *os,
                                   "with --reintegrate"));
     }
 
+#ifdef SVN_WITH_SYMMETRIC_MERGE
+  if (opt_state->symmetric_merge)
+    {
+      if (two_sources_specified || opt_state->reintegrate)
+        return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
+                                _("--reintegrate and SOURCE2 can't be used "
+                                  "with --symmetric"));
+      SVN_ERR_W(svn_cl__check_related_source_and_target(
+                  sourcepath1, &peg_revision1, targetpath, &unspecified,
+                  ctx, pool),
+                _("Source and target must be different but related branches"));
+
+      err = symmetric_merge(sourcepath1, &peg_revision1, targetpath,
+                            opt_state->depth,
+                            opt_state->ignore_ancestry,
+                            opt_state->force,
+                            opt_state->record_only,
+                            opt_state->dry_run,
+                            opt_state->allow_mixed_rev,
+                            options, ctx, pool);
+    }
+  else
+#endif
   if (opt_state->reintegrate)
     {
       SVN_ERR_W(svn_cl__check_related_source_and_target(



Re: svn commit: r1303016 - in /subversion/trunk/subversion: include/private/svn_client_private.h libsvn_client/merge.c svn/cl.h svn/main.c svn/merge-cmd.c

Posted by Julian Foad <ju...@btopenworld.com>.
Branko Čibej wrote:

> Julian Foad wrote:
>>  Yes, that's true for a criss-cross.  However, it's not a problem
>> for  normal cases; criss-cross is a rare case.  As I wrote in the
>>  criss-cross merge section of
>>  <http://wiki.apache.org/subversion/SymmetricMerge>, in that case we
>>  probably should consider the relative ages of A1, B1, A3, B3, and A2,
>>  but I haven't yet thought about what's the best way to compare them. 
> 
> Isn't the age inversely proportional to the revision number?

Inversely linear with the revision number, maybe.  But that's not the point.

There are five variables and we need one answer.

function which_of_the_two_bases_to_use(A1, B1, A3, B3, A2) => boolean

- Julian

Re: svn commit: r1303016 - in /subversion/trunk/subversion: include/private/svn_client_private.h libsvn_client/merge.c svn/cl.h svn/main.c svn/merge-cmd.c

Posted by Branko Čibej <br...@apache.org>.
On 22.03.2012 09:54, Julian Foad wrote:
> Yes, that's true for a criss-cross.  However, it's not a problem for
> normal cases; criss-cross is a rare case.  As I wrote in the
> criss-cross merge section of
> <http://wiki.apache.org/subversion/SymmetricMerge>, in that case we
> probably should consider the relative ages of A1, B1, A3, B3, and A2,
> but I haven't yet thought about what's the best way to compare them. 

Isn't the age inversely proportional to the revision number?

-- Brane


Re: svn commit: r1303016 - in /subversion/trunk/subversion: include/private/svn_client_private.h libsvn_client/merge.c svn/cl.h svn/main.c svn/merge-cmd.c

Posted by Julian Foad <ju...@btopenworld.com>.
Daniel Shahaf wrote:

> Julian Foad wrote:
>>  IGNORE_ANCESTRY doesn't affect the high level operation of the merge, 
>> it only affects how file diffs are shown -- even if the source and 
>>  target file are not historically related it will show a diff rather than
>> a delete and an add of the file -- or something similar to that.  From 
>> svn_client_merge4():
>> 
>>   * Use @a ignore_ancestry to control whether or not items being
>>   * diffed will be checked for relatedness first.  Unrelated items
>>   * are typically transmitted to the editor as a deletion of one thing
>>   * and the addition of another, but if this flag is TRUE, unrelated
>>   * items will be diffed as if they were related.
> 
> So, IGNORE_ANCESTRY controls how the tree delta is communicated.  Okay.
> I was going by `svh help`, and the first mention there was effectively
> "Ignore mergeinfo on the source when computing the merge" -- hence my
> question.

I had to check the code to confirm this.  I can report that the IGNORE_ANCESTRY option ignores mergeinfo (in addition to affecting the diffing of files) for some merges, specifically merges which call the 2-URL merge API "svn_client_merge4()".  It doesn't cause mergeinfo to be ignored in the "pegged merge" API which is used for sync merges, nor in the "reintegrate merge" API, nor in the "symmetric merge" API.

- Julian

Re: svn commit: r1303016 - in /subversion/trunk/subversion: include/private/svn_client_private.h libsvn_client/merge.c svn/cl.h svn/main.c svn/merge-cmd.c

Posted by Daniel Shahaf <da...@elego.de>.
Julian Foad wrote on Thu, Mar 22, 2012 at 08:54:09 +0000:
> Daniel Shahaf wrote:
> > julianfoad@apache.org wrote:
> >>  http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_client_private.h?rev=1303016&r1=1303015&r2=1303016&view=diff
> > ==============================================================================
> >>  --- subversion/trunk/subversion/include/private/svn_client_private.h
> >>  +++ subversion/trunk/subversion/include/private/svn_client_private.h
> >>  +/* Perform a symmetric merge.
> >>  + *
> >>  + * Merge according to MERGE into the WC at TARGET_WCPATH.
> >>  + */
> >>  +svn_error_t *
> >>  +svn_client__do_symmetric_merge(const svn_client__symmetric_merge_t *merge,
> >>  +                               const char *target_wcpath,
> >>  +                               svn_depth_t depth,
> >>  +                               svn_boolean_t ignore_ancestry,
> > 
> > What does IGNORE_ANCESTRY mean in the context of symmetric merge?  In
> > particular, is it meaningful for the second merge in a 'sync A->B,
> > sync A->B' scenario?
> 
> Clearly I need to fill in the doc strings.
> 
> IGNORE_ANCESTRY doesn't affect the high level operation of the merge, it only affects how file diffs are shown -- even if the source and 
> target file are not historically related it will show a diff rather than
>  a delete and an add of the file -- or something similar to that.  From svn_client_merge4():
> 
>  * Use @a ignore_ancestry to control whether or not items being
>  * diffed will be checked for relatedness first.  Unrelated items
>  * are typically transmitted to the editor as a deletion of one thing
>  * and the addition of another, but if this flag is TRUE, unrelated
>  * items will be diffed as if they were related.
> 

So, IGNORE_ANCESTRY controls how the tree delta is communicated.  Okay.
I was going by `svh help`, and the first mention there was effectively
"Ignore mergeinfo on the source when computing the merge" -- hence my
question.

> >>  +                               svn_boolean_t force,
> >>  +                               svn_boolean_t record_only,
> >>  +                               svn_boolean_t dry_run,
> >>  +                               const apr_array_header_t *merge_options,
> >>  +                               svn_client_ctx_t *ctx,
> >>  +                               apr_pool_t *scratch_pool);
> 
> >> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1303016&r1=1303015&r2=1303016&view=diff
> > ==============================================================================
> >>  --- subversion/trunk/subversion/libsvn_client/merge.c
> >>  +++ subversion/trunk/subversion/libsvn_client/merge.c
> >>  @@ -10864,3 +10864,409 @@ 
> >>  +/* */
> >>  +static svn_error_t *
> >>  +find_symmetric_merge(repo_location_t **yca_p,
> >>  +                     repo_location_t **base_p,
> >>  +                     repo_location_t **mid_p,
> >>  +                     source_and_target_t *s_t,
> >>  +                     svn_client_ctx_t *ctx,
> >>  +                     apr_pool_t *result_pool,
> >>  +                     apr_pool_t *scratch_pool)
> >>  +{
> >>  +  repo_location_t *yca, *base_on_source, *base_on_target, *mid;
> >>  +
> >>  +  yca = apr_palloc(result_pool, sizeof(*yca));
> >>  +  SVN_ERR(svn_client__get_youngest_common_ancestor(
> >>  +            NULL, &yca->url, &yca->rev,
> >>  +            s_t->source->url, s_t->source->rev,
> >>  +            s_t->target->loc.url, s_t->target->loc.rev,
> >>  +            ctx, result_pool));
> >>  +  *yca_p = yca;
> >>  +
> >>  +  /* Find the latest revision of A synced to B and the latest
> >>  +   * revision of B synced to A.
> >>  +   *
> >>  +   *   base_on_source = youngest_complete_synced_point(source, target)
> >>  +   *   base_on_target = youngest_complete_synced_point(target, source)
> >>  +   */
> >>  +  SVN_ERR(find_base_on_source(&base_on_source, s_t,
> >>  +                              ctx, scratch_pool, scratch_pool));
> >>  +  SVN_ERR(find_base_on_target(&base_on_target, &mid, s_t,
> >>  +                              ctx, scratch_pool, scratch_pool));
> [...]
> >>  +  /* Choose a base. */
> >>  +  if (base_on_source
> >>  +      && (! base_on_target || (base_on_source->rev > base_on_target->rev)))
> >>  +    {
> > 
> > The last part of this condition seems arbitrary: in the criss-cross
> > scenario, the order in which the 'criss' and the 'cross' are 
> > committed shouldn't affect the base the algorithm chooses.
> 
> Yes, that's true for a criss-cross.  However, it's not a problem for
> normal cases; criss-cross is a rare case.  As I wrote in the
> criss-cross merge section of
> <http://wiki.apache.org/subversion/SymmetricMerge>, in that case we
> probably should consider the relative ages of A1, B1, A3, B3, and A2,
> but I haven't yet thought about what's the best way to compare them.
> 

In other words, work in progress.  Fair enough.

Cheers,

daniel

> >>  +      *base_p = base_on_source;
> >>  +      *mid_p = NULL;
> >>  +    }
> >>  +  else if (base_on_target)
> >>  +    {
> >>  +      *base_p = base_on_target;
> >>  +      *mid_p = mid;
> >>  +    }
> >>  +  else
> >>  +    {
> >>  +      /* No previous merge was found, so this is the simple case where
> >>  +       * the base is the youngest common ancestor of the branches.  We'll
> >>  +       * set MID=NULL; in theory the end result should be the same if we
> >>  +       * set MID=YCA instead. */
> >>  +      *base_p = yca;
> >>  +      *mid_p = NULL;
> >>  +    }
> >>  +
> >>  +  return SVN_NO_ERROR;
> >>  +}
> 
> Thanks.
> 
> - Julian

Re: svn commit: r1303016 - in /subversion/trunk/subversion: include/private/svn_client_private.h libsvn_client/merge.c svn/cl.h svn/main.c svn/merge-cmd.c

Posted by Julian Foad <ju...@btopenworld.com>.
Hi Daniel.  Thanks for reviewing...


(Dropping commits@ from the CC.)


Daniel Shahaf wrote:
> julianfoad@apache.org wrote:
>>  http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_client_private.h?rev=1303016&r1=1303015&r2=1303016&view=diff
> ==============================================================================
>>  --- subversion/trunk/subversion/include/private/svn_client_private.h
>>  +++ subversion/trunk/subversion/include/private/svn_client_private.h
>>  +/* Perform a symmetric merge.
>>  + *
>>  + * Merge according to MERGE into the WC at TARGET_WCPATH.
>>  + */
>>  +svn_error_t *
>>  +svn_client__do_symmetric_merge(const svn_client__symmetric_merge_t *merge,
>>  +                               const char *target_wcpath,
>>  +                               svn_depth_t depth,
>>  +                               svn_boolean_t ignore_ancestry,
> 
> What does IGNORE_ANCESTRY mean in the context of symmetric merge?  In
> particular, is it meaningful for the second merge in a 'sync A->B,
> sync A->B' scenario?

Clearly I need to fill in the doc strings.

IGNORE_ANCESTRY doesn't affect the high level operation of the merge, it only affects how file diffs are shown -- even if the source and 
target file are not historically related it will show a diff rather than
 a delete and an add of the file -- or something similar to that.  From svn_client_merge4():

 * Use @a ignore_ancestry to control whether or not items being
 * diffed will be checked for relatedness first.  Unrelated items
 * are typically transmitted to the editor as a deletion of one thing
 * and the addition of another, but if this flag is TRUE, unrelated
 * items will be diffed as if they were related.

>>  +                               svn_boolean_t force,
>>  +                               svn_boolean_t record_only,
>>  +                               svn_boolean_t dry_run,
>>  +                               const apr_array_header_t *merge_options,
>>  +                               svn_client_ctx_t *ctx,
>>  +                               apr_pool_t *scratch_pool);

>> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1303016&r1=1303015&r2=1303016&view=diff
> ==============================================================================
>>  --- subversion/trunk/subversion/libsvn_client/merge.c
>>  +++ subversion/trunk/subversion/libsvn_client/merge.c
>>  @@ -10864,3 +10864,409 @@ 
>>  +/* */
>>  +static svn_error_t *
>>  +find_symmetric_merge(repo_location_t **yca_p,
>>  +                     repo_location_t **base_p,
>>  +                     repo_location_t **mid_p,
>>  +                     source_and_target_t *s_t,
>>  +                     svn_client_ctx_t *ctx,
>>  +                     apr_pool_t *result_pool,
>>  +                     apr_pool_t *scratch_pool)
>>  +{
>>  +  repo_location_t *yca, *base_on_source, *base_on_target, *mid;
>>  +
>>  +  yca = apr_palloc(result_pool, sizeof(*yca));
>>  +  SVN_ERR(svn_client__get_youngest_common_ancestor(
>>  +            NULL, &yca->url, &yca->rev,
>>  +            s_t->source->url, s_t->source->rev,
>>  +            s_t->target->loc.url, s_t->target->loc.rev,
>>  +            ctx, result_pool));
>>  +  *yca_p = yca;
>>  +
>>  +  /* Find the latest revision of A synced to B and the latest
>>  +   * revision of B synced to A.
>>  +   *
>>  +   *   base_on_source = youngest_complete_synced_point(source, target)
>>  +   *   base_on_target = youngest_complete_synced_point(target, source)
>>  +   */
>>  +  SVN_ERR(find_base_on_source(&base_on_source, s_t,
>>  +                              ctx, scratch_pool, scratch_pool));
>>  +  SVN_ERR(find_base_on_target(&base_on_target, &mid, s_t,
>>  +                              ctx, scratch_pool, scratch_pool));
[...]
>>  +  /* Choose a base. */
>>  +  if (base_on_source
>>  +      && (! base_on_target || (base_on_source->rev > base_on_target->rev)))
>>  +    {
> 
> The last part of this condition seems arbitrary: in the criss-cross
> scenario, the order in which the 'criss' and the 'cross' are 
> committed shouldn't affect the base the algorithm chooses.

Yes, that's true for a criss-cross.  However, it's not a problem for normal cases; criss-cross is a rare case.  As I wrote in the criss-cross merge section of <http://wiki.apache.org/subversion/SymmetricMerge>, in that case we probably should consider the relative ages of A1, B1, A3, B3, and A2, but I haven't yet thought about what's the best way to compare them.

>>  +      *base_p = base_on_source;
>>  +      *mid_p = NULL;
>>  +    }
>>  +  else if (base_on_target)
>>  +    {
>>  +      *base_p = base_on_target;
>>  +      *mid_p = mid;
>>  +    }
>>  +  else
>>  +    {
>>  +      /* No previous merge was found, so this is the simple case where
>>  +       * the base is the youngest common ancestor of the branches.  We'll
>>  +       * set MID=NULL; in theory the end result should be the same if we
>>  +       * set MID=YCA instead. */
>>  +      *base_p = yca;
>>  +      *mid_p = NULL;
>>  +    }
>>  +
>>  +  return SVN_NO_ERROR;
>>  +}

Thanks.

- Julian

Re: svn commit: r1303016 - in /subversion/trunk/subversion: include/private/svn_client_private.h libsvn_client/merge.c svn/cl.h svn/main.c svn/merge-cmd.c

Posted by Daniel Shahaf <da...@elego.de>.
julianfoad@apache.org wrote on Tue, Mar 20, 2012 at 17:15:23 -0000:
> Modified: subversion/trunk/subversion/include/private/svn_client_private.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_client_private.h?rev=1303016&r1=1303015&r2=1303016&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/private/svn_client_private.h (original)
> +++ subversion/trunk/subversion/include/private/svn_client_private.h Tue Mar 20 17:15:22 2012
> @@ -100,6 +100,51 @@ 
> +svn_error_t *
> +svn_client__find_symmetric_merge(svn_client__symmetric_merge_t **merge,
> +                                 const char *source_path_or_url,
> +                                 const svn_opt_revision_t *source_revision,
> +                                 const char *target_wcpath,
> +                                 svn_boolean_t allow_mixed_rev,
> +                                 svn_client_ctx_t *ctx,
> +                                 apr_pool_t *result_pool,
> +                                 apr_pool_t *scratch_pool);
> +
> +/* Perform a symmetric merge.
> + *
> + * Merge according to MERGE into the WC at TARGET_WCPATH.
> + */
> +svn_error_t *
> +svn_client__do_symmetric_merge(const svn_client__symmetric_merge_t *merge,
> +                               const char *target_wcpath,
> +                               svn_depth_t depth,
> +                               svn_boolean_t ignore_ancestry,

What does IGNORE_ANCESTRY mean in the context of symmetric merge?  In
particular, is it meaningful for the second merge in a 'sync A->B,
sync A->B' scenario?

> +                               svn_boolean_t force,
> +                               svn_boolean_t record_only,
> +                               svn_boolean_t dry_run,
> +                               const apr_array_header_t *merge_options,
> +                               svn_client_ctx_t *ctx,
> +                               apr_pool_t *scratch_pool);
> +
> +#endif /* SVN_WITH_SYMMETRIC_MERGE */
> +
>  #ifdef __cplusplus
>  }
>  #endif /* __cplusplus */
> 
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1303016&r1=1303015&r2=1303016&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Tue Mar 20 17:15:22 2012
> @@ -10864,3 +10864,409 @@ 
> +/* */
> +static svn_error_t *
> +find_symmetric_merge(repo_location_t **yca_p,
> +                     repo_location_t **base_p,
> +                     repo_location_t **mid_p,
> +                     source_and_target_t *s_t,
> +                     svn_client_ctx_t *ctx,
> +                     apr_pool_t *result_pool,
> +                     apr_pool_t *scratch_pool)
> +{
> +  repo_location_t *yca, *base_on_source, *base_on_target, *mid;
> +
> +  yca = apr_palloc(result_pool, sizeof(*yca));
> +  SVN_ERR(svn_client__get_youngest_common_ancestor(
> +            NULL, &yca->url, &yca->rev,
> +            s_t->source->url, s_t->source->rev,
> +            s_t->target->loc.url, s_t->target->loc.rev,
> +            ctx, result_pool));
> +  *yca_p = yca;
> +
> +  /* Find the latest revision of A synced to B and the latest
> +   * revision of B synced to A.
> +   *
> +   *   base_on_source = youngest_complete_synced_point(source, target)
> +   *   base_on_target = youngest_complete_synced_point(target, source)
> +   */
> +  SVN_ERR(find_base_on_source(&base_on_source, s_t,
> +                              ctx, scratch_pool, scratch_pool));
> +  SVN_ERR(find_base_on_target(&base_on_target, &mid, s_t,
> +                              ctx, scratch_pool, scratch_pool));
> +
> +  if (base_on_source)
> +    SVN_DBG(("base on source: %s@%ld\n", base_on_source->url, base_on_source->rev));
> +  if (base_on_target)
> +    SVN_DBG(("base on target: %s@%ld\n", base_on_target->url, base_on_target->rev));
> +
> +  /* Choose a base. */
> +  if (base_on_source
> +      && (! base_on_target || (base_on_source->rev > base_on_target->rev)))
> +    {

The last part of this condition seems arbitrary: in the criss-cross
scenario, the order in which the 'criss' and the 'cross' are committed
shouldn't affect the base the algorithm chooses.

> +      *base_p = base_on_source;
> +      *mid_p = NULL;
> +    }
> +  else if (base_on_target)
> +    {
> +      *base_p = base_on_target;
> +      *mid_p = mid;
> +    }
> +  else
> +    {
> +      /* No previous merge was found, so this is the simple case where
> +       * the base is the youngest common ancestor of the branches.  We'll
> +       * set MID=NULL; in theory the end result should be the same if we
> +       * set MID=YCA instead. */
> +      *base_p = yca;
> +      *mid_p = NULL;
> +    }
> +
> +  return SVN_NO_ERROR;
> +}

Re: svn commit: r1303016 - in /subversion/trunk/subversion: include/private/svn_client_private.h libsvn_client/merge.c svn/cl.h svn/main.c svn/merge-cmd.c

Posted by Daniel Shahaf <da...@elego.de>.
julianfoad@apache.org wrote on Tue, Mar 20, 2012 at 17:15:23 -0000:
> Modified: subversion/trunk/subversion/include/private/svn_client_private.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_client_private.h?rev=1303016&r1=1303015&r2=1303016&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/private/svn_client_private.h (original)
> +++ subversion/trunk/subversion/include/private/svn_client_private.h Tue Mar 20 17:15:22 2012
> @@ -100,6 +100,51 @@ 
> +svn_error_t *
> +svn_client__find_symmetric_merge(svn_client__symmetric_merge_t **merge,
> +                                 const char *source_path_or_url,
> +                                 const svn_opt_revision_t *source_revision,
> +                                 const char *target_wcpath,
> +                                 svn_boolean_t allow_mixed_rev,
> +                                 svn_client_ctx_t *ctx,
> +                                 apr_pool_t *result_pool,
> +                                 apr_pool_t *scratch_pool);
> +
> +/* Perform a symmetric merge.
> + *
> + * Merge according to MERGE into the WC at TARGET_WCPATH.
> + */
> +svn_error_t *
> +svn_client__do_symmetric_merge(const svn_client__symmetric_merge_t *merge,
> +                               const char *target_wcpath,
> +                               svn_depth_t depth,
> +                               svn_boolean_t ignore_ancestry,

What does IGNORE_ANCESTRY mean in the context of symmetric merge?  In
particular, is it meaningful for the second merge in a 'sync A->B,
sync A->B' scenario?

> +                               svn_boolean_t force,
> +                               svn_boolean_t record_only,
> +                               svn_boolean_t dry_run,
> +                               const apr_array_header_t *merge_options,
> +                               svn_client_ctx_t *ctx,
> +                               apr_pool_t *scratch_pool);
> +
> +#endif /* SVN_WITH_SYMMETRIC_MERGE */
> +
>  #ifdef __cplusplus
>  }
>  #endif /* __cplusplus */
> 
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1303016&r1=1303015&r2=1303016&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Tue Mar 20 17:15:22 2012
> @@ -10864,3 +10864,409 @@ 
> +/* */
> +static svn_error_t *
> +find_symmetric_merge(repo_location_t **yca_p,
> +                     repo_location_t **base_p,
> +                     repo_location_t **mid_p,
> +                     source_and_target_t *s_t,
> +                     svn_client_ctx_t *ctx,
> +                     apr_pool_t *result_pool,
> +                     apr_pool_t *scratch_pool)
> +{
> +  repo_location_t *yca, *base_on_source, *base_on_target, *mid;
> +
> +  yca = apr_palloc(result_pool, sizeof(*yca));
> +  SVN_ERR(svn_client__get_youngest_common_ancestor(
> +            NULL, &yca->url, &yca->rev,
> +            s_t->source->url, s_t->source->rev,
> +            s_t->target->loc.url, s_t->target->loc.rev,
> +            ctx, result_pool));
> +  *yca_p = yca;
> +
> +  /* Find the latest revision of A synced to B and the latest
> +   * revision of B synced to A.
> +   *
> +   *   base_on_source = youngest_complete_synced_point(source, target)
> +   *   base_on_target = youngest_complete_synced_point(target, source)
> +   */
> +  SVN_ERR(find_base_on_source(&base_on_source, s_t,
> +                              ctx, scratch_pool, scratch_pool));
> +  SVN_ERR(find_base_on_target(&base_on_target, &mid, s_t,
> +                              ctx, scratch_pool, scratch_pool));
> +
> +  if (base_on_source)
> +    SVN_DBG(("base on source: %s@%ld\n", base_on_source->url, base_on_source->rev));
> +  if (base_on_target)
> +    SVN_DBG(("base on target: %s@%ld\n", base_on_target->url, base_on_target->rev));
> +
> +  /* Choose a base. */
> +  if (base_on_source
> +      && (! base_on_target || (base_on_source->rev > base_on_target->rev)))
> +    {

The last part of this condition seems arbitrary: in the criss-cross
scenario, the order in which the 'criss' and the 'cross' are committed
shouldn't affect the base the algorithm chooses.

> +      *base_p = base_on_source;
> +      *mid_p = NULL;
> +    }
> +  else if (base_on_target)
> +    {
> +      *base_p = base_on_target;
> +      *mid_p = mid;
> +    }
> +  else
> +    {
> +      /* No previous merge was found, so this is the simple case where
> +       * the base is the youngest common ancestor of the branches.  We'll
> +       * set MID=NULL; in theory the end result should be the same if we
> +       * set MID=YCA instead. */
> +      *base_p = yca;
> +      *mid_p = NULL;
> +    }
> +
> +  return SVN_NO_ERROR;
> +}