Attached is a patch for issue 1199:
http://subversion.tigris.org/issues/show_bug.cgi?id=1199
In spite of the 'bite-sized' keyword, this patch took a fair amount of
work, and I'm not yet certain that it is ready for commit, so I'm
sending it here for review first. The questions I have are:
* The notification is still very sparse, and doesn't say what was
actually committed. For single URL rm, that's not an issue, but here
it is. Suggestions on how to improve the output welcomed (but I don't
want this to hold up the patch).
* I don't know if this added issue 3242-like problems. The approach
is to parent the session at the repos_root, and then operate on all
the paths relative to that. In theory, I don't think this should be a
problem. In practice, I'm not so sure.
Other questions/comments/concerns are of course welcomed, but I'd like
to get this committed sooner, rather than later.
Thanks,
-Hyrum
[[[
Fix issue 1199: Make 'svn rm URL1 URL2 URL3' work, even if the URLs are from
different repositories.
* subversion/tests/cmdline/basic_tests.py
(test_list): Make the basic_rm_urls_multi_repos test pass.
* subversion/libsvn_client/delete.c
(delete_urls): Remove.
(single_repos_delete): New.
(delete_urls_multi_repos): New.
(svn_client_delete4): Call delete_urls_multi_repos(), instead of the old
function.
]]]
Index: subversion/tests/cmdline/basic_tests.py
===================================================================
--- subversion/tests/cmdline/basic_tests.py (revision 967311)
+++ subversion/tests/cmdline/basic_tests.py (working copy)
@@ -2565,7 +2565,7 @@ test_list = [ None,
delete_keep_local_twice,
windows_paths_in_repos,
basic_rm_urls_one_repo,
- XFail(basic_rm_urls_multi_repos),
+ basic_rm_urls_multi_repos,
automatic_conflict_resolution,
info_nonexisting_file,
basic_relative_url_using_current_dir,
Index: subversion/libsvn_client/delete.c
===================================================================
--- subversion/libsvn_client/delete.c (revision 967311)
+++ subversion/libsvn_client/delete.c (working copy)
@@ -134,59 +134,42 @@ path_driver_cb_func(void **dir_baton,
return editor->delete_entry(path, SVN_INVALID_REVNUM, parent_baton, pool);
}
-
static svn_error_t *
-delete_urls(const apr_array_header_t *paths,
- const apr_hash_t *revprop_table,
- svn_client_ctx_t *ctx,
- apr_pool_t *pool)
+single_repos_delete(svn_ra_session_t *ra_session,
+ const char *repos_root,
+ const apr_array_header_t *relpaths,
+ const apr_hash_t *revprop_table,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
{
- svn_ra_session_t *ra_session = NULL;
const svn_delta_editor_t *editor;
+ apr_hash_t *commit_revprops;
void *edit_baton;
const char *log_msg;
- svn_node_kind_t kind;
- apr_array_header_t *targets;
- apr_hash_t *commit_revprops;
+ int i;
svn_error_t *err;
- const char *common;
- int i;
- apr_pool_t *subpool = svn_pool_create(pool);
- /* Condense our list of deletion targets. */
- SVN_ERR(svn_uri_condense_targets(&common, &targets, paths, TRUE,
- pool, pool));
- if (! targets->nelts)
- {
- const char *bname;
- svn_uri_split(&common, &bname, common, pool);
- APR_ARRAY_PUSH(targets, const char *) = svn_path_uri_decode(bname, pool);
- }
-
/* Create new commit items and add them to the array. */
if (SVN_CLIENT__HAS_LOG_MSG_FUNC(ctx))
{
svn_client_commit_item3_t *item;
const char *tmp_file;
apr_array_header_t *commit_items
- = apr_array_make(pool, targets->nelts, sizeof(item));
+ = apr_array_make(pool, relpaths->nelts, sizeof(item));
- for (i = 0; i < targets->nelts; i++)
+ for (i = 0; i < relpaths->nelts; i++)
{
- const char *path = APR_ARRAY_IDX(targets, i, const char *);
+ const char *relpath = APR_ARRAY_IDX(relpaths, i, const char *);
item = svn_client_commit_item3_create(pool);
- item->url = svn_uri_join(common, path, pool);
+ item->url = svn_uri_join(repos_root, relpath, pool);
item->state_flags = SVN_CLIENT_COMMIT_ITEM_DELETE;
APR_ARRAY_PUSH(commit_items, svn_client_commit_item3_t *) = item;
}
SVN_ERR(svn_client__get_log_msg(&log_msg, &tmp_file, commit_items,
ctx, pool));
if (! log_msg)
- {
- svn_pool_destroy(subpool);
- return SVN_NO_ERROR;
- }
+ return SVN_NO_ERROR;
}
else
log_msg = "";
@@ -194,46 +177,9 @@ static svn_error_t *
SVN_ERR(svn_client__ensure_revprop_table(&commit_revprops, revprop_table,
log_msg, ctx, pool));
- /* Verify that each thing to be deleted actually exists (to prevent
- the creation of a revision that has no changes, since the
- filesystem allows for no-op deletes). While here, we'll
- URI-decode our targets. */
- for (i = 0; i < targets->nelts; i++)
- {
- const char *path = APR_ARRAY_IDX(targets, i, const char *);
- const char *item_url;
+ /* Reparent the RA_session to the repos_root. */
+ SVN_ERR(svn_ra_reparent(ra_session, repos_root, pool));
- svn_pool_clear(subpool);
- item_url = svn_path_url_add_component2(common, path, subpool);
- path = svn_path_uri_decode(path, pool);
- APR_ARRAY_IDX(targets, i, const char *) = path;
-
- /* If we've not yet done so, open an RA session for the
- URL. Note that we don't have a local directory, nor a place
- to put temp files. Otherwise, reparent our existing
- session. */
- if (! ra_session)
- {
- SVN_ERR(svn_client__open_ra_session_internal(&ra_session, item_url,
- NULL, NULL, FALSE,
- TRUE, ctx, pool));
- }
- else
- {
- SVN_ERR(svn_ra_reparent(ra_session, item_url, subpool));
- }
-
- SVN_ERR(svn_ra_check_path(ra_session, "", SVN_INVALID_REVNUM,
- &kind, subpool));
- if (kind == svn_node_none)
- return svn_error_createf(SVN_ERR_FS_NOT_FOUND, NULL,
- "URL '%s' does not exist", item_url);
- }
- svn_pool_destroy(subpool);
-
- /* Reparent the RA_session to the common parent of our deletees. */
- SVN_ERR(svn_ra_reparent(ra_session, common, pool));
-
/* Fetch RA commit editor */
SVN_ERR(svn_ra_get_commit_editor3(ra_session, &editor, &edit_baton,
commit_revprops,
@@ -244,7 +190,7 @@ static svn_error_t *
/* Call the path-based editor driver. */
err = svn_delta_path_driver(editor, edit_baton, SVN_INVALID_REVNUM,
- targets, path_driver_cb_func,
+ relpaths, path_driver_cb_func,
(void *)editor, pool);
if (err)
{
@@ -257,6 +203,97 @@ static svn_error_t *
return editor->close_edit(edit_baton, pool);
}
+static svn_error_t *
+delete_urls_multi_repos(const apr_array_header_t *uris,
+ const apr_hash_t *revprop_table,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ apr_hash_t *sessions = apr_hash_make(pool);
+ apr_hash_t *relpaths = apr_hash_make(pool);
+ apr_hash_index_t *hi;
+ int i;
+
+ /* Create a hash of repos_root -> ra_session maps and repos_root -> relpaths
+ maps, used to group the various targets. */
+ for (i = 0; i < uris->nelts; i++)
+ {
+ const char *uri = APR_ARRAY_IDX(uris, i, const char *);
+ svn_ra_session_t *ra_session = NULL;
+ const char *repos_root = NULL;
+ const char *repos_relpath = NULL;
+ apr_array_header_t *relpaths_list;
+ svn_node_kind_t kind;
+
+ for (hi = apr_hash_first(pool, sessions); hi; hi = apr_hash_next(hi))
+ {
+ repos_root = svn__apr_hash_index_key(hi);
+ repos_relpath = svn_uri_is_child(repos_root, uri, pool);
+
+ if (repos_relpath)
+ {
+ /* Great! We've found another uri underneath this session,
+ store it and move on. */
+ ra_session = svn__apr_hash_index_val(hi);
+ relpaths_list = apr_hash_get(relpaths, repos_root,
+ APR_HASH_KEY_STRING);
+
+ APR_ARRAY_PUSH(relpaths_list, const char *) =
+ svn_path_uri_decode(repos_relpath, pool);
+
+ break;
+ }
+ }
+
+ if (!ra_session)
+ {
+ /* If we haven't found a session yet, we need to open one up.
+ Note that we don't have a local directory, nor a place
+ to put temp files. */
+ SVN_ERR(svn_client__open_ra_session_internal(&ra_session, uri,
+ NULL, NULL, FALSE,
+ TRUE, ctx, pool));
+ SVN_ERR(svn_ra_get_repos_root2(ra_session, &repos_root, pool));
+
+ apr_hash_set(sessions, repos_root, APR_HASH_KEY_STRING, ra_session);
+ repos_relpath = svn_uri_is_child(repos_root, uri, pool);
+
+ relpaths_list = apr_array_make(pool, 1, sizeof(const char *));
+ apr_hash_set(relpaths, repos_root, APR_HASH_KEY_STRING,
+ relpaths_list);
+ APR_ARRAY_PUSH(relpaths_list, const char *) =
+ svn_path_uri_decode(repos_relpath, pool);
+ }
+
+ /* Now, test to see if the thing actually exists. */
+ SVN_ERR(svn_ra_reparent(ra_session, uri, pool));
+ SVN_ERR(svn_ra_check_path(ra_session, "", SVN_INVALID_REVNUM, &kind,
+ pool));
+ if (kind == svn_node_none)
+ return svn_error_createf(SVN_ERR_FS_NOT_FOUND, NULL,
+ "URL '%s' does not exist", uri);
+ }
+
+ /* At this point, we should have two hashs:
+ SESSIONS maps repos_roots to ra_sessions.
+ RELPATHS maps repos_roots to a list of decoded relpaths for that root.
+
+ Now we iterate over the collection of sessions and do a commit for each
+ one with the collected relpaths. */
+ for (hi = apr_hash_first(pool, sessions); hi; hi = apr_hash_next(hi))
+ {
+ const char *repos_root = svn__apr_hash_index_key(hi);
+ svn_ra_session_t *ra_session = svn__apr_hash_index_val(hi);
+ apr_array_header_t *relpaths_list = apr_hash_get(relpaths, repos_root,
+ APR_HASH_KEY_STRING);
+
+ SVN_ERR(single_repos_delete(ra_session, repos_root, relpaths_list,
+ revprop_table, ctx, pool));
+ }
+
+ return SVN_NO_ERROR;
+}
+
svn_error_t *
svn_client__wc_delete(const char *path,
svn_boolean_t force,
@@ -323,7 +360,7 @@ svn_client_delete4(const apr_array_header_t *paths
if (svn_path_is_url(APR_ARRAY_IDX(paths, 0, const char *)))
{
- SVN_ERR(delete_urls(paths, revprop_table, ctx, pool));
+ SVN_ERR(delete_urls_multi_repos(paths, revprop_table, ctx, pool));
}
else
{
