Author: rhuijben
Date: Wed Oct 17 13:15:39 2012
New Revision: 1399220
URL: http://svn.apache.org/viewvc?rev=1399220&view=rev
Log:
Resolve a recently introduced regression in the 'svn cp PATH PATH' code
and move the wc check code to a single place to avoid fixing two cases.
* subversion/libsvn_client/copy.c
(verify_wc_dsts): New function with extra filestat, extracted from ...
(verify_wc_srcs_and_dsts): ... this function. Split pool handling.
(repos_to_wc_copy_locked): Remove duplicated code and call verify_wc_dsts
instead.
(try_copy): Update caller.
* subversion/tests/cmdline/copy_tests.py
(mv_unversioned_file,
(copy_deleted_dir): Expect more detailed error messages.
Modified:
subversion/trunk/subversion/libsvn_client/copy.c
subversion/trunk/subversion/tests/cmdline/copy_tests.py
Modified: subversion/trunk/subversion/libsvn_client/copy.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/copy.c?rev=1399220&r1=1399219&r2=1399220&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/copy.c (original)
+++ subversion/trunk/subversion/libsvn_client/copy.c Wed Oct 17 13:15:39 2012
@@ -365,18 +365,21 @@ do_wc_to_wc_moves(const apr_array_header
return svn_error_trace(err);
}
-
+/* Verify that the destinations stored in COPY_PAIRS are valid working copy
+ destinations and set pair->dst_parent_abspath and pair->base_name for each
+ item to the resulting location if they do */
static svn_error_t *
-verify_wc_srcs_and_dsts(const apr_array_header_t *copy_pairs,
- svn_boolean_t make_parents,
- svn_boolean_t is_move,
- svn_client_ctx_t *ctx,
- apr_pool_t *pool)
+verify_wc_dsts(const apr_array_header_t *copy_pairs,
+ svn_boolean_t make_parents,
+ svn_boolean_t is_move,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
{
int i;
- apr_pool_t *iterpool = svn_pool_create(pool);
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
- /* Check that all of our SRCs exist, and all the DSTs don't. */
+ /* Check that DST does not exist, but its parent does */
for (i = 0; i < copy_pairs->nelts; i++)
{
svn_client__copy_pair_t *pair = APR_ARRAY_IDX(copy_pairs, i,
@@ -385,15 +388,6 @@ verify_wc_srcs_and_dsts(const apr_array_
svn_pool_clear(iterpool);
- /* Verify that SRC_PATH exists. */
- SVN_ERR(svn_io_check_path(pair->src_abspath_or_url, &pair->src_kind,
- iterpool));
- if (pair->src_kind == svn_node_none)
- return svn_error_createf(
- SVN_ERR_NODE_UNKNOWN_KIND, NULL,
- _("Path '%s' does not exist"),
- svn_dirent_local_style(pair->src_abspath_or_url, pool));
-
/* If DST_PATH does not exist, then its basename will become a new
file or dir added to its parent (possibly an implicit '.').
Else, just error out. */
@@ -419,8 +413,30 @@ verify_wc_srcs_and_dsts(const apr_array_
svn_dirent_local_style(pair->dst_abspath_or_url, iterpool));
}
- if ((! is_not_present)
- && is_move
+ if (! is_not_present)
+ {
+ svn_boolean_t is_deleted;
+
+ SVN_ERR(svn_wc__node_is_status_deleted(&is_deleted, ctx->wc_ctx,
+ pair->dst_abspath_or_url,
+ scratch_pool));
+
+ if (! is_deleted)
+ return svn_error_createf(
+ SVN_ERR_ENTRY_EXISTS, NULL,
+ _("Path '%s' already exists"),
+ svn_dirent_local_style(pair->dst_abspath_or_url,
+ scratch_pool));
+ }
+ }
+
+ /* Check that there is no unversioned obstruction */
+ SVN_ERR(svn_io_check_path(pair->dst_abspath_or_url, &dst_kind,
+ iterpool));
+
+ if (dst_kind != svn_node_none)
+ {
+ if (is_move
&& copy_pairs->nelts == 1
&& strcmp(svn_dirent_dirname(pair->src_abspath_or_url, iterpool),
svn_dirent_dirname(pair->dst_abspath_or_url,
@@ -435,7 +451,7 @@ verify_wc_srcs_and_dsts(const apr_array_
SVN_ERR(svn_path_cstring_from_utf8(&dst,
pair->dst_abspath_or_url,
- pool));
+ scratch_pool));
apr_err = apr_filepath_merge(&dst_apr, NULL, dst,
APR_FILEPATH_TRUENAME, iterpool);
@@ -452,31 +468,22 @@ verify_wc_srcs_and_dsts(const apr_array_
{
/* Ok, we have a single case only rename. Get out of here */
svn_dirent_split(&pair->dst_parent_abspath, &pair->base_name,
- pair->dst_abspath_or_url, pool);
+ pair->dst_abspath_or_url, result_pool);
svn_pool_destroy(iterpool);
return SVN_NO_ERROR;
}
}
- if (! is_not_present)
- {
- svn_boolean_t is_deleted;
-
- SVN_ERR(svn_wc__node_is_status_deleted(&is_deleted, ctx->wc_ctx,
- pair->dst_abspath_or_url,
pool));
-
- if (! is_deleted)
- return svn_error_createf(
+ return svn_error_createf(
SVN_ERR_ENTRY_EXISTS, NULL,
- _("Path '%s' already exists"),
+ _("Path '%s' already exists as unversioned node"),
svn_dirent_local_style(pair->dst_abspath_or_url,
- pool));
- }
+ scratch_pool));
}
svn_dirent_split(&pair->dst_parent_abspath, &pair->base_name,
- pair->dst_abspath_or_url, pool);
+ pair->dst_abspath_or_url, result_pool);
/* Make sure the destination parent is a directory and produce a clear
error message if it is not. */
@@ -502,7 +509,7 @@ verify_wc_srcs_and_dsts(const apr_array_
return svn_error_createf(SVN_ERR_WC_NOT_WORKING_COPY, NULL,
_("Path '%s' is not a directory"),
svn_dirent_local_style(
- pair->dst_parent_abspath, pool));
+ pair->dst_parent_abspath, scratch_pool));
}
}
@@ -511,6 +518,43 @@ verify_wc_srcs_and_dsts(const apr_array_
return SVN_NO_ERROR;
}
+static svn_error_t *
+verify_wc_srcs_and_dsts(const apr_array_header_t *copy_pairs,
+ svn_boolean_t make_parents,
+ svn_boolean_t is_move,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+{
+ int i;
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+
+ /* Check that all of our SRCs exist. */
+ for (i = 0; i < copy_pairs->nelts; i++)
+ {
+ svn_client__copy_pair_t *pair = APR_ARRAY_IDX(copy_pairs, i,
+ svn_client__copy_pair_t *);
+ svn_pool_clear(iterpool);
+
+ /* Verify that SRC_PATH exists. */
+ SVN_ERR(svn_wc_read_kind(&pair->src_kind, ctx->wc_ctx,
+ pair->src_abspath_or_url, FALSE, iterpool));
+ if (pair->src_kind == svn_node_none)
+ return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
+ _("Path '%s' does not exist"),
+ svn_dirent_local_style(
+ pair->src_abspath_or_url,
+ scratch_pool));
+ }
+
+ SVN_ERR(verify_wc_dsts(copy_pairs, make_parents, is_move, ctx,
+ result_pool, iterpool));
+
+ svn_pool_destroy(iterpool);
+
+ return SVN_NO_ERROR;
+}
+
/* Path-specific state used as part of path_driver_cb_baton. */
typedef struct path_driver_info_t
@@ -1638,74 +1682,24 @@ repos_to_wc_copy_locked(const apr_array_
apr_pool_t *scratch_pool)
{
int i;
- const char *src_uuid = NULL, *dst_uuid = NULL;
svn_boolean_t same_repositories;
apr_pool_t *iterpool = svn_pool_create(scratch_pool);
/* We've already checked for physical obstruction by a working file.
But there could also be logical obstruction by an entry whose
working file happens to be missing.*/
- for (i = 0; i < copy_pairs->nelts; i++)
- {
- svn_client__copy_pair_t *pair = APR_ARRAY_IDX(copy_pairs, i,
- svn_client__copy_pair_t *);
- svn_node_kind_t kind;
- svn_boolean_t is_not_present;
- svn_boolean_t is_excluded;
- svn_boolean_t is_server_excluded;
- svn_boolean_t is_deleted;
-
- svn_pool_clear(iterpool);
-
- SVN_ERR(svn_wc_read_kind(&kind, ctx->wc_ctx, pair->dst_abspath_or_url,
- TRUE /* show_hidden */, iterpool));
- if (kind == svn_node_none)
- continue;
-
- /* ### TODO(#2843): Rework these error report. Maybe we can
- ### simplify the conditions? */
-
- /* Hidden by client exclusion */
- SVN_ERR(svn_wc__node_is_not_present(&is_not_present, &is_excluded,
- &is_server_excluded, ctx->wc_ctx,
- pair->dst_abspath_or_url,
- iterpool));
-
- if (is_not_present)
- continue;
-
- if (is_excluded || is_server_excluded)
- {
- return svn_error_createf(
- SVN_ERR_WC_OBSTRUCTED_UPDATE,
- NULL, _("Path '%s' exists, but is excluded"),
- svn_dirent_local_style(pair->dst_abspath_or_url, iterpool));
- }
-
- /* Working file missing to something other than being scheduled
- for addition or in "deleted" state. */
-
- SVN_ERR(svn_wc__node_is_status_deleted(&is_deleted, ctx->wc_ctx,
- pair->dst_abspath_or_url,
- iterpool));
-
- if (! is_deleted)
- {
- return svn_error_createf(
- SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
- _("Path '%s' exists, but the working file is missing"),
- svn_dirent_local_style(pair->dst_abspath_or_url, iterpool));
- }
- }
+ SVN_ERR(verify_wc_dsts(copy_pairs, FALSE, FALSE, ctx,
+ scratch_pool, iterpool));
/* Decide whether the two repositories are the same or not. */
{
svn_error_t *src_err, *dst_err;
const char *parent;
const char *parent_abspath;
+ const char *src_uuid, *dst_uuid;
/* Get the repository uuid of SRC_URL */
- src_err = svn_ra_get_uuid2(ra_session, &src_uuid, scratch_pool);
+ src_err = svn_ra_get_uuid2(ra_session, &src_uuid, iterpool);
if (src_err && src_err->apr_err != SVN_ERR_RA_NO_REPOS_UUID)
return svn_error_trace(src_err);
@@ -1720,7 +1714,7 @@ repos_to_wc_copy_locked(const apr_array_
SVN_ERR(svn_dirent_get_absolute(&parent_abspath, parent, scratch_pool));
dst_err = svn_client_get_repos_root(NULL /* root_url */, &dst_uuid,
parent_abspath, ctx,
- scratch_pool, scratch_pool);
+ iterpool, iterpool);
if (dst_err && dst_err->apr_err != SVN_ERR_RA_NO_REPOS_UUID)
return dst_err;
@@ -1730,7 +1724,6 @@ repos_to_wc_copy_locked(const apr_array_
copy-history is attempted. */
if (src_err || dst_err || (! src_uuid) || (! dst_uuid))
same_repositories = FALSE;
-
else
same_repositories = (strcmp(src_uuid, dst_uuid) == 0);
}
@@ -2167,7 +2160,7 @@ try_copy(const apr_array_header_t *sourc
if ((! srcs_are_urls) && (! dst_is_url))
{
SVN_ERR(verify_wc_srcs_and_dsts(copy_pairs, make_parents, is_move,
- ctx, pool));
+ ctx, pool, pool));
/* Copy or move all targets. */
if (is_move)
Modified: subversion/trunk/subversion/tests/cmdline/copy_tests.py
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/copy_tests.py?rev=1399220&r1=1399219&r2=1399220&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/copy_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/copy_tests.py Wed Oct 17 13:15:39
2012
@@ -1878,12 +1878,14 @@ def mv_unversioned_file(sbox):
# Try to move an unversioned file.
svntest.actions.run_and_verify_svn(None, None,
- ".*unversioned1.* is not under version
control.*",
+ ".*unversioned1' " +
+ "(does not exist|is not under version
control)",
'mv', unver_path_1, dest_path_1)
# Try to forcibly move an unversioned file.
svntest.actions.run_and_verify_svn(None, None,
- ".*unversioned2.* is not under version
control.*",
+ ".*unversioned2.* " +
+ "(does not exist|is not under version
control)",
'mv',
unver_path_2, dest_path_2)
@@ -5457,11 +5459,13 @@ def copy_deleted_dir(sbox):
sbox.simple_rm('A')
svntest.actions.run_and_verify_svn(None, None,
- 'svn: E145000: Path.* does not exist',
+ '(svn: E145000: Path.* does not exist)|' +
+ "(svn: E155035: Deleted node .* copied)",
'cp', sbox.ospath('iota'),
sbox.ospath('new_iota'))
svntest.actions.run_and_verify_svn(None, None,
- 'svn: E145000: Path.* does not exist',
+ '(svn: E145000: Path.* does not exist)|' +
+ "(svn: E155035: Deleted node .* copied)",
'cp', sbox.ospath('A/D'),
sbox.ospath('new_D'))
