Stefan Sperling wrote:
> I'd like to propose that we disallow merges into mixed-revision working
> copies by default for 1.7, and only allow such merges if a special command
> line option has been passed.
>
> The reason for this is that merges into mixed-rev WCs can easily create
> conflicts which are tedious to resolve. Even if you know about the problem
> and try to run "svn update" before every merge, it's quite easy to forget
> about running "svn update".
>
> Also, we've been advertising this as best practice for some time.
> Quoting the book on the matter:
> Never merge into working copies with a mixture of working revision
> numbers, or with “switched” subdirectories (as described next in the
> section called “Traversing Branches”). A merge target should be a
> working copy which represents a single location in the repository at a
> single point in time.
>
> Note that while the book recommends not to merge into working copies
> with switched subtrees, I don't intend to enforce this by default.
>
> I've talked about this with Paul on IRC and he had no objections.
> See http://colabti.org/irclogger/irclogger_log/svn-dev?date=2010-09-30#l468
> for the full discussion.
>
> Below is a work-in-progress diff that starts making this change.
> Tests fail with it. I've only checked the merge tests for now.
>
> Before continuing with this, fixing up tests, bumping APIs, and eventually
> adding the command line option that allows merges into mixed-rev WCs,
> I'd like to ask whether anyone has concerns about this idea.
The proposal sounds good to me.
To be more specific about what the proposal is:
* It does not apply to re-integrate merges. (They always enforce this
anyway?)
* It applies to merge-tracking merges.
* It [applies/does not appy] to non-merge-tracking merges?
As for the tests: I think it would be best to keep some or all of the
tests doing merges into a mixed-rev WC as they do now, partly so as not
to lose test coverage of this condition, but more importantly to
preserve the purpose of any regression tests that happen to be testing
for bugs which only showed up in mixed-rev mode.
In other words, fix most of them by adding the bypass flag rather than
by adding an "svn update" command.
- Julian
> Below is the list of currently failing merge tests, and the
> work-in-progress diff.
>
> Thanks,
> Stefan
>
> FAIL: merge_tests.py 1: performing a merge, with mixed results
> FAIL: merge_tests.py 9: merge change into unchanged binary file
> FAIL: merge_tests.py 15: merge should skip over unversioned obstructions
> FAIL: merge_tests.py 16: merge into missing must not break working copy
> FAIL: merge_tests.py 23: merge between branches (Issue #2222)
> FAIL: merge_tests.py 31: merge a replacement of a file to mixed rev wc
> FAIL: merge_tests.py 32: ignore whitespace when merging
> FAIL: merge_tests.py 33: ignore eolstyle when merging
> FAIL: merge_tests.py 44: merge to path with switched children
> FAIL: merge_tests.py 69: subtrees added after start of merge range are ok
> FAIL: merge_tests.py 70: cyclic merges don't add mergeinfo from own history
> FAIL: merge_tests.py 77: merges that remove all mergeinfo work
> FAIL: merge_tests.py 89: target and subtrees need nonintersecting revs
> FAIL: merge_tests.py 90: merge two successive edits to the same property
> FAIL: merge_tests.py 91: merge an EOL unification and set svn:eol-style
> FAIL: merge_tests.py 103: automatic conflict resolutions work with merge
> FAIL: merge_tests.py 105: committed case only move causes revert to fail
> FAIL: merge_tests.py 107: merge del and ps variants from a foreign repos
> FAIL: merge_tests.py 111: merge into locally added file
> FAIL: merge_tests.py 112: merge into locally added directory
>
>
> Index: subversion/libsvn_client/merge.c
> ===================================================================
> --- subversion/libsvn_client/merge.c (revision 1003282)
> +++ subversion/libsvn_client/merge.c (working copy)
> @@ -8815,6 +8815,54 @@ merge_cousins_and_supplement_mergeinfo(const char
> return SVN_NO_ERROR;
> }
>
> +/* Perform checks to determine whether of the working copy at TARGET_ABSPATH
> + * can safely be used as a merge target. Checks are performed according to
> + * the FORBID_MIXED_REV, FORBID_LOCAL_MODS, and FORBID_SWITCHED_SUBTREES
> + * parameters. If any checks fail, raise SVN_ERR_CLIENT_NOT_READY_TO_MERGE.
> + *
> + * E.g. if all the FORBID_* parameters are TRUE, TARGET_ABSPATH must
> + * be a single-revision, pristine, unswitched working copy.
> + * In other words, it must reflect a subtree of the repostiory as found
> + * at single revision -- although sparse checkouts are permitted. */
> +static svn_error_t *
> +ensure_wc_is_suitable_merge_target(const char *target_abspath,
> + svn_client_ctx_t *ctx,
> + svn_boolean_t forbid_mixed_rev,
> + svn_boolean_t forbid_local_mods,
> + svn_boolean_t forbid_switched_subtrees,
> + apr_pool_t *scratch_pool)
> +{
> + svn_wc_revision_status_t *wc_stat;
> +
> + /* Get a WC summary with min/max revisions set to the BASE revision. */
> + SVN_ERR(svn_wc_revision_status2(&wc_stat, ctx->wc_ctx, target_abspath, NULL,
> + FALSE, ctx->cancel_func, ctx->cancel_baton,
> + scratch_pool, scratch_pool));
> +
> + if (forbid_switched_subtrees && wc_stat->switched)
> + return svn_error_create(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> + _("Cannot merge into a working copy "
> + "with a switched subtree"));
> +
> + if (forbid_local_mods && wc_stat->modified)
> + return svn_error_create(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> + _("Cannot merge into a working copy "
> + "that has local modifications"));
> +
> + if (! (SVN_IS_VALID_REVNUM(wc_stat->min_rev)
> + && SVN_IS_VALID_REVNUM(wc_stat->max_rev)))
> + return svn_error_create(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> + _("Cannot determine revision of working copy"));
> +
> + if (forbid_mixed_rev && wc_stat->min_rev != wc_stat->max_rev)
> + return svn_error_create(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> + _("Cannot merge into mixed-revision "
> + "working copy; try updating first"));
> +
> + return SVN_NO_ERROR;
> +}
> +
> +
> /*-----------------------------------------------------------------------*/
>
> /*** Public APIs ***/
> @@ -8905,6 +8953,12 @@ merge_locked(const char *source1,
> _("Merge target '%s' does not exist in the "
> "working copy"), target_abspath));
>
> +
> + /* Disallow merges into mixed-revision working copies. */
> + SVN_ERR(ensure_wc_is_suitable_merge_target(target_abspath, ctx,
> + TRUE, FALSE, FALSE,
> + scratch_pool));
> +
> /* Determine the working copy target's repository root URL. */
> working_rev.kind = svn_opt_revision_working;
> SVN_ERR(svn_client__get_repos_root(&wc_repos_root, target_abspath,
> @@ -9188,45 +9242,6 @@ svn_client_merge3(const char *source1,
> }
>
>
> -/* If TARGET_WCPATH does not reflect a single-revision, pristine,
> - unswitched working copy -- in other words, a subtree found in a
> - single revision (although sparse checkouts are permitted) -- raise
> - SVN_ERR_CLIENT_NOT_READY_TO_MERGE. */
> -static svn_error_t *
> -ensure_wc_reflects_repository_subtree(const char *target_abspath,
> - svn_client_ctx_t *ctx,
> - apr_pool_t *scratch_pool)
> -{
> - svn_wc_revision_status_t *wc_stat;
> -
> - /* Get a WC summary with min/max revisions set to the BASE revision. */
> - SVN_ERR(svn_wc_revision_status2(&wc_stat, ctx->wc_ctx, target_abspath, NULL,
> - FALSE, ctx->cancel_func, ctx->cancel_baton,
> - scratch_pool, scratch_pool));
> -
> - if (wc_stat->switched)
> - return svn_error_create(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> - _("Cannot reintegrate into a working copy "
> - "with a switched subtree"));
> -
> - if (wc_stat->modified)
> - return svn_error_create(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> - _("Cannot reintegrate into a working copy "
> - "that has local modifications"));
> -
> - if (! (SVN_IS_VALID_REVNUM(wc_stat->min_rev)
> - && SVN_IS_VALID_REVNUM(wc_stat->max_rev)))
> - return svn_error_create(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> - _("Cannot determine revision of working copy"));
> -
> - if (wc_stat->min_rev != wc_stat->max_rev)
> - return svn_error_create(SVN_ERR_CLIENT_NOT_READY_TO_MERGE, NULL,
> - _("Cannot reintegrate into mixed-revision "
> - "working copy; try updating first"));
> -
> - return SVN_NO_ERROR;
> -}
> -
> /* Check if mergeinfo for a given path is described explicitly or via
> inheritance in a mergeinfo catalog.
>
> @@ -10174,8 +10189,11 @@ merge_reintegrate_locked(const char *source,
> svn_dirent_local_style(target_abspath,
> scratch_pool));
>
> - SVN_ERR(ensure_wc_reflects_repository_subtree(target_abspath, ctx,
> - scratch_pool));
> + /* A reintegrate merge requires the merge target to reflect a subtree
> + * of the repository as found at a single revision. */
> + SVN_ERR(ensure_wc_is_suitable_merge_target(target_abspath, ctx,
> + TRUE, TRUE, TRUE,
> + scratch_pool));
> SVN_ERR(svn_wc__node_get_base_rev(&target_base_rev, ctx->wc_ctx,
> target_abspath, scratch_pool));
>
> @@ -10432,6 +10450,11 @@ merge_peg_locked(const char *source,
> _("Merge target '%s' does not exist in the "
> "working copy"), target_abspath));
>
> + /* Disallow merges into mixed-revision working copies. */
> + SVN_ERR(ensure_wc_is_suitable_merge_target(target_abspath, ctx,
> + TRUE, FALSE, FALSE,
> + scratch_pool));
> +
> /* Determine the working copy target's repository root URL. */
> working_rev.kind = svn_opt_revision_working;
> SVN_ERR(svn_client__get_repos_root(&wc_repos_root, target_abspath,
Received on 2010-10-01 13:26:22 CEST