Author: rhuijben
Date: Wed May 4 11:03:35 2011
New Revision: 1099411
URL: http://svn.apache.org/viewvc?rev=1099411&view=rev
Log:
In the commit harvester: Stop handling not-present descandants of a copy as
a local operation. Handle them as part of the operation root instead.
This will allow enabling the non-recursive copy commit and it also makes
it easy to resolve issue #3314.
* subversion/libsvn_client/client.h
(svn_client__check_url_kind_t): New typedef.
(svn_client__harvest_committables,
svn_client__get_copy_committables): Add check url kind callback and baton.
* subversion/libsvn_client/commit.c
(check_url_kind_baton): New struct.
(check_url_kind): New function.
(svn_client_commit5): Pass check_url_kind to
svn_client__harvest_committables.
* subversion/libsvn_client/commit_util.c
(harvest_committables): Skip all non-operational deletes.
(handle_descendants_baton): New struct.
(svn_client__harvest_committables): Call handle_descendants over all items.
(copy_committables_baton): Add callback.
(harvest_copy_committables): Call handle_descendants over all items.
(svn_client__get_copy_committables): Store callbacks in baton.
* subversion/libsvn_client/copy.c
(wc_to_repos_copy): Update caller. Just leave a TODO as this issue is less
relevant, because copy is always recursive.
* subversion/tests/cmdline/copy_tests.py
(wc_to_wc_copy_deleted): Expect that the commit notices that these paths
don't exist.
(mixed_rev_copy_del): Remove XFail.
Modified:
subversion/trunk/subversion/libsvn_client/client.h
subversion/trunk/subversion/libsvn_client/commit.c
subversion/trunk/subversion/libsvn_client/commit_util.c
subversion/trunk/subversion/libsvn_client/copy.c
subversion/trunk/subversion/tests/cmdline/copy_tests.py
Modified: subversion/trunk/subversion/libsvn_client/client.h
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/client.h?rev=1099411&r1=1099410&r2=1099411&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/client.h (original)
+++ subversion/trunk/subversion/libsvn_client/client.h Wed May 4 11:03:35 2011
@@ -777,6 +777,13 @@ typedef struct svn_client__copy_pair_t
*/
+/* Callback for the commit harvester to check if a node exists at the specified
+ url */
+typedef svn_error_t *(*svn_client__check_url_kind_t)(void *baton,
+ svn_node_kind_t *kind,
+ const char *url,
+ apr_pool_t *scratch_pool);
+
/* Recursively crawl a set of working copy paths (DIR_ABSPATH + each
item in the TARGETS array) looking for commit candidates, locking
working copy directories as the crawl progresses. For each
@@ -828,6 +835,8 @@ svn_client__harvest_committables(apr_has
svn_depth_t depth,
svn_boolean_t just_locked,
const apr_array_header_t *changelists,
+ svn_client__check_url_kind_t check_url_func,
+ void *check_url_baton,
svn_client_ctx_t *ctx,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
@@ -845,11 +854,12 @@ svn_client__harvest_committables(apr_has
svn_error_t *
svn_client__get_copy_committables(apr_hash_t **committables,
const apr_array_header_t *copy_pairs,
+ svn_client__check_url_kind_t check_url_func,
+ void *check_url_baton,
svn_client_ctx_t *ctx,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool);
-
/* A qsort()-compatible sort routine for sorting an array of
svn_client_commit_item_t's by their URL member. */
int svn_client__sort_commit_item_urls(const void *a, const void *b);
Modified: subversion/trunk/subversion/libsvn_client/commit.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/commit.c?rev=1099411&r1=1099410&r2=1099411&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/commit.c (original)
+++ subversion/trunk/subversion/libsvn_client/commit.c Wed May 4 11:03:35 2011
@@ -1129,6 +1129,39 @@ determine_lock_targets(apr_array_header_
return SVN_NO_ERROR;
}
+/* Baton for check_url_kind */
+struct check_url_kind_baton
+{
+ apr_pool_t *pool;
+ svn_ra_session_t *session;
+ const char *repos_root_url;
+ svn_client_ctx_t *ctx;
+};
+
+/* Implements svn_client__check_url_kind_t for svn_client_commit5 */
+static svn_error_t *
+check_url_kind(void *baton,
+ svn_node_kind_t *kind,
+ const char *url,
+ apr_pool_t *scratch_pool)
+{
+ struct check_url_kind_baton *cukb = baton;
+
+ /* If we don't have a session or can't use the session, get one */
+ if (!cukb->session || !svn_uri_is_ancestor(cukb->repos_root_url, url))
+ {
+ SVN_ERR(svn_client_open_ra_session(&cukb->session, url, cukb->ctx,
+ cukb->pool));
+ SVN_ERR(svn_ra_get_repos_root2(cukb->session, &cukb->repos_root_url,
+ cukb->pool));
+ }
+ else
+ SVN_ERR(svn_ra_reparent(cukb->session, url, scratch_pool));
+
+ return svn_error_return(
+ svn_ra_check_path(cukb->session, "", SVN_INVALID_REVNUM,
+ kind, scratch_pool));
+}
svn_error_t *
svn_client_commit5(const apr_array_header_t *targets,
@@ -1250,17 +1283,29 @@ svn_client_commit5(const apr_array_heade
}
/* Crawl the working copy for commit items. */
- cmt_err = svn_error_return(
- svn_client__harvest_committables(&committables,
- &lock_tokens,
- base_abspath,
- rel_targets,
- depth,
- ! keep_locks,
- changelists,
- ctx,
- pool,
- iterpool));
+ {
+ struct check_url_kind_baton cukb;
+
+ /* Prepare for when we have a copy containing not-present nodes. */
+ cukb.pool = iterpool;
+ cukb.session = NULL; /* ### Can we somehow reuse session? */
+ cukb.repos_root_url = NULL;
+ cukb.ctx = ctx;
+
+ cmt_err = svn_error_return(
+ svn_client__harvest_committables(&committables,
+ &lock_tokens,
+ base_abspath,
+ rel_targets,
+ depth,
+ ! keep_locks,
+ changelists,
+ check_url_kind,
+ &cukb,
+ ctx,
+ pool,
+ iterpool));
+ }
if (cmt_err)
goto cleanup;
Modified: subversion/trunk/subversion/libsvn_client/commit_util.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/commit_util.c?rev=1099411&r1=1099410&r2=1099411&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/commit_util.c (original)
+++ subversion/trunk/subversion/libsvn_client/commit_util.c Wed May 4 11:03:35
2011
@@ -475,6 +475,9 @@ harvest_committables(svn_wc_context_t *w
}
}
+ if (is_deleted && !is_op_root /* && !is_added */)
+ return SVN_NO_ERROR; /* Not an operational delete and not an add. */
+
if (node_relpath == NULL)
SVN_ERR(svn_wc__node_get_repos_relpath(&node_relpath,
wc_ctx, local_abspath,
@@ -722,6 +725,119 @@ harvest_committables(svn_wc_context_t *w
return SVN_NO_ERROR;
}
+/* Baton for handle_descendants */
+struct handle_descendants_baton
+{
+ svn_wc_context_t *wc_ctx;
+ svn_cancel_func_t cancel_func;
+ void *cancel_baton;
+ svn_client__check_url_kind_t check_url_func;
+ void *check_url_baton;
+};
+
+/* Helper for the commit harvesters */
+static svn_error_t *
+handle_descendants(void *baton,
+ const void *key, apr_ssize_t klen, void *val,
+ apr_pool_t *pool)
+{
+ struct handle_descendants_baton *hdb = baton;
+ apr_array_header_t *commit_items = val;
+ apr_pool_t *iterpool = svn_pool_create(pool);
+ int i;
+
+ for (i = 0; i < commit_items->nelts; i++)
+ {
+ svn_client_commit_item3_t *item =
+ APR_ARRAY_IDX(commit_items, i, svn_client_commit_item3_t *);
+ const apr_array_header_t *absent_descendants;
+ int j;
+
+ /* Is this a copy operation? */
+ if (!(item->state_flags & SVN_CLIENT_COMMIT_ITEM_ADD)
+ || ! item->copyfrom_url)
+ continue;
+
+ if (hdb->cancel_func)
+ SVN_ERR(hdb->cancel_func(hdb->cancel_baton));
+
+ svn_pool_clear(iterpool);
+
+ SVN_ERR(svn_wc__get_not_present_descendants(&absent_descendants,
+ hdb->wc_ctx, item->path,
+ iterpool, iterpool));
+
+ for (j = 0; j < absent_descendants->nelts; j++)
+ {
+ int k;
+ svn_boolean_t found_item = FALSE;
+ svn_node_kind_t kind;
+ const char *relpath = APR_ARRAY_IDX(absent_descendants, j,
+ const char *);
+ const char *local_abspath = svn_dirent_join(item->path, relpath,
+ iterpool);
+
+ /* If the path has a commit operation, we do nothing.
+ (It will be deleted by the operation) */
+ for (k = 0; k < commit_items->nelts; k++)
+ {
+ svn_client_commit_item3_t *cmt_item =
+ APR_ARRAY_IDX(commit_items, k, svn_client_commit_item3_t *);
+
+ if (! strcmp(cmt_item->path, local_abspath))
+ {
+ found_item = TRUE;
+ break;
+ }
+ }
+
+ if (found_item)
+ continue; /* We have an explicit delete or replace for this path */
+
+ /* ### Need a sub-iterpool? */
+
+ if (hdb->check_url_func)
+ {
+ const char *from_url = svn_path_url_add_component2(
+ item->copyfrom_url, relpath,
+ iterpool);
+
+ SVN_ERR(hdb->check_url_func(hdb->check_url_baton,
+ &kind, from_url, iterpool));
+
+ if (kind == svn_node_none)
+ continue; /* This node is already deleted */
+ }
+ else
+ kind = svn_node_unknown; /* 'Ok' for a delete of something */
+
+ {
+ /* Add a new commit item that describes the delete */
+ apr_pool_t *result_pool = commit_items->pool;
+ svn_client_commit_item3_t *new_item
+ = svn_client_commit_item3_create(result_pool);
+
+ new_item->path = svn_dirent_join(item->path, relpath,
+ result_pool);
+ new_item->kind = kind;
+ new_item->url = svn_path_url_add_component2(item->url, relpath,
+ result_pool);
+ new_item->revision = SVN_INVALID_REVNUM;
+ new_item->state_flags = SVN_CLIENT_COMMIT_ITEM_DELETE;
+ new_item->incoming_prop_changes = apr_array_make(result_pool, 1,
+ sizeof(svn_prop_t *));
+
+ APR_ARRAY_PUSH(commit_items, svn_client_commit_item3_t *)
+ = new_item;
+ }
+ }
+ }
+
+ svn_pool_destroy(iterpool);
+ return SVN_NO_ERROR;
+}
+
+
/* BATON is an apr_hash_t * of harvested committables. */
static svn_error_t *
@@ -758,6 +874,8 @@ svn_client__harvest_committables(apr_has
svn_depth_t depth,
svn_boolean_t just_locked,
const apr_array_header_t *changelists,
+ svn_client__check_url_kind_t check_url_func,
+ void *check_url_baton,
svn_client_ctx_t *ctx,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool)
@@ -766,6 +884,7 @@ svn_client__harvest_committables(apr_has
apr_pool_t *iterpool = svn_pool_create(scratch_pool);
apr_hash_t *changelist_hash = NULL;
svn_wc_context_t *wc_ctx = ctx->wc_ctx;
+ struct handle_descendants_baton hdb;
/* It's possible that one of the named targets has a parent that is
* itself scheduled for addition or replacement -- that is, the
@@ -903,6 +1022,15 @@ svn_client__harvest_committables(apr_has
result_pool, iterpool));
}
+ hdb.wc_ctx = ctx->wc_ctx;
+ hdb.cancel_func = ctx->cancel_func;
+ hdb.cancel_baton = ctx->cancel_baton;
+ hdb.check_url_func = check_url_func;
+ hdb.check_url_baton = check_url_baton;
+
+ SVN_ERR(svn_iter_apr_hash(NULL, *committables,
+ handle_descendants, &hdb, iterpool));
+
/* Make sure that every path in danglers is part of the commit. */
SVN_ERR(svn_iter_apr_hash(NULL,
danglers, validate_dangler, *committables,
@@ -918,6 +1046,8 @@ struct copy_committables_baton
svn_client_ctx_t *ctx;
apr_hash_t *committables;
apr_pool_t *result_pool;
+ svn_client__check_url_kind_t check_url_func;
+ void *check_url_baton;
};
static svn_error_t *
@@ -927,6 +1057,7 @@ harvest_copy_committables(void *baton, v
svn_client__copy_pair_t *pair = *(svn_client__copy_pair_t **)item;
const char *repos_root_url;
const char *commit_relpath;
+ struct handle_descendants_baton hdb;
/* Read the entry for this SRC. */
SVN_ERR_ASSERT(svn_dirent_is_absolute(pair->src_abspath_or_url));
@@ -941,19 +1072,30 @@ harvest_copy_committables(void *baton, v
pool);
/* Handle this SRC. */
- return harvest_committables(btn->ctx->wc_ctx,
- pair->src_abspath_or_url,
- btn->committables, NULL,
- repos_root_url,
- commit_relpath,
- TRUE, /* COPY_MODE_ROOT */
- svn_depth_infinity,
- FALSE, /* JUST_LOCKED */
- NULL,
- FALSE, FALSE, /* skip files, dirs */
- btn->ctx->cancel_func,
- btn->ctx->cancel_baton,
- btn->result_pool, pool);
+ SVN_ERR(harvest_committables(btn->ctx->wc_ctx,
+ pair->src_abspath_or_url,
+ btn->committables, NULL,
+ repos_root_url,
+ commit_relpath,
+ TRUE, /* COPY_MODE_ROOT */
+ svn_depth_infinity,
+ FALSE, /* JUST_LOCKED */
+ NULL,
+ FALSE, FALSE, /* skip files, dirs */
+ btn->ctx->cancel_func,
+ btn->ctx->cancel_baton,
+ btn->result_pool, pool));
+
+ hdb.wc_ctx = btn->ctx->wc_ctx;
+ hdb.cancel_func = btn->ctx->cancel_func;
+ hdb.cancel_baton = btn->ctx->cancel_baton;
+ hdb.check_url_func = btn->check_url_func;
+ hdb.check_url_baton = btn->check_url_baton;
+
+ SVN_ERR(svn_iter_apr_hash(NULL, btn->committables,
+ handle_descendants, &hdb, pool));
+
+ return SVN_NO_ERROR;
}
@@ -961,6 +1103,8 @@ harvest_copy_committables(void *baton, v
svn_error_t *
svn_client__get_copy_committables(apr_hash_t **committables,
const apr_array_header_t *copy_pairs,
+ svn_client__check_url_kind_t check_url_func,
+ void *check_url_baton,
svn_client_ctx_t *ctx,
apr_pool_t *result_pool,
apr_pool_t *scratch_pool)
@@ -973,6 +1117,9 @@ svn_client__get_copy_committables(apr_ha
btn.committables = *committables;
btn.result_pool = result_pool;
+ btn.check_url_func = check_url_func;
+ btn.check_url_baton = check_url_baton;
+
/* For each copy pair, harvest the committables for that pair into the
committables hash. */
return svn_iter_apr_array(NULL, copy_pairs,
Modified: subversion/trunk/subversion/libsvn_client/copy.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/copy.c?rev=1099411&r1=1099410&r2=1099411&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/copy.c (original)
+++ subversion/trunk/subversion/libsvn_client/copy.c Wed May 4 11:03:35 2011
@@ -1291,8 +1291,10 @@ wc_to_repos_copy(const apr_array_header_
message, ctx, pool));
/* Crawl the working copy for commit items. */
+ /* ### TODO: Pass check_url_func for issue #3314 handling */
SVN_ERR(svn_client__get_copy_committables(&committables,
copy_pairs,
+ NULL, NULL, /* check_url_func */
ctx, pool, pool));
/* The committables are keyed by the repository root */
Modified: subversion/trunk/subversion/tests/cmdline/copy_tests.py
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/copy_tests.py?rev=1099411&r1=1099410&r2=1099411&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/copy_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/copy_tests.py Wed May 4 11:03:35
2011
@@ -1583,9 +1583,11 @@ def wc_to_wc_copy_deleted(sbox):
copied=None, wc_rev=3)
expected_output = svntest.wc.State(wc_dir, {
'A/B2' : Item(verb='Adding'),
- 'A/B2/E/alpha' : Item(verb='Deleting'),
- 'A/B2/lambda' : Item(verb='Deleting'),
- 'A/B2/F' : Item(verb='Deleting'),
+ # Before the commit processor verified not-present deletes
+ # the output would also contain
+ # 'A/B2/E/alpha' : Item(verb='Deleting'),
+ # 'A/B2/lambda' : Item(verb='Deleting'),
+ # 'A/B2/F' : Item(verb='Deleting'),
})
svntest.actions.run_and_verify_commit(wc_dir,
@@ -4697,7 +4699,6 @@ def copy_over_deleted_dir(sbox):
main.run_svn(None, 'cp', os.path.join(sbox.wc_dir, 'A/D'),
os.path.join(sbox.wc_dir, 'A/B'))
-@XFail()
@Issue(3314)
def mixed_rev_copy_del(sbox):
"""copy mixed-rev and delete children"""
