Hi, merge fans.
It's time to lose the temporary '--symmetric' option and make it the default
mode in 'svn'.
Some people have been kind enough to try using the 'symmetric' mode and report
back, mainly Paul to whom I am most grateful. I am aware of two current issues
([1],[2]) which I am investigating and they are looking simple enough to be
resolved soon. Please help me flag up any other issues.
Before we consider the details of the UI changes, I thought back to how the
user base would see 'symmetric merge' in the context of the history of
Subversion's merge tracking, and wrote a brief overview at
<http://wiki.apache.org/subversion/SymmetricMerge#The_User.27s_Perspective>.
UI CHANGES
I think, and please tell me if you disagree, that we will make the best
progression with a UI like this, which the attached patch [6] roughly
implements for illustration:
* Use the 'symmetric'/'auto'[3] mode for any merge that was previously a
reintegrate or a non-reintegrate 'sync' merge. That is, when there is:
- just one source;
- no start revision;
- (an end revision may be specified, which can be done without a start rev
by specifying peg-rev syntax: SOURCE@END_REV);
- no incompatible options (such as --ignore-ancestry).
* If the code chooses a reintegrate-like merge, it will apply the strict WC
checks like reintegrate does in 1.7.
* Make the '--reintegrate' option mean the user believes a reintegrate-style
merge is needed, so check that the previous merge is as expected (that it, in
the opposite direction) and bail out if not.
* Mark the '--reintegrate' option as deprecated, as the reason for its
existence has gone.
* Delete the '--symmetric' option; it was only there for testing and has not
been released.
WHAT WILL NOT CHANGE
Two specific things to call out:
* A merge specifying '--reintegrate' merge will not change in any scenario
where 'merge --reintegrate' is the user's best option today.
* A merge specifying '-r X:Y' will not change. This requests a cherry-pick
merge, using merge tracking to avoid re-merging any revs in the range that are
already merged. Currently, if 'X' is 1 or a low enough number such that there
are no unmerged revisions before 'X', then Subversion performs exactly the same
kind of merge as when 'X' is not specified, which we call a 'sync' merge. I
believe we could make a later enhancement to automatically use 'reintegrate'
mode if that would be the correct thing to do for merging the requested range.
OPTIONS NOT ALLOWED WITH --REINTEGRATE
The following options are not allowed with '--reintegrate' in 1.7 but are
allowed for non-reintegrate merge-tracking merges:
--record-only
--depth
--force
--allow-mixed-revisions
My recommendation for these options is that if we choose a reintegrate-style
merge then we error out, saying "the required merge is reintegrate-like[4], and
option --X is not supported for this kind of merge". Otherwise we handle the
option just like in 1.7.
Please let me know if this seems good, while I go and adjust the help text.
- Julian
[1] Unexpected revision number in the "Merging r6 through r9" notification,
<http://svn.haxx.se/dev/archive-2012-08/0005.shtml>.
[2] Behaviour when the root has not been merged, a subtree has, and now we
merge the root in the other direction,
<http://svn.haxx.se/dev/archive-2012-08/0008.shtml>.
[3] Every time I say 'symmetric', I try to think of a better name. It's not
symmetric, it's just based on a symmetric model. 'automatic'? 'bidirectional'?
[4] Need to choose terminology for a reintegrate-style merge, for the UI.
"reversing direction"?
[5] Wiki page, <http://wiki.apache.org/subversion/SymmetricMerge>.
[6] Attachment, 'enable-symmetric-merge-1.patch'.
[7] GUIs and other clients can switch to the new svn_client API when
they are ready to do so; here I'm just talking about our 'svn' client.
--
Subversion Live 2012 - 2 Days: training, networking, demos & more!
http://bit.ly/NgUaRi
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Enable symmetric merge as the default merge in 'svn'. If the --reintegrate
option is given but a non-reintegrate merge is the kind needed, bail out.
Stop recognizing the '--symmetric' option, as that was a temporary
development tool.
### This patch should be extended to delete the '--symmetric' option and to
### update the merge help text.
* subversion/include/private/svn_client_private.h
(svn_client__symmetric_merge_t): Move structure definition to here ...
* subversion/libsvn_client/merge.c
(svn_client__symmetric_merge_t): ... from here.
(do_symmetric_merge_locked): Reject unsupported options in reintegrate
mode.
* subversion/svn/merge-cmd.c
(pathrev_equal): New function.
(symmetric_merge): Bail out if a non-reintegrate kind of merge is needed
but the '--reintegrate' option was specified. Reject unsupported options
in reintegrate mode.
(svn_cl__merge):
--This line, and those below, will be ignored--
Index: subversion/include/private/svn_client_private.h
===================================================================
--- subversion/include/private/svn_client_private.h (revision 1368758)
+++ subversion/include/private/svn_client_private.h (working copy)
@@ -185,7 +185,10 @@ svn_client__wc_node_get_origin(svn_clien
#ifdef SVN_WITH_SYMMETRIC_MERGE
/* Details of a symmetric merge. */
-typedef struct svn_client__symmetric_merge_t svn_client__symmetric_merge_t;
+typedef struct svn_client__symmetric_merge_t
+{
+ svn_client__pathrev_t *yca, *base, *mid, *right;
+} 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
Index: subversion/libsvn_client/merge.c
===================================================================
--- subversion/libsvn_client/merge.c (revision 1368758)
+++ subversion/libsvn_client/merge.c (working copy)
@@ -11149,12 +11149,6 @@ location_on_branch_at_rev(const branch_h
return NULL;
}
-/* Details of a symmetric merge. */
-struct svn_client__symmetric_merge_t
-{
- svn_client__pathrev_t *yca, *base, *mid, *right;
-};
-
/* */
typedef struct source_and_target_t
{
@@ -11654,6 +11648,24 @@ do_symmetric_merge_locked(const svn_clie
svn_ra_session_t *right_ra_session = NULL;
svn_ra_session_t *target_ra_session = NULL;
+ if (record_only)
+ return svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL,
+ _("The required merge is reintegrate-like, "
+ "and the record-only option "
+ "cannot be used with this kind of merge"));
+
+ if (depth != svn_depth_unknown)
+ return svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL,
+ _("The required merge is reintegrate-like, "
+ "and the depth option "
+ "cannot be used with this kind of merge"));
+
+ if (force)
+ return svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL,
+ _("The required merge is reintegrate-like, "
+ "and the force option "
+ "cannot be used with this kind of merge"));
+
SVN_ERR(ensure_ra_session_url(&base_ra_session, merge->base->url,
ctx, scratch_pool));
SVN_ERR(ensure_ra_session_url(&right_ra_session, merge->right->url,
Index: subversion/svn/merge-cmd.c
===================================================================
--- subversion/svn/merge-cmd.c (revision 1368758)
+++ subversion/svn/merge-cmd.c (working copy)
@@ -85,6 +85,15 @@ merge_reintegrate(const char *source_pat
return SVN_NO_ERROR;
}
+/* */
+static svn_boolean_t
+pathrev_equal(const svn_client__pathrev_t *pathrev1,
+ const svn_client__pathrev_t *pathrev2)
+{
+ return (pathrev1->rev == pathrev2->rev
+ && strcmp(pathrev1->url, pathrev2->url) == 0);
+}
+
/* Throw an error if PATH_OR_URL is a path and REVISION isn't a repository
* revision. */
static svn_error_t *
@@ -116,13 +125,15 @@ symmetric_merge(const char *source_path_
svn_boolean_t record_only,
svn_boolean_t dry_run,
svn_boolean_t allow_mixed_rev,
- svn_boolean_t allow_local_mods,
- svn_boolean_t allow_switched_subtrees,
+ svn_boolean_t reintegrate,
const apr_array_header_t *merge_options,
svn_client_ctx_t *ctx,
apr_pool_t *scratch_pool)
{
+ svn_boolean_t allow_local_mods = ! reintegrate;
+ svn_boolean_t allow_switched_subtrees = ! reintegrate;
svn_client__symmetric_merge_t *merge;
+ svn_boolean_t reintegrate_like;
/* Find the 3-way merges needed (and check suitability of the WC). */
SVN_ERR(svn_client__find_symmetric_merge(&merge,
@@ -131,5 +142,41 @@ symmetric_merge(const char *source_path_
allow_local_mods, allow_switched_subtrees,
ctx, scratch_pool, scratch_pool));
+ reintegrate_like = (merge->mid != NULL);
+
+ if (reintegrate && ! reintegrate_like
+ && ! pathrev_equal(merge->yca, merge->base))
+ {
+ return svn_error_create(SVN_ERR_CL_MUTUALLY_EXCLUSIVE_ARGS, NULL,
+ _("The --reintegrate option was given but the "
+ "required merge is not reintegrate-like"));
+ }
+ if (reintegrate_like)
+ {
+ if (record_only)
+ return svn_error_create(SVN_ERR_CL_MUTUALLY_EXCLUSIVE_ARGS, NULL,
+ _("The required merge is reintegrate-like, "
+ "and the --record-only option "
+ "cannot be used with this kind of merge"));
+
+ if (depth != svn_depth_unknown)
+ return svn_error_create(SVN_ERR_CL_MUTUALLY_EXCLUSIVE_ARGS, NULL,
+ _("The required merge is reintegrate-like, "
+ "and the --depth option "
+ "cannot be used with this kind of merge"));
+
+ if (force)
+ return svn_error_create(SVN_ERR_CL_MUTUALLY_EXCLUSIVE_ARGS, NULL,
+ _("The required merge is reintegrate-like, "
+ "and the --force option "
+ "cannot be used with this kind of merge"));
+
+ if (allow_mixed_rev)
+ return svn_error_create(SVN_ERR_CL_MUTUALLY_EXCLUSIVE_ARGS, NULL,
+ _("The required merge is reintegrate-like, "
+ "and the --allow-mixed-revisions option "
+ "cannot be used with this kind of merge"));
+ }
+
/* Perform the 3-way merges */
SVN_ERR(svn_client__do_symmetric_merge(merge, target_wcpath, depth,
@@ -405,19 +452,11 @@ svn_cl__merge(apr_getopt_t *os,
* If any conflicts occur we'll run the conflict resolver later. */
#ifdef SVN_WITH_SYMMETRIC_MERGE
- if (opt_state->symmetric_merge)
+ /* Do a symmetric merge if just one source and no revisions. */
+ if ((! two_sources_specified) && (! opt_state->ignore_ancestry)
+ && first_range_start.kind == svn_opt_revision_unspecified
+ && first_range_end.kind == svn_opt_revision_unspecified)
{
- svn_boolean_t allow_local_mods = ! opt_state->reintegrate;
- svn_boolean_t allow_switched_subtrees = ! opt_state->reintegrate;
-
- if (two_sources_specified)
- return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
- _("SOURCE2 can't be used with --symmetric"));
- if (first_range_start.kind != svn_opt_revision_unspecified
- || first_range_end.kind != svn_opt_revision_unspecified)
- return svn_error_create(SVN_ERR_CL_ARG_PARSING_ERROR, NULL,
- _("a revision range can't be used with --symmetric"));
-
SVN_ERR_W(svn_cl__check_related_source_and_target(
sourcepath1, &peg_revision1, targetpath, &unspecified,
ctx, pool),
@@ -430,8 +469,7 @@ svn_cl__merge(apr_getopt_t *os,
opt_state->record_only,
opt_state->dry_run,
opt_state->allow_mixed_rev,
- allow_local_mods,
- allow_switched_subtrees,
+ opt_state->reintegrate,
options, ctx, pool);
}
else
