You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2010/09/21 19:11:31 UTC
svn commit: r999505 - in /subversion/trunk/subversion: include/
libsvn_client/ libsvn_wc/ svn/ tests/cmdline/
Author: cmpilato
Date: Tue Sep 21 17:11:30 2010
New Revision: 999505
URL: http://svn.apache.org/viewvc?rev=999505&view=rev
Log:
Disallow relocation of working copy subtrees and relocation at depths
other than infinity. This change breaks some compatibility with
previous APIs at both the svn_client and svn_wc levels -- I'll just be
honest about that. But from what I've seen, while the APIs used to
"work" (that is, do what they claimed), the result was a working copy
that didn't. Also, the command-line client disallowed some of this
stuff already, and just errored out on some parts that it didn't
disallow. In all, relocation was a mess. So I'm extremely
comfortable with calling this a bugfix.
* subversion/include/svn_wc.h
(svn_wc_relocate4): Note that local_abspath is the root directory of
a working copy.
(svn_wc_relocate3): Note that 'recurse' is replaced by 'depth' now.
* subversion/include/svn_client.h
(svn_client_relocate2): New.
(svn_client_relocate): Deprecated with noted limitations to backward
compatibility.
* subversion/libsvn_wc/relocate.c
(svn_wc_relocate4): Check local_abspath to see if it's a strict
working copy root; if not, disallow the relocation. Try to tell
the caller where to find the working copy root.
* subversion/libsvn_wc/deprecated.c
(svn_wc_relocate3): Disallow recurse=FALSE with an error. Update
call to svn_wc_relocate4().
* subversion/libsvn_client/relocate.c
(svn_client_relocate2): Rev svn_client_relocate(), this time without
a 'recurse' parameter.
(svn_client_relocate): Moved to deprecate.c.
* subversion/libsvn_client/deprecated.c
(svn_client_relocate): Moved here from relocate.c and made into a
simple wrapper around svn_client_relocate2().
* subversion/libsvn_client/externals.c
(switch_dir_external): Now use svn_client_relocate2().
* subversion/svn/main.c
(main): Since we disallow --relocate with --depth, it makes sense to
also disallow --relocate with --non-recursive, so do so.
* subversion/svn/switch-cmd.c
(rewrite_urls): Lose 'recurse' parameter, and now use
svn_client_relocate2().
(svn_cl__switch): Update call to rewrite_urls().
* subversion/tests/cmdline/switch_tests.py
(relocate_beyond_repos_root): Rewrite a bit to still test the
intended things without triggering the new error situation.
(single_file_relocate): Tweak expected error message.
Modified:
subversion/trunk/subversion/include/svn_client.h
subversion/trunk/subversion/include/svn_wc.h
subversion/trunk/subversion/libsvn_client/deprecated.c
subversion/trunk/subversion/libsvn_client/externals.c
subversion/trunk/subversion/libsvn_client/relocate.c
subversion/trunk/subversion/libsvn_wc/deprecated.c
subversion/trunk/subversion/libsvn_wc/relocate.c
subversion/trunk/subversion/svn/main.c
subversion/trunk/subversion/svn/switch-cmd.c
subversion/trunk/subversion/tests/cmdline/switch_tests.py
Modified: subversion/trunk/subversion/include/svn_client.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_client.h?rev=999505&r1=999504&r2=999505&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_client.h (original)
+++ subversion/trunk/subversion/include/svn_client.h Tue Sep 21 17:11:30 2010
@@ -3418,18 +3418,35 @@ svn_client_upgrade(const char *dir,
*/
/**
- * Modify a working copy directory @a dir, changing any
- * repository URLs that begin with @a from to begin with @a to instead,
- * recursing into subdirectories if @a recurse is TRUE.
+ * Recursively modify a working copy rooted at @a wcroot_dir, changing any
+ * repository URLs that begin with @a from to begin with @a to instead.
*
- * @param dir Working copy directory
+ * @param wcroot_dir Working copy root directory
* @param from Original URL
* @param to New URL
- * @param recurse Whether to recurse
* @param ctx svn_client_ctx_t
* @param pool The pool from which to perform memory allocations
+ *
+ * @since New in 1.7
*/
svn_error_t *
+svn_client_relocate2(const char *wcroot_dir,
+ const char *from,
+ const char *to,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool);
+
+/**
+ * Similar to svn_client_relocate2().
+ *
+ * @note As of the 1.7 API, @a dir is required to be a working copy
+ * root directory, and @a recurse is required to be TRUE.
+ *
+ * @deprecated Provided for limited backwards compatibility with the
+ * 1.6 API.
+ */
+SVN_DEPRECATED
+svn_error_t *
svn_client_relocate(const char *dir,
const char *from,
const char *to,
@@ -3437,7 +3454,6 @@ svn_client_relocate(const char *dir,
svn_client_ctx_t *ctx,
apr_pool_t *pool);
-
/** @} */
/**
Modified: subversion/trunk/subversion/include/svn_wc.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc.h?rev=999505&r1=999504&r2=999505&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_wc.h (original)
+++ subversion/trunk/subversion/include/svn_wc.h Tue Sep 21 17:11:30 2010
@@ -6791,13 +6791,13 @@ typedef svn_error_t *(*svn_wc_relocation
const char *uuid,
const char *url);
-/** Change repository references at @a local_abspath and all it's children.
- * The pre-change URL should be @a from, and the post-change URL will be
- * @a to. @a validator (and its baton, @a validator_baton), will be called
- * for the newly generated base URL and calculated repo root.
+/** Recursively change repository references at @a wcroot_abspath
+ * (which is the root directory of a working copy). The pre-change
+ * URL should be @a from, and the post-change URL will be @a to. @a
+ * validator (and its baton, @a validator_baton), will be called for
+ * the newly generated base URL and calculated repo root.
*
- * If @a recurse is @c FALSE, none of the children of @a local_abspath will
- * be changed. @a wc_ctx is an working copy context.
+ * @a wc_ctx is an working copy context.
*
* @a scratch_pool will be used for temporary allocations.
*
@@ -6805,10 +6805,9 @@ typedef svn_error_t *(*svn_wc_relocation
*/
svn_error_t *
svn_wc_relocate4(svn_wc_context_t *wc_ctx,
- const char *local_abspath,
+ const char *wcroot_abspath,
const char *from,
const char *to,
- svn_boolean_t recurse,
svn_wc_relocation_validator3_t validator,
void *validator_baton,
apr_pool_t *scratch_pool);
@@ -6816,8 +6815,12 @@ svn_wc_relocate4(svn_wc_context_t *wc_ct
/** Similar to svn_wc_relocate4(), but with a #svn_wc_adm_access_t /
* relative path parameter pair.
*
+ * @note As of the 1.7 API, @a path is required to be a working copy
+ * root directory, and @a recurse is required to be TRUE.
+ *
* @since New in 1.5.
- * @deprecated Provided for backwards compatibility with the 1.6 API.
+ * @deprecated Provided for limited backwards compatibility with the
+ * 1.6 API.
*/
SVN_DEPRECATED
svn_error_t *
Modified: subversion/trunk/subversion/libsvn_client/deprecated.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/deprecated.c?rev=999505&r1=999504&r2=999505&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/deprecated.c (original)
+++ subversion/trunk/subversion/libsvn_client/deprecated.c Tue Sep 21 17:11:30 2010
@@ -2039,3 +2039,18 @@ svn_client_mergeinfo_log_eligible(const
svn_depth_empty, revprops, ctx,
pool);
}
+
+/*** From relocate.c ***/
+svn_error_t *
+svn_client_relocate(const char *path,
+ const char *from,
+ const char *to,
+ svn_boolean_t recurse,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ if (! recurse)
+ svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, 0,
+ _("Non-recursive relocation not supported"));
+ return svn_client_relocate2(path, from, to, ctx, pool);
+}
Modified: subversion/trunk/subversion/libsvn_client/externals.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/externals.c?rev=999505&r1=999504&r2=999505&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/externals.c (original)
+++ subversion/trunk/subversion/libsvn_client/externals.c Tue Sep 21 17:11:30 2010
@@ -229,8 +229,8 @@ switch_dir_external(const char *path,
SVN_ERR(svn_ra_get_repos_root2(ra_session, &repos_root,
subpool));
- err = svn_client_relocate(path, repos_root_url, repos_root,
- TRUE, ctx, subpool);
+ err = svn_client_relocate2(path, repos_root_url, repos_root,
+ ctx, subpool);
/* If the relocation failed because the new URL points
to another repository, then we need to relegate and
check out a new WC. */
Modified: subversion/trunk/subversion/libsvn_client/relocate.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/relocate.c?rev=999505&r1=999504&r2=999505&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/relocate.c (original)
+++ subversion/trunk/subversion/libsvn_client/relocate.c Tue Sep 21 17:11:30 2010
@@ -126,24 +126,23 @@ validator_func(void *baton,
}
svn_error_t *
-svn_client_relocate(const char *path,
- const char *from,
- const char *to,
- svn_boolean_t recurse,
- svn_client_ctx_t *ctx,
- apr_pool_t *pool)
+svn_client_relocate2(const char *wcroot_dir,
+ const char *from,
+ const char *to,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
{
struct validator_baton_t vb;
const char *local_abspath;
- /* Now, populate our validator callback baton, and call the relocate code. */
+ /* Populate our validator callback baton, and call the relocate code. */
vb.ctx = ctx;
- vb.path = path;
+ vb.path = wcroot_dir;
vb.url_uuids = apr_array_make(pool, 1, sizeof(struct url_uuid_t));
vb.pool = pool;
- SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
- SVN_ERR(svn_wc_relocate4(ctx->wc_ctx, local_abspath, from, to, recurse,
+ SVN_ERR(svn_dirent_get_absolute(&local_abspath, wcroot_dir, pool));
+ SVN_ERR(svn_wc_relocate4(ctx->wc_ctx, local_abspath, from, to,
validator_func, &vb, pool));
return SVN_NO_ERROR;
Modified: subversion/trunk/subversion/libsvn_wc/deprecated.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/deprecated.c?rev=999505&r1=999504&r2=999505&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/deprecated.c (original)
+++ subversion/trunk/subversion/libsvn_wc/deprecated.c Tue Sep 21 17:11:30 2010
@@ -3364,12 +3364,16 @@ svn_wc_relocate3(const char *path,
const char *local_abspath;
svn_wc_context_t *wc_ctx;
+ if (! recurse)
+ svn_error_create(SVN_ERR_UNSUPPORTED_FEATURE, 0,
+ _("Non-recursive relocation not supported"));
+
SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, pool));
SVN_ERR(svn_wc__context_create_with_db(&wc_ctx, NULL /* config */,
svn_wc__adm_get_db(adm_access),
pool));
- SVN_ERR(svn_wc_relocate4(wc_ctx, local_abspath, from, to, recurse,
+ SVN_ERR(svn_wc_relocate4(wc_ctx, local_abspath, from, to,
validator, validator_baton, pool));
return svn_error_return(svn_wc_context_destroy(wc_ctx));
Modified: subversion/trunk/subversion/libsvn_wc/relocate.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/relocate.c?rev=999505&r1=999504&r2=999505&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/relocate.c (original)
+++ subversion/trunk/subversion/libsvn_wc/relocate.c Tue Sep 21 17:11:30 2010
@@ -76,7 +76,6 @@ svn_wc_relocate4(svn_wc_context_t *wc_ct
const char *local_abspath,
const char *from,
const char *to,
- svn_boolean_t recurse,
svn_wc_relocation_validator3_t validator,
void *validator_baton,
apr_pool_t *scratch_pool)
@@ -87,6 +86,35 @@ svn_wc_relocate4(svn_wc_context_t *wc_ct
const char *old_url;
const char *new_repos_root;
const char *uuid;
+ svn_boolean_t is_wc_root;
+
+ SVN_ERR(svn_wc__strictly_is_wc_root(&is_wc_root, wc_ctx, local_abspath,
+ scratch_pool));
+ if (! is_wc_root)
+ {
+ const char *wcroot_abspath;
+ svn_error_t *err;
+
+ err = svn_wc__db_get_wcroot(&wcroot_abspath, wc_ctx->db,
+ local_abspath, scratch_pool, scratch_pool);
+ if (err)
+ {
+ svn_error_clear(err);
+ return svn_error_createf(
+ SVN_ERR_WC_INVALID_OP_ON_CWD, NULL,
+ _("Cannot relocate '%s' as it is not the root of a working copy"),
+ svn_dirent_local_style(local_abspath, scratch_pool));
+ }
+ else
+ {
+ return svn_error_createf(
+ SVN_ERR_WC_INVALID_OP_ON_CWD, NULL,
+ _("Cannot relocate '%s' as it is not the root of a working copy; "
+ "try relocating '%s' instead"),
+ svn_dirent_local_style(local_abspath, scratch_pool),
+ svn_dirent_local_style(wcroot_abspath, scratch_pool));
+ }
+ }
SVN_ERR(svn_wc__db_read_info(NULL, &kind, NULL, &repos_relpath,
&old_repos_root, &uuid,
@@ -112,48 +140,7 @@ svn_wc_relocate4(svn_wc_context_t *wc_ct
SVN_ERR(validator(validator_baton, uuid, to, new_repos_root, scratch_pool));
- /* ### FIXME: This will ultimately cause the DAV cache to be
- recursively cleared, which is great in the recursive case, but
- overreaching otherwise. Granted, this only affects performance,
- and that only for DAV RA implementations that rely on the DAV
- cache. */
- SVN_ERR(svn_wc__db_global_relocate(wc_ctx->db, local_abspath, new_repos_root,
- scratch_pool));
-
- if (!recurse)
- {
- /* This gets sticky. We need to do the above relocation, and then
- relocate each of the children *back* to the original location. Ugh.
- */
- const apr_array_header_t *children;
- apr_pool_t *iterpool;
- int i;
-
- SVN_ERR(svn_wc__db_read_children(&children, wc_ctx->db, local_abspath,
- scratch_pool, scratch_pool));
- iterpool = svn_pool_create(scratch_pool);
- for (i = 0; i < children->nelts; i++)
- {
- const char *child = APR_ARRAY_IDX(children, i, const char *);
- const char *child_abspath;
- const char *child_from;
- const char *child_to;
-
- svn_pool_clear(iterpool);
- child_abspath = svn_dirent_join(local_abspath, child, iterpool);
-
- /* We invert the "from" and "to" because we're switching the
- children back to the original location. */
- child_from = svn_uri_join(to, child, iterpool);
- child_to = svn_uri_join(from, child, iterpool);
-
- SVN_ERR(svn_wc_relocate4(wc_ctx, child_abspath, child_from,
- child_to, TRUE, validator, validator_baton,
- iterpool));
- }
-
- svn_pool_destroy(iterpool);
- }
-
- return SVN_NO_ERROR;
+ return svn_error_return(svn_wc__db_global_relocate(wc_ctx->db, local_abspath,
+ new_repos_root,
+ scratch_pool));
}
Modified: subversion/trunk/subversion/svn/main.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/main.c?rev=999505&r1=999504&r2=999505&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/main.c (original)
+++ subversion/trunk/subversion/svn/main.c Tue Sep 21 17:11:30 2010
@@ -2087,12 +2087,24 @@ main(int argc, const char *argv[])
}
}
- if (opt_state.relocate && (opt_state.depth != svn_depth_unknown))
+ /* Relocation is infinite-depth only. */
+ if (opt_state.relocate)
{
- err = svn_error_create(SVN_ERR_CL_MUTUALLY_EXCLUSIVE_ARGS, NULL,
- _("--relocate and --depth are mutually "
- "exclusive"));
- return svn_cmdline_handle_exit_error(err, pool, "svn: ");
+ if (opt_state.depth != svn_depth_unknown)
+ {
+ err = svn_error_create(SVN_ERR_CL_MUTUALLY_EXCLUSIVE_ARGS, NULL,
+ _("--relocate and --depth are mutually "
+ "exclusive"));
+ return svn_cmdline_handle_exit_error(err, pool, "svn: ");
+ }
+ if (! descend)
+ {
+ err = svn_error_create(
+ SVN_ERR_CL_MUTUALLY_EXCLUSIVE_ARGS, NULL,
+ _("--relocate and --non-recursive (-N) are mutually "
+ "exclusive"));
+ return svn_cmdline_handle_exit_error(err, pool, "svn: ");
+ }
}
/* Only a few commands can accept a revision range; the rest can take at
Modified: subversion/trunk/subversion/svn/switch-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/switch-cmd.c?rev=999505&r1=999504&r2=999505&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/switch-cmd.c (original)
+++ subversion/trunk/subversion/svn/switch-cmd.c Tue Sep 21 17:11:30 2010
@@ -41,7 +41,6 @@
static svn_error_t *
rewrite_urls(const apr_array_header_t *targets,
- svn_boolean_t recurse,
svn_client_ctx_t *ctx,
apr_pool_t *pool)
{
@@ -67,7 +66,7 @@ rewrite_urls(const apr_array_header_t *t
if (targets->nelts == 2)
{
- SVN_ERR(svn_client_relocate("", from, to, recurse, ctx, pool));
+ SVN_ERR(svn_client_relocate2("", from, to, ctx, pool));
}
else
{
@@ -75,8 +74,7 @@ rewrite_urls(const apr_array_header_t *t
{
const char *target = APR_ARRAY_IDX(targets, i, const char *);
svn_pool_clear(subpool);
- SVN_ERR(svn_client_relocate(target, from, to, recurse,
- ctx, subpool));
+ SVN_ERR(svn_client_relocate2(target, from, to, ctx, subpool));
}
}
@@ -109,9 +107,7 @@ svn_cl__switch(apr_getopt_t *os,
/* handle only-rewrite case specially */
if (opt_state->relocate)
- return rewrite_urls(targets,
- SVN_DEPTH_IS_RECURSIVE(opt_state->depth),
- ctx, scratch_pool);
+ return rewrite_urls(targets, ctx, scratch_pool);
if (targets->nelts < 1)
return svn_error_create(SVN_ERR_CL_INSUFFICIENT_ARGS, 0, NULL);
Modified: subversion/trunk/subversion/tests/cmdline/switch_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/switch_tests.py?rev=999505&r1=999504&r2=999505&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/switch_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/switch_tests.py Tue Sep 21 17:11:30 2010
@@ -1049,18 +1049,25 @@ def commit_mods_below_switch(sbox):
def relocate_beyond_repos_root(sbox):
"relocate with prefixes longer than repo root"
- sbox.build(read_only = True)
+ sbox.build(read_only=True, create_wc=False)
+
+ wc_backup = sbox.add_wc_path('backup')
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)
-
A_url = repo_url + "/A"
+ A_wc_dir = wc_dir
other_A_url = other_repo_url + "/A"
other_B_url = other_repo_url + "/B"
- A_wc_dir = os.path.join(wc_dir, "A")
+
+ svntest.main.safe_rmtree(wc_dir, 1)
+ svntest.actions.run_and_verify_svn(None, None, [], 'checkout',
+ repo_url + '/A', wc_dir)
+
+ svntest.main.copy_repos(repo_dir, other_repo_dir, 1, 0)
+
# A relocate that changes the repo path part of the URL shouldn't work.
# This tests for issue #2380.
@@ -2959,7 +2966,7 @@ def single_file_relocate(sbox):
svntest.main.copy_repos(repo_dir, other_repo_dir, 1, 0)
svntest.main.safe_rmtree(repo_dir, 1)
svntest.actions.run_and_verify_svn(None, None,
- ".*Cannot relocate a single file\n",
+ ".*Cannot relocate.*",
'switch', '--relocate',
iota_url, other_iota_url, iota_path)
Re: svn_client_relocate2()'s docstring
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Tue, Sep 21, 2010 at 23:53:22 +0200:
> > /**
> > * Recursively modify a working copy rooted at @a wcroot_dir, changing any
> > * repository URLs that begin with @a from to begin with @a to instead.
> > *
> > * @param wcroot_dir Working copy root directory
> > * @param from Original URL
> > * @param to New URL
> > * @param ctx svn_client_ctx_t
> > * @param pool The pool from which to perform memory allocations
> > *
Hmm. 'svn praise' tells me that these lines (modulo the CTX) line
originated at r846244 --- i.e., they haven't changed in the last half
decade.
(That is: I still dislike information-free docstrings, but I realize
that this particular docstring is in no way related to the one I've
replied to hwright about earlier today.)
svn_client_relocate2()'s docstring
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
> /**
> * Recursively modify a working copy rooted at @a wcroot_dir, changing any
> * repository URLs that begin with @a from to begin with @a to instead.
> *
> * @param wcroot_dir Working copy root directory
> * @param from Original URL
> * @param to New URL
> * @param ctx svn_client_ctx_t
> * @param pool The pool from which to perform memory allocations
> *
The doc strings of CTX and POOL are information-free. (They say nothing
that isn't implied by the signature.)
Can we please have documentation that isn't information free?
Thanks,
Daniel
> * @since New in 1.7
> */
> svn_error_t *
> svn_client_relocate2(const char *wcroot_dir,
> const char *from,
> const char *to,
> svn_client_ctx_t *ctx,
> apr_pool_t *pool);