Vlad Georgescu <vgeorgescu_at_gmail.com> writes:
> With this patch, you can relocate a WC with just a plain switch,
> without --relocate. And you can relocate to a new repo root _and_
> switch the FS path in one step.
In other words, this makes the '--relocate' option obsolete?
After we introduced the '--relocate' option, we sometimes wondered
whether we'd made a mistake: maybe it should have been a separate
command, because what 'switch --relocate' does is conceptually different
from what a plain 'switch' does.
However, I think it might be the case that having '--relocate' as an
option is the worst of all worlds. Either it should be a separate
command, or plain 'switch' should just DTRT when handed a new root. The
latter is what your patch does, right? (And I see that it does protect
against switching to a different repos UUID, as it should.)
I'm close to +1 on this, but would like others' thoughts...
-Karl
> [[[
> Make vanilla switch capable of changing the repository root.
>
> * subversion/include/svn_wc.h
> (svn_wc_get_switch_editor4): New function.
> (svn_wc_get_switch_editor3): Deprecate.
>
> * subversion/libsvn_client/switch.c
> (svn_client__switch_internal): Determine if the switch URL is in
> another repository. Call svn_wc_get_switch_editor4().
>
> * subversion/libsvn_wc/update_editor.c
> (make_editor): Take a new_repos argument. If it is non-NULL, store
> it in the editor baton.
> (svn_wc_get_update_editor3): Update call to make_editor().
> (svn_wc_get_switch_editor4): New function. Propagates new_repo
> to make_editor().
> (svn_wc_get_switch_editor3, svn_wc_get_switch_editor2,
> svn_wc_get_switch_editor): Call svn_wc_get_switch_editor4().
>
> * subversion/libsvn_wc/entries.c
> (svn_wc__tweak_entry): Rewrite the repo root and the copyfrom URL,
> if necessary. Remove the consistency check that tried to ensure that
> we never end up with mixed-repository directories. It was redundant
> before, and now it gets in the way.
>
> * subversion/tests/cmdline/switch_tests.py
> (switch_to_repo_with_different_uuid): Rename from
> switch_change_repos_root(), because we allow that now. However,
> the test tries to switch to a repo with a different UUID, which
> we still need to prevent.
> (switch_to_different_repo): New test.
> (test_list): Make the necessary additions/renames.
> ]]]
>
> --
> Vlad
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 32702)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -3530,10 +3530,12 @@
> * A variant of svn_wc_get_update_editor().
> *
> * Set @a *editor and @a *edit_baton to an editor and baton for "switching"
> - * a working copy to a new @a switch_url. (Right now, this URL must be
> - * within the same repository that the working copy already comes
> - * from.) @a switch_url must not be @c NULL.
> + * a working copy to a new @a switch_url. @a switch_url must not be @c NULL.
> *
> + * If @switch_url is not within the same repository that the working copy
> + * already comes from, then @new_repos must be set to the repository root URL
> + * for the target repo, otherwise @new_repos must be NULL.
> + *
> * If @a ti is non-NULL, record traversal info in @a ti, for use by
> * post-traversal accessors such as svn_wc_edited_externals().
> *
> @@ -3582,9 +3584,38 @@
> * If @a allow_unver_obstructions is TRUE, then allow unversioned
> * obstructions when adding a path.
> *
> - * @since New in 1.5.
> + * @since New in 1.6.
> */
> svn_error_t *
> +svn_wc_get_switch_editor4(svn_revnum_t *target_revision,
> + svn_wc_adm_access_t *anchor,
> + const char *target,
> + const char *switch_url,
> + const char *new_repos,
> + svn_boolean_t use_commit_times,
> + svn_depth_t depth,
> + svn_boolean_t depth_is_sticky,
> + svn_boolean_t allow_unver_obstructions,
> + svn_wc_notify_func2_t notify_func,
> + void *notify_baton,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + svn_wc_conflict_resolver_func_t conflict_func,
> + void *conflict_baton,
> + const char *diff3_cmd,
> + apr_array_header_t *preserved_exts,
> + const svn_delta_editor_t **editor,
> + void **edit_baton,
> + svn_wc_traversal_info_t *ti,
> + apr_pool_t *pool);
> +
> +/*
> + * Similar to svn_wc_get_switch_editor4(), but with the @new_repos
> + * parameter always set to NULL.
> + *
> + * @deprecated Provided for backward compatibility with the 1.5 API.
> + */
> +svn_error_t *
> svn_wc_get_switch_editor3(svn_revnum_t *target_revision,
> svn_wc_adm_access_t *anchor,
> const char *target,
> Index: subversion/libsvn_client/switch.c
> ===================================================================
> --- subversion/libsvn_client/switch.c (revision 32702)
> +++ subversion/libsvn_client/switch.c (working copy)
> @@ -66,7 +66,7 @@
> const svn_ra_reporter3_t *reporter;
> void *report_baton;
> const svn_wc_entry_t *entry;
> - const char *URL, *anchor, *target, *source_root, *switch_rev_url;
> + const char *URL, *anchor, *target, *source_root, *switch_rev_url, *new_repo;
> svn_ra_session_t *ra_session;
> svn_revnum_t revnum;
> svn_error_t *err = SVN_NO_ERROR;
> @@ -124,8 +124,6 @@
> _("Directory '%s' has no URL"),
> svn_path_local_style(anchor, pool));
>
> - URL = apr_pstrdup(pool, entry->url);
> -
> /* Open an RA session to 'source' URL */
> SVN_ERR(svn_client__ra_session_from_path(&ra_session, &revnum,
> &switch_rev_url,
> @@ -134,22 +132,40 @@
> ctx, pool));
> SVN_ERR(svn_ra_get_repos_root2(ra_session, &source_root, pool));
>
> - /* Disallow a switch operation to change the repository root of the
> - target. */
> - if (! svn_path_is_ancestor(source_root, URL))
> - return svn_error_createf
> - (SVN_ERR_WC_INVALID_SWITCH, NULL,
> - _("'%s'\n"
> - "is not the same repository as\n"
> - "'%s'"), URL, source_root);
> + if (svn_path_is_ancestor(source_root, entry->url))
> + {
> + URL = apr_pstrdup(pool, entry->url);
> + new_repo = NULL;
> + }
> + else if (entry->repos)
> + {
> + if (depth != svn_depth_unknown && depth != svn_depth_infinity)
> + return svn_error_create
> + (SVN_ERR_WC_INVALID_SWITCH, NULL,
> + _("A switch that changes the repository root can only have "
> + "a depth of unknown or infinity"));
>
> + URL = apr_pstrcat(pool, source_root,
> + entry->url + strlen(entry->repos), NULL);
> + new_repo = source_root;
> + }
> + else
> + {
> + return svn_error_createf
> + (SVN_ERR_WC_INVALID_SWITCH, NULL,
> + _("'%s'\n"
> + "is not the same repository as\n"
> + "'%s'"), entry->url, source_root);
> + }
> +
> SVN_ERR(svn_ra_reparent(ra_session, URL, pool));
>
> /* Fetch the switch (update) editor. If REVISION is invalid, that's
> okay; the RA driver will call editor->set_target_revision() later on. */
> - SVN_ERR(svn_wc_get_switch_editor3(&revnum, adm_access, target,
> - switch_rev_url, use_commit_times, depth,
> - depth_is_sticky, allow_unver_obstructions,
> + SVN_ERR(svn_wc_get_switch_editor4(&revnum, adm_access, target,
> + switch_rev_url, new_repo,
> + use_commit_times, depth, depth_is_sticky,
> + allow_unver_obstructions,
> ctx->notify_func2, ctx->notify_baton2,
> ctx->cancel_func, ctx->cancel_baton,
> ctx->conflict_func, ctx->conflict_baton,
> Index: subversion/libsvn_wc/entries.c
> ===================================================================
> --- subversion/libsvn_wc/entries.c (revision 32702)
> +++ subversion/libsvn_wc/entries.c (working copy)
> @@ -2707,38 +2707,16 @@
> && entry->url
> && svn_path_is_ancestor(repos, entry->url))
> {
> - svn_boolean_t set_repos = TRUE;
> + *write_required = TRUE;
>
> - /* Setting the repository root on THIS_DIR will make files in this
> - directory inherit that property. So to not make the WC corrupt,
> - we have to make sure that the repos root is valid for such entries as
> - well. Note that this shouldn't happen in normal circumstances. */
> - if (strcmp(entry->name, SVN_WC_ENTRY_THIS_DIR) == 0)
> + if (entry->copyfrom_url
> + && ! svn_path_is_ancestor(repos, entry->copyfrom_url))
> {
> - apr_hash_index_t *hi;
> - for (hi = apr_hash_first(pool, entries); hi;
> - hi = apr_hash_next(hi))
> - {
> - void *value;
> - const svn_wc_entry_t *child_entry;
> -
> - apr_hash_this(hi, NULL, NULL, &value);
> - child_entry = value;
> -
> - if (! child_entry->repos && child_entry->url
> - && ! svn_path_is_ancestor(repos, child_entry->url))
> - {
> - set_repos = FALSE;
> - break;
> - }
> - }
> + const char *relative;
> + relative = entry->copyfrom_url + strlen(entry->repos);
> + entry->copyfrom_url = apr_pstrcat(pool, repos, relative, NULL);
> }
> -
> - if (set_repos)
> - {
> - *write_required = TRUE;
> - entry->repos = apr_pstrdup(pool, repos);
> - }
> + entry->repos = apr_pstrdup(pool, repos);
> }
>
> if ((SVN_IS_VALID_REVNUM(new_rev))
> Index: subversion/libsvn_wc/update_editor.c
> ===================================================================
> --- subversion/libsvn_wc/update_editor.c (revision 32702)
> +++ subversion/libsvn_wc/update_editor.c (working copy)
> @@ -3291,6 +3291,7 @@
> const char *target,
> svn_boolean_t use_commit_times,
> const char *switch_url,
> + const char *new_repos,
> svn_depth_t depth,
> svn_boolean_t depth_is_sticky,
> svn_boolean_t allow_unver_obstructions,
> @@ -3323,9 +3324,9 @@
> /* Get the anchor entry, so we can fetch the repository root. */
> SVN_ERR(svn_wc_entry(&entry, anchor, adm_access, FALSE, pool));
>
> - /* Disallow a switch operation to change the repository root of the target,
> - if that is known. */
> - if (switch_url && entry && entry->repos &&
> + /* If new_repos is NULL, disallow a switch operation to a URL outside
> + the current repository root of the target, if that is known. */
> + if (switch_url && entry && entry->repos && ! new_repos &&
> ! svn_path_is_ancestor(entry->repos, switch_url))
> return svn_error_createf
> (SVN_ERR_WC_INVALID_SWITCH, NULL,
> @@ -3339,7 +3340,6 @@
> eb->use_commit_times = use_commit_times;
> eb->target_revision = target_revision;
> eb->switch_url = switch_url;
> - eb->repos = entry ? entry->repos : NULL;
> eb->adm_access = adm_access;
> eb->anchor = anchor;
> eb->target = target;
> @@ -3358,6 +3358,17 @@
> eb->allow_unver_obstructions = allow_unver_obstructions;
> eb->skipped_paths = apr_hash_make(subpool);
> eb->ext_patterns = preserved_exts;
> +
> + if (new_repos)
> + {
> + SVN_ERR_ASSERT(switch_url);
> + SVN_ERR_ASSERT(svn_path_is_ancestor(new_repos, switch_url));
> + eb->repos = new_repos;
> + }
> + else
> + {
> + eb->repos = entry ? entry->repos : NULL;
> + }
>
> /* Construct an editor. */
> tree_editor->set_target_revision = set_target_revision;
> @@ -3452,10 +3463,10 @@
> apr_pool_t *pool)
> {
> return make_editor(target_revision, anchor, svn_wc_adm_access_path(anchor),
> - target, use_commit_times, NULL, depth, depth_is_sticky,
> - allow_unver_obstructions, notify_func, notify_baton,
> - cancel_func, cancel_baton, conflict_func, conflict_baton,
> - fetch_func, fetch_baton,
> + target, use_commit_times, NULL, NULL, depth,
> + depth_is_sticky, allow_unver_obstructions,
> + notify_func, notify_baton, cancel_func, cancel_baton,
> + conflict_func, conflict_baton, fetch_func, fetch_baton,
> diff3_cmd, preserved_exts, editor, edit_baton,
> traversal_info, pool);
> }
> @@ -3518,10 +3529,11 @@
> }
>
> svn_error_t *
> -svn_wc_get_switch_editor3(svn_revnum_t *target_revision,
> +svn_wc_get_switch_editor4(svn_revnum_t *target_revision,
> svn_wc_adm_access_t *anchor,
> const char *target,
> const char *switch_url,
> + const char *new_repo,
> svn_boolean_t use_commit_times,
> svn_depth_t depth,
> svn_boolean_t depth_is_sticky,
> @@ -3542,7 +3554,7 @@
> SVN_ERR_ASSERT(switch_url);
>
> return make_editor(target_revision, anchor, svn_wc_adm_access_path(anchor),
> - target, use_commit_times, switch_url,
> + target, use_commit_times, switch_url, new_repo,
> depth, depth_is_sticky, allow_unver_obstructions,
> notify_func, notify_baton, cancel_func, cancel_baton,
> conflict_func, conflict_baton,
> @@ -3552,6 +3564,40 @@
> }
>
> svn_error_t *
> +svn_wc_get_switch_editor3(svn_revnum_t *target_revision,
> + svn_wc_adm_access_t *anchor,
> + const char *target,
> + const char *switch_url,
> + svn_boolean_t use_commit_times,
> + svn_depth_t depth,
> + svn_boolean_t depth_is_sticky,
> + svn_boolean_t allow_unver_obstructions,
> + svn_wc_notify_func2_t notify_func,
> + void *notify_baton,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + svn_wc_conflict_resolver_func_t conflict_func,
> + void *conflict_baton,
> + const char *diff3_cmd,
> + apr_array_header_t *preserved_exts,
> + const svn_delta_editor_t **editor,
> + void **edit_baton,
> + svn_wc_traversal_info_t *traversal_info,
> + apr_pool_t *pool)
> +{
> + SVN_ERR_ASSERT(switch_url);
> +
> + return svn_wc_get_switch_editor4(target_revision, anchor, target,
> + switch_url, NULL, use_commit_times,
> + depth, depth_is_sticky,
> + allow_unver_obstructions,
> + notify_func, notify_baton,
> + cancel_func, cancel_baton, conflict_func,
> + conflict_baton, diff3_cmd, preserved_exts,
> + editor, edit_baton, traversal_info, pool);
> +}
> +
> +svn_error_t *
> svn_wc_get_switch_editor2(svn_revnum_t *target_revision,
> svn_wc_adm_access_t *anchor,
> const char *target,
> @@ -3570,8 +3616,8 @@
> {
> SVN_ERR_ASSERT(switch_url);
>
> - return svn_wc_get_switch_editor3(target_revision, anchor, target,
> - switch_url, use_commit_times,
> + return svn_wc_get_switch_editor4(target_revision, anchor, target,
> + switch_url, NULL, use_commit_times,
> SVN_DEPTH_INFINITY_OR_FILES(recurse), FALSE,
> FALSE, notify_func, notify_baton,
> cancel_func, cancel_baton,
> @@ -3601,8 +3647,8 @@
> nb->func = notify_func;
> nb->baton = notify_baton;
>
> - return svn_wc_get_switch_editor3(target_revision, anchor, target,
> - switch_url, use_commit_times,
> + return svn_wc_get_switch_editor4(target_revision, anchor, target,
> + switch_url, NULL, use_commit_times,
> SVN_DEPTH_INFINITY_OR_FILES(recurse), FALSE,
> FALSE, svn_wc__compat_call_notify_func, nb,
> cancel_func, cancel_baton,
> Index: subversion/tests/cmdline/switch_tests.py
> ===================================================================
> --- subversion/tests/cmdline/switch_tests.py (revision 32702)
> +++ subversion/tests/cmdline/switch_tests.py (working copy)
> @@ -1036,9 +1036,10 @@
> "to a branch on which its svn:needs-lock property "
> "is not set" % mu_path)
>
> -# Check that switch can't change the repository root.
> -def switch_change_repos_root(sbox):
> - "switch shouldn't allow changing repos root"
> +# Check that switch can't change the repository root if the target repo
> +# has a different UUID.
> +def switch_to_repo_with_different_uuid(sbox):
> + "switch to repo with another UUID shouldn't work"
> sbox.build()
>
> wc_dir = sbox.wc_dir
> @@ -1061,7 +1062,7 @@
> 'switch',
> other_A_url, A_wc_dir)
>
> - # Test 2: A switch that changes the repo root part of the URL shouldn't work.
> + # Test 2: A switch to a repo with a different UUID should'n work.
> other_repo_dir, other_repo_url = sbox.add_repo_path('other')
> other_A_url = other_repo_url + "/A"
>
> @@ -2108,6 +2109,73 @@
> expected_disk,
> expected_status)
>
> +# Do a repository root change and a switch in one step,
> +# using vanilla switch.
> +def switch_to_different_repo(sbox):
> + "switch to another repository"
> + sbox.build(read_only = True)
> +
> + wc_dir = sbox.wc_dir
> + repo_dir = sbox.repo_dir
> + repo_url = sbox.repo_url
> + other_repo_dir, other_repo_url = sbox.add_repo_path('other')
> + svntest.main.copy_repos(repo_dir, other_repo_dir, 1, 0)
> +
> + C_url = repo_url + '/A/C'
> + other_B_url = other_repo_url + '/A/B'
> + C_path = os.path.join(wc_dir, 'A', 'C')
> + gamma_src_path = os.path.join(wc_dir, 'A', 'D', 'gamma')
> + gamma_dst_path = os.path.join(wc_dir, 'A', 'C', 'gamma')
> + switched_gamma_url = other_repo_url + '/A/B/gamma'
> + copyfrom_gamma_url = other_repo_url + '/A/D/gamma'
> +
> + svntest.main.run_svn(None, 'cp', gamma_src_path, C_path)
> +
> + # Check that we can't do a shallow switch that changes
> + # the repository root.
> + svntest.actions.run_and_verify_svn(None, None, ".*depth.*",
> + "switch", "--depth", "files",
> + other_B_url, C_path)
> +
> + expected_output = svntest.wc.State(wc_dir, {
> + 'A/C/lambda' : Item(status='A '),
> + 'A/C/E' : Item(status='A '),
> + 'A/C/E/alpha' : Item(status='A '),
> + 'A/C/E/beta' : Item(status='A '),
> + 'A/C/F' : Item(status='A '),
> + })
> + expected_disk = svntest.main.greek_state.copy()
> + expected_disk.add_state('A/C', expected_disk.subtree('A').subtree('B'))
> + expected_disk.add({
> + 'A/C/gamma' : Item(contents="This is the file 'gamma'.\n"),
> + })
> + # Can't do expected_status because a subtree of the wc is now in
> + # a different repostitory and the status would error out.
> + svntest.actions.run_and_verify_switch(wc_dir, C_path, other_B_url,
> + expected_output,
> + expected_disk,
> + None)
> +
> + # Check that we can contact the repository, meaning that the
> + # switch actually changed the URI. Escape the expected URI to
> + # avoid problems from any regex meta-characters it may contain
> + # (e.g. '+').
> + escaped_exp = '^URL: ' + re.escape(other_B_url) + '$' \
> + '|Path.+|Repository.+|Revision.+|Node.+|Last.+|\n'
> + svntest.actions.run_and_verify_svn(None, escaped_exp, [],
> + 'info', '-rHEAD', C_path)
> + # Check that the copyfrom url was rewritten correctly
> + escaped_exp = '^URL: ' + re.escape(switched_gamma_url) + '$' \
> + '|^Copied From URL: ' + re.escape(copyfrom_gamma_url) + '$' \
> + '|^Repository Root: ' + re.escape(other_repo_url) + '$' \
> + '|^Copied From Rev: 1$' + \
> + '|Path.+|Name.+|Revision.+|Node.+|Schedule.+|Checksum.+|\n'
> + svntest.actions.run_and_verify_svn(None, escaped_exp, [],
> + 'info', gamma_dst_path)
> +
> + svntest.actions.run_and_verify_svn(None, None, [],
> + 'ci', '-m', 'log_msg', C_path)
> +
> ########################################################################
> # Run the tests
>
> @@ -2130,7 +2198,7 @@
> commit_mods_below_switch,
> relocate_beyond_repos_root,
> refresh_read_only_attribute,
> - switch_change_repos_root,
> + switch_to_repo_with_different_uuid,
> XFail(relocate_and_propset, svntest.main.is_ra_type_dav),
> forced_switch,
> forced_switch_failures,
> @@ -2142,6 +2210,7 @@
> switch_urls_with_spaces,
> switch_to_dir_with_peg_rev2,
> switch_to_root,
> + switch_to_different_repo,
> ]
>
> if __name__ == '__main__':
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-27 21:26:40 CEST