Does the attached patch look reasonable? It provides an incremental version of
the existing svn_delta_path_driver2() function, in which the caller can provide
one path at a time instead of all at once at the start.
We have a few (5 ~ 10) uses of svn_delta_path_driver2() in our codebase and I
think a few more places already where an incremental path driver could be used
if it existed, in addition to me now wanting to use it in "unshelve" as I
mentioned the other day. I will try doing that, and, if/when working usefully,
will commit it, along with any changes suggested here.
It already works as a factored-out body of svn_delta_path_driver2(): tests pass
with this patch applied.
In svn_delta_path_driver2() there is some special handling of the root path,
which we probably want to include in the incremental driver too; I just haven't
got around to studying and deciding on that part yet.
--
- Julian
Teach the delta editor path driver to work incrementally.
Instead of passing in the complete list of paths to be driven all at once,
this adds the option of passing in one path at a time.
* subversion/include/svn_delta.h,
subversion/libsvn_delta/path_driver.c
(svn_delta_path_driver_state_t,
svn_delta_path_driver_start,
svn_delta_path_driver_step,
svn_delta_path_driver_finish): New.
(svn_delta_path_driver2): Rewrite to use the incremental API.
--This line, and those below, will be ignored--
Index: subversion/include/svn_delta.h
===================================================================
--- subversion/include/svn_delta.h (revision 1851442)
+++ subversion/include/svn_delta.h (working copy)
@@ -1309,12 +1309,31 @@ typedef svn_error_t *(*svn_delta_path_dr
void *parent_baton,
void *callback_baton,
const char *path,
apr_pool_t *pool);
+typedef struct svn_delta_path_driver_state_t svn_delta_path_driver_state_t;
+
+svn_error_t *
+svn_delta_path_driver_start(svn_delta_path_driver_state_t **state_p,
+ const svn_delta_editor_t *editor,
+ void *edit_baton,
+ svn_delta_path_driver_cb_func_t callback_func,
+ void *callback_baton,
+ apr_pool_t *pool);
+
+svn_error_t *
+svn_delta_path_driver_step(svn_delta_path_driver_state_t *state,
+ const char *path,
+ apr_pool_t *iterpool);
+
+svn_error_t *
+svn_delta_path_driver_finish(svn_delta_path_driver_state_t *state,
+ apr_pool_t *pool);
+
/** Drive @a editor (with its @a edit_baton) to visit each path in @a paths.
* As each path is hit as part of the editor drive, use
* @a callback_func and @a callback_baton to allow the caller to handle
* the portion of the editor drive related to that path.
*
* Each path in @a paths is a (const char *) relpath, relative
Index: subversion/libsvn_delta/path_driver.c
===================================================================
--- subversion/libsvn_delta/path_driver.c (revision 1851442)
+++ subversion/libsvn_delta/path_driver.c (working copy)
@@ -127,28 +127,59 @@ count_components(const char *path)
return count;
}
/*** Public interfaces ***/
+struct svn_delta_path_driver_state_t
+{
+ const svn_delta_editor_t *editor;
+ void *edit_baton;
+ svn_delta_path_driver_cb_func_t callback_func;
+ void *callback_baton;
+ apr_array_header_t *db_stack;
+ const char *last_path;
+ apr_pool_t *pool; /* at least the lifetime of the entire drive */
+};
+
+svn_error_t *
+svn_delta_path_driver_start(svn_delta_path_driver_state_t **state_p,
+ const svn_delta_editor_t *editor,
+ void *edit_baton,
+ svn_delta_path_driver_cb_func_t callback_func,
+ void *callback_baton,
+ apr_pool_t *pool)
+{
+ svn_delta_path_driver_state_t *state = apr_pcalloc(pool, sizeof(*state));
+
+ state->editor = editor;
+ state->edit_baton = edit_baton;
+ state->callback_func = callback_func;
+ state->callback_baton = callback_baton;
+ state->db_stack = apr_array_make(pool, 4, sizeof(void *));
+ state->last_path = NULL;
+ state->pool = pool;
+
+ *state_p = state;
+ return SVN_NO_ERROR;
+}
+
svn_error_t *
svn_delta_path_driver2(const svn_delta_editor_t *editor,
void *edit_baton,
const apr_array_header_t *paths,
svn_boolean_t sort_paths,
svn_delta_path_driver_cb_func_t callback_func,
void *callback_baton,
apr_pool_t *pool)
{
- apr_array_header_t *db_stack = apr_array_make(pool, 4, sizeof(void *));
- const char *last_path = NULL;
+ svn_delta_path_driver_state_t *state;
int i = 0;
- void *parent_db = NULL, *db = NULL;
+ void *db = NULL;
const char *path;
apr_pool_t *subpool, *iterpool;
- dir_stack_t *item;
/* Do nothing if there are no paths. */
if (! paths->nelts)
return SVN_NO_ERROR;
subpool = svn_pool_create(pool);
@@ -159,141 +190,171 @@ svn_delta_path_driver2(const svn_delta_e
{
apr_array_header_t *sorted = apr_array_copy(subpool, paths);
svn_sort__array(sorted, svn_sort_compare_paths);
paths = sorted;
}
- item = apr_pcalloc(subpool, sizeof(*item));
+ SVN_ERR(svn_delta_path_driver_start(&state,
+ editor, edit_baton,
+ callback_func, callback_baton,
+ pool));
/* If the root of the edit is also a target path, we want to call
the callback function to let the user open the root directory and
do what needs to be done. Otherwise, we'll do the open_root()
ourselves. */
path = APR_ARRAY_IDX(paths, 0, const char *);
if (svn_path_is_empty(path))
{
SVN_ERR(callback_func(&db, NULL, callback_baton, path, subpool));
- last_path = path;
+ state->last_path = path;
i++;
}
else
{
SVN_ERR(editor->open_root(edit_baton, SVN_INVALID_REVNUM, subpool, &db));
}
- item->pool = subpool;
- item->dir_baton = db;
- APR_ARRAY_PUSH(db_stack, void *) = item;
+ {
+ dir_stack_t *item = apr_pcalloc(subpool, sizeof (*item));
+
+ item->pool = subpool;
+ item->dir_baton = db;
+ APR_ARRAY_PUSH(state->db_stack, void *) = item;
+ }
/* Now, loop over the commit items, traversing the URL tree and
driving the editor. */
for (; i < paths->nelts; i++)
{
- const char *pdir;
- const char *common = "";
- size_t common_len;
-
/* Clear the iteration pool. */
svn_pool_clear(iterpool);
/* Get the next path. */
path = APR_ARRAY_IDX(paths, i, const char *);
- /*** Step A - Find the common ancestor of the last path and the
- current one. For the first iteration, this is just the
- empty string. ***/
- if (i > 0)
- common = (last_path[0] == '/')
- ? svn_fspath__get_longest_ancestor(last_path, path, iterpool)
- : svn_relpath_get_longest_ancestor(last_path, path, iterpool);
- common_len = strlen(common);
-
- /*** Step B - Close any directories between the last path and
- the new common ancestor, if any need to be closed.
- Sometimes there is nothing to do here (like, for the first
- iteration, or when the last path was an ancestor of the
- current one). ***/
- if ((i > 0) && (strlen(last_path) > common_len))
- {
- const char *rel = last_path + (common_len ? (common_len + 1) : 0);
- int count = count_components(rel);
- while (count--)
- {
- SVN_ERR(pop_stack(db_stack, editor));
- }
- }
+ SVN_ERR(svn_delta_path_driver_step(state, path, iterpool));
+ }
- /*** Step C - Open any directories between the common ancestor
- and the parent of the current path. ***/
- if (*path == '/')
- pdir = svn_fspath__dirname(path, iterpool);
- else
- pdir = svn_relpath_dirname(path, iterpool);
+ /* Destroy the iteration subpool. */
+ svn_pool_destroy(iterpool);
- if (strlen(pdir) > common_len)
- {
- const char *piece = pdir + common_len + 1;
+ SVN_ERR(svn_delta_path_driver_finish(state, pool));
- while (1)
- {
- const char *rel = pdir;
-
- /* Find the first separator. */
- piece = strchr(piece, '/');
-
- /* Calculate REL as the portion of PDIR up to (but not
- including) the location to which PIECE is pointing. */
- if (piece)
- rel = apr_pstrmemdup(iterpool, pdir, piece - pdir);
-
- /* Open the subdirectory. */
- SVN_ERR(open_dir(db_stack, editor, rel, pool));
-
- /* If we found a '/', advance our PIECE pointer to
- character just after that '/'. Otherwise, we're
- done. */
- if (piece)
- piece++;
- else
- break;
- }
- }
+ return SVN_NO_ERROR;
+}
- /*** Step D - Tell our caller to handle the current path. ***/
- item = APR_ARRAY_IDX(db_stack, db_stack->nelts - 1, void *);
- parent_db = item->dir_baton;
- subpool = svn_pool_create(pool);
- SVN_ERR(callback_func(&db, parent_db, callback_baton, path, subpool));
- if (db)
+svn_error_t *
+svn_delta_path_driver_step(svn_delta_path_driver_state_t *state,
+ const char *path,
+ apr_pool_t *iterpool)
+{
+ const char *pdir;
+ const char *common = "";
+ size_t common_len;
+ apr_pool_t *subpool;
+ dir_stack_t *item;
+ void *parent_db = NULL, *db = NULL;
+
+ /*** Step A - Find the common ancestor of the last path and the
+ current one. For the first iteration, this is just the
+ empty string. ***/
+ if (state->last_path)
+ common = (state->last_path[0] == '/')
+ ? svn_fspath__get_longest_ancestor(state->last_path, path, iterpool)
+ : svn_relpath_get_longest_ancestor(state->last_path, path, iterpool);
+ common_len = strlen(common);
+
+ /*** Step B - Close any directories between the last path and
+ the new common ancestor, if any need to be closed.
+ Sometimes there is nothing to do here (like, for the first
+ iteration, or when the last path was an ancestor of the
+ current one). ***/
+ if ((state->last_path) && (strlen(state->last_path) > common_len))
+ {
+ const char *rel = state->last_path + (common_len ? (common_len + 1) : 0);
+ int count = count_components(rel);
+ while (count--)
{
- item = apr_pcalloc(subpool, sizeof(*item));
- item->dir_baton = db;
- item->pool = subpool;
- APR_ARRAY_PUSH(db_stack, void *) = item;
+ SVN_ERR(pop_stack(state->db_stack, state->editor));
}
- else
+ }
+
+ /*** Step C - Open any directories between the common ancestor
+ and the parent of the current path. ***/
+ if (*path == '/')
+ pdir = svn_fspath__dirname(path, iterpool);
+ else
+ pdir = svn_relpath_dirname(path, iterpool);
+
+ if (strlen(pdir) > common_len)
+ {
+ const char *piece = pdir + common_len + 1;
+
+ while (1)
{
- svn_pool_destroy(subpool);
+ const char *rel = pdir;
+
+ /* Find the first separator. */
+ piece = strchr(piece, '/');
+
+ /* Calculate REL as the portion of PDIR up to (but not
+ including) the location to which PIECE is pointing. */
+ if (piece)
+ rel = apr_pstrmemdup(iterpool, pdir, piece - pdir);
+
+ /* Open the subdirectory. */
+ SVN_ERR(open_dir(state->db_stack, state->editor, rel, state->pool));
+
+ /* If we found a '/', advance our PIECE pointer to
+ character just after that '/'. Otherwise, we're
+ done. */
+ if (piece)
+ piece++;
+ else
+ break;
}
+ }
- /*** Step E - Save our state for the next iteration. If our
- caller opened or added PATH as a directory, that becomes
- our LAST_PATH. Otherwise, we use PATH's parent
- directory. ***/
-
- /* NOTE: The variable LAST_PATH needs to outlive the loop. */
- if (db)
- last_path = path; /* lives in a pool outside our control. */
- else
- last_path = apr_pstrdup(pool, pdir); /* duping into POOL. */
+ /*** Step D - Tell our caller to handle the current path. ***/
+ item = APR_ARRAY_IDX(state->db_stack, state->db_stack->nelts - 1, void *);
+ parent_db = item->dir_baton;
+ subpool = svn_pool_create(state->pool);
+ SVN_ERR(state->callback_func(&db, parent_db, state->callback_baton,
+ path, subpool));
+ if (db)
+ {
+ item = apr_pcalloc(subpool, sizeof (*item));
+ item->dir_baton = db;
+ item->pool = subpool;
+ APR_ARRAY_PUSH(state->db_stack, void *) = item;
+ }
+ else
+ {
+ svn_pool_destroy(subpool);
}
- /* Destroy the iteration subpool. */
- svn_pool_destroy(iterpool);
+ /*** Step E - Save our state for the next iteration. If our
+ caller opened or added PATH as a directory, that becomes
+ our LAST_PATH. Otherwise, we use PATH's parent
+ directory. ***/
+
+ /* NOTE: The variable LAST_PATH needs to outlive the loop. */
+ if (db)
+ state->last_path = path; /* lives in a pool outside our control. */
+ else
+ state->last_path = apr_pstrdup(state->pool, pdir); /* duping into POOL. */
+
+ return SVN_NO_ERROR;
+}
+svn_error_t *
+svn_delta_path_driver_finish(svn_delta_path_driver_state_t *state,
+ apr_pool_t *pool)
+{
/* Close down any remaining open directory batons. */
- while (db_stack->nelts)
+ while (state->db_stack->nelts)
{
- SVN_ERR(pop_stack(db_stack, editor));
+ SVN_ERR(pop_stack(state->db_stack, state->editor));
}
return SVN_NO_ERROR;
}
