Hi,
attached is a patch I'm working on to solve issue 1199: "svn rm url1
url2 url3 should use a single transaction per repository".
As you can see from my last comment on that issue, my patch is purely
path-based, it doesn't use the repo UUID's. Basically this is the behavior:
1. Take the list of url's, sort them and filter out the duplicates
2. Get the log message from the callback, all commits to all
repositories will share the same log message.
3. Open a ra_session to the first url in the list, reparent the session
to the repository root.
4. Iterate over the sorted list of urls, and add all the url's in the
open repository to the list of commit items
5. Commit those items.
6. Go back to step 3, handle the remaining url's.
The existing svn_path_condense_targets wasn't really useful, since that
finds a common ancestor outside of the repositories. In fact, before
this patch, if you remotely try to delete some directories where one
doesn't exist, the error message will show a path relative to the common
ancestor instead of the root of the repository.
Open ends:
1. There is no way now to give the user a clear log of what has
happened. svn_client_delete3 only receives info on the last commit, so
it only prints that revision number.
I'd like to have it print something like this:
$ svn rm http://srv/repos1/A http://srv/repos1/B http://srv/repos2/X
D http://srv/repos1/A
D http://srv/repos1/B
Committed revision 2.
D http://srv/repos2/X
Committed revision 5.
I didn't look into it more deeply, so that might be easy to do or not.
Suggestions are welcome.
2. The issue report talks about using the UUID to match paths with
repositories, but I fail to see the reason why that would be needed. Is
there anything wrong with my naive path-based implementation?
3. I have to write some unit tests for the two new path functions and
add some comments.
Lieven
Index: subversion/include/svn_path.h
===================================================================
--- subversion/include/svn_path.h (revision 25174)
+++ subversion/include/svn_path.h (working copy)
@@ -295,7 +295,28 @@
const apr_array_header_t *targets,
svn_boolean_t remove_redundancies,
apr_pool_t *pool);
+/**
+ * TODO: write comment
+ *
+ * @since New in 1.5.
+ */
+svn_error_t *
+svn_path_condense_targets_to_path(const char **pcommon,
+ const char *expected_root,
+ apr_array_header_t **pcondensed_targets,
+ const apr_array_header_t *targets,
+ svn_boolean_t remove_redundancies,
+ apr_pool_t *pool);
+/**
+ * TODO: write comment
+ *
+ * @since New in 1.5.
+ */
+svn_error_t *
+svn_path_sort_and_remove_duplicates(apr_array_header_t **pcondensed_targets,
+ const apr_array_header_t *targets,
+ apr_pool_t *pool);
/** Copy a list of canonicalized @a targets, one at a time, into @a
* pcondensed_targets, omitting any targets that are found earlier in
Index: subversion/libsvn_client/delete.c
===================================================================
--- subversion/libsvn_client/delete.c (revision 25174)
+++ subversion/libsvn_client/delete.c (working copy)
@@ -128,20 +128,12 @@
const char *log_msg;
apr_hash_t *revprop_table;
svn_node_kind_t kind;
- apr_array_header_t *targets;
+ apr_array_header_t *sfpaths, *targets;
svn_error_t *err;
- const char *common;
- int i;
+ int i, pathidx = 0;
apr_pool_t *subpool = svn_pool_create(pool);
- /* Condense our list of deletion targets. */
- SVN_ERR(svn_path_condense_targets(&common, &targets, paths, TRUE, pool));
- if (! targets->nelts)
- {
- const char *bname;
- svn_path_split(common, &common, &bname, pool);
- APR_ARRAY_PUSH(targets, const char *) = bname;
- }
+ svn_path_sort_and_remove_duplicates(&sfpaths, paths, pool);
/* Create new commit items and add them to the array. */
if (SVN_CLIENT__HAS_LOG_MSG_FUNC(ctx))
@@ -149,14 +141,13 @@
svn_client_commit_item3_t *item;
const char *tmp_file;
apr_array_header_t *commit_items
- = apr_array_make(pool, targets->nelts, sizeof(item));
-
- for (i = 0; i < targets->nelts; i++)
+ = apr_array_make(pool, sfpaths->nelts, sizeof(item));
+
+ for (i = 0; i < sfpaths->nelts; i++)
{
- const char *path = APR_ARRAY_IDX(targets, i, const char *);
SVN_ERR(svn_client_commit_item_create
((const svn_client_commit_item3_t **) &item, pool));
- item->url = svn_path_join(common, path, pool);
+ item->url = APR_ARRAY_IDX(sfpaths, i, const char *);
item->state_flags = SVN_CLIENT_COMMIT_ITEM_DELETE;
APR_ARRAY_PUSH(commit_items, svn_client_commit_item3_t *) = item;
}
@@ -173,52 +164,83 @@
SVN_ERR(svn_client__get_revprop_table(&revprop_table, log_msg, ctx, pool));
- /* Open an RA session for the URL. 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, common, NULL,
- NULL, NULL, FALSE, TRUE,
- ctx, pool));
+ /* Group the commits per repository */
+ while (pathidx < sfpaths->nelts)
+ {
+ const char *url, *common, *repos_root;
+ apr_array_header_t *abs_targets;
- /* 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). */
- for (i = 0; i < targets->nelts; i++)
- {
- const char *path = APR_ARRAY_IDX(targets, i, const char *);
+ url = svn_path_dirname(APR_ARRAY_IDX(sfpaths, pathidx, const char *),
+ pool);
+ /* Open an RA session for the URL. 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, url, NULL,
+ NULL, NULL, FALSE, TRUE,
+ ctx, pool));
+ SVN_ERR(svn_ra_get_repos_root(ra_session, &repos_root, pool));
+
+ /* We need to reparent the session to the repository root here,
+ otherwise all our paths would have to be relative to the first
+ target for this repository. */
+ SVN_ERR(svn_ra_reparent(ra_session, repos_root, pool));
+
+ /* Create a list of absolute paths within this repository. Update pathidx
+ to match the first target from another repository */
+ abs_targets = apr_array_make(pool, sfpaths->nelts, sizeof(const char *));
+ for ( ; pathidx < sfpaths->nelts ; pathidx++)
+ {
+ const char *path = APR_ARRAY_IDX(sfpaths, pathidx, const char *);
+ if (! svn_path_is_ancestor(repos_root, path))
+ break;
+ APR_ARRAY_PUSH(abs_targets, const char *) = path;
+ }
+
+ SVN_ERR(svn_path_condense_targets_to_path(&common, repos_root, &targets,
+ abs_targets, TRUE, 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). */
+ for (i = 0; i < targets->nelts; i++)
+ {
+ const char *path = APR_ARRAY_IDX(targets, i, const char *);
+ svn_pool_clear(subpool);
+ path = svn_path_uri_decode(path, pool);
+ APR_ARRAY_IDX(targets, i, const char *) = path;
+ SVN_ERR(svn_ra_check_path(ra_session, path, 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",
+ svn_path_local_style(path, pool));
+ }
svn_pool_clear(subpool);
- path = svn_path_uri_decode(path, pool);
- APR_ARRAY_IDX(targets, i, const char *) = path;
- SVN_ERR(svn_ra_check_path(ra_session, path, 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",
- svn_path_local_style(path, pool));
- }
- svn_pool_destroy(subpool);
- /* Fetch RA commit editor */
- SVN_ERR(svn_client__commit_get_baton(&commit_baton, commit_info_p, pool));
- SVN_ERR(svn_ra_get_commit_editor3(ra_session, &editor, &edit_baton,
- revprop_table,
- svn_client__commit_callback,
- commit_baton,
- NULL, TRUE, /* No lock tokens */
- pool));
+ /* Fetch RA commit editor */
+ SVN_ERR(svn_client__commit_get_baton(&commit_baton, commit_info_p, pool));
+ SVN_ERR(svn_ra_get_commit_editor3(ra_session, &editor, &edit_baton,
+ revprop_table,
+ svn_client__commit_callback,
+ commit_baton,
+ NULL, TRUE, /* No lock tokens */
+ pool));
- /* Call the path-based editor driver. */
- err = svn_delta_path_driver(editor, edit_baton, SVN_INVALID_REVNUM,
- targets, path_driver_cb_func,
- (void *)editor, pool);
- if (err)
- {
- /* At least try to abort the edit (and fs txn) before throwing err. */
- svn_error_clear(editor->abort_edit(edit_baton, pool));
- return err;
+ /* Call the path-based editor driver. */
+ err = svn_delta_path_driver(editor, edit_baton, SVN_INVALID_REVNUM,
+ targets, path_driver_cb_func,
+ (void *)editor, pool);
+ if (err)
+ {
+ /* At least try to abort the edit (and fs txn) before throwing err. */
+ svn_error_clear(editor->abort_edit(edit_baton, pool));
+ return err;
+ }
+
+ /* Close the edit. */
+ SVN_ERR(editor->close_edit(edit_baton, pool));
}
- /* Close the edit. */
- SVN_ERR(editor->close_edit(edit_baton, pool));
+ svn_pool_destroy(subpool);
return SVN_NO_ERROR;
}
Index: subversion/libsvn_subr/target.c
===================================================================
--- subversion/libsvn_subr/target.c (revision 25174)
+++ subversion/libsvn_subr/target.c (working copy)
@@ -26,6 +26,7 @@
#include "svn_pools.h"
#include "svn_error.h"
#include "svn_path.h"
+#include "svn_sorts.h"
/*** Code. ***/
@@ -189,6 +190,46 @@
svn_error_t *
+svn_path_condense_targets_to_path(const char **pcommon,
+ const char *expected_root,
+ apr_array_header_t **pcondensed_targets,
+ const apr_array_header_t *targets,
+ svn_boolean_t remove_redundancies,
+ apr_pool_t *pool)
+{
+ SVN_ERR(svn_path_condense_targets(pcommon, pcondensed_targets,
+ targets, remove_redundancies, pool));
+ if (strcmp(*pcommon, expected_root) > 0)
+ {
+ int i;
+
+ /* The common ancestor is deeper as our expected root, so cut the extra
+ path from *pcommon and add it to all the relative paths in targets. */
+ apr_size_t exp_len = strlen(expected_root);
+ apr_size_t d = strlen(*pcommon) - exp_len;
+
+ if ((*pcondensed_targets)->nelts == 0)
+ {
+ char *rel = apr_pstrmemdup(pool, *pcommon + exp_len + 1, d - 1);
+ APR_ARRAY_PUSH(*pcondensed_targets, const char *) = rel;
+ }
+ else
+ {
+ for (i = 0; i < (*pcondensed_targets)->nelts; i++)
+ {
+ const char *rel_path = APR_ARRAY_IDX(*pcondensed_targets, i,
+ const char *);
+ APR_ARRAY_IDX(*pcondensed_targets, i, const char *)
+ = svn_path_join(*pcommon + exp_len + 1, rel_path, pool);
+ }
+ }
+ *pcommon = apr_pstrdup(pool, expected_root);
+ }
+
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
svn_path_remove_redundancies(apr_array_header_t **pcondensed_targets,
const apr_array_header_t *targets,
apr_pool_t *pool)
@@ -275,3 +316,48 @@
return SVN_NO_ERROR;
}
+
+svn_error_t *
+svn_path_sort_and_remove_duplicates(apr_array_header_t **pcondensed_targets,
+ const apr_array_header_t *targets,
+ apr_pool_t *pool)
+{
+ apr_array_header_t *new_targets;
+ const char *last_path;
+ int i = 0;
+
+ if ((targets->nelts <= 0) || (! pcondensed_targets))
+ {
+ /* No targets or no place to store our work means this function
+ really has nothing to do. */
+ if (pcondensed_targets)
+ *pcondensed_targets = NULL;
+ return SVN_NO_ERROR;
+ }
+
+ /* Sort the paths in a depth-last order. */
+ qsort(targets->elts, targets->nelts,
+ targets->elt_size, svn_sort_compare_paths);
+
+ new_targets = apr_array_make(pool, targets->nelts,
+ sizeof(const char *));
+
+ last_path = APR_ARRAY_IDX(targets, i, const char *);
+ APR_ARRAY_PUSH(new_targets, const char *) = last_path;
+
+ /* Copy all paths except duplicates */
+ for (i = 1; i < targets->nelts; i++)
+ {
+ const char *path = APR_ARRAY_IDX(targets, i, const char *);
+
+ if (strcmp(path, last_path) == 0)
+ continue;
+
+ APR_ARRAY_PUSH(new_targets, const char *) = path;
+ last_path = path;
+ }
+
+ *pcondensed_targets = new_targets;
+
+ return SVN_NO_ERROR;
+}
\ No newline at end of file
Index: subversion/tests/cmdline/basic_tests.py
===================================================================
--- subversion/tests/cmdline/basic_tests.py (revision 25174)
+++ subversion/tests/cmdline/basic_tests.py (working copy)
@@ -2035,7 +2035,7 @@
delete_keep_local,
windows_paths_in_repos,
basic_rm_urls_one_repo,
- XFail(basic_rm_urls_multi_repos),
+ basic_rm_urls_multi_repos,
]
if __name__ == '__main__':
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon May 28 22:06:43 2007