On Tue, Mar 9, 2010 at 08:53, <philip_at_apache.org> wrote:
>...
> +++ subversion/trunk/subversion/include/svn_wc.h Tue Mar 9 13:53:38 2010
> @@ -4729,14 +4729,14 @@ svn_wc_committed_queue_create(apr_pool_t
>
> /**
> * Queue committed items to be processed later by
> - * svn_wc_process_committed_queue().
> + * svn_wc_process_committed_queue2().
> *
> - * All pointer data passed to this function (@a path, @a adm_access,
> - * @a wcprop_changes and @a checksum) should remain valid until the queue
> - * has been processed by svn_wc_process_committed_queue().
> + * All pointer data passed to this function (@a path, @a wcprop_changes
> + * and @a checksum) should remain valid until the queue
> + * has been processed by svn_wc_process_committed_queue2().
> *
> * Record in @a queue that @a path will need to be bumped after a commit
> - * succeeds. @a adm_access must hold a write lock appropriate for @a path.
> + * succeeds.
> *
> * If non-NULL, @a wcprop_changes is an array of <tt>svn_prop_t *</tt>
> * changes to wc properties; if an #svn_prop_t->value is NULL, then
> @@ -4763,7 +4763,25 @@ svn_wc_committed_queue_create(apr_pool_t
> * it will bump ALL nodes under the directory, regardless of their
> * actual inclusion in the new revision.
> *
> + * @since New in 1.7.
> + */
> +svn_error_t *
> +svn_wc_queue_committed3(svn_wc_committed_queue_t *queue,
> + const char *path,
> + svn_boolean_t recurse,
> + const apr_array_header_t *wcprop_changes,
> + svn_boolean_t remove_lock,
> + svn_boolean_t remove_changelist,
> + const svn_checksum_t *checksum,
> + apr_pool_t *scratch_pool);
Note: internally, I renamed those booleans to match their command-line
flags. no_unlock and keep_changelist. Those are *inverted* from the
above semantics, but I think they're inherently more descriptive since
they correspond to actual user operations. Since you are revising the
function, we could take this opportunity to rename the flags and
invert their meaning.
My only caution here, would be somebody adding a "3" to their function
call and dropping the access baton. And NOT stopping to invert the
meaning.
Thoughts?
>...
> @@ -4805,11 +4823,23 @@ svn_wc_queue_committed(svn_wc_committed_
> * @a rev_date and @a rev_author are the (server-side) date and author
> * of the new revision; one or both may be @c NULL.
> *
> - * @a adm_access must be associated with all affected directories, and
> - * must hold a write lock in each one.
> + * @since New in 1.7.
> + */
> +svn_error_t *
> +svn_wc_process_committed_queue2(svn_wc_committed_queue_t *queue,
> + svn_wc_context_t *wc_ctx,
> + svn_revnum_t new_revnum,
> + const char *rev_date,
> + const char *rev_author,
> + apr_pool_t *pool);
Please call that @a scratch_pool.
>...
> +++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue Mar 9 13:53:38 2010
>...
> @@ -462,13 +460,17 @@ process_committed_internal(svn_wc__db_t
> apr_pool_t *scratch_pool)
> {
> svn_wc__db_kind_t kind;
> - const char *local_abspath;
> + const char *local_abspath, *adm_abspath;
>
> SVN_ERR(svn_dirent_get_absolute(&local_abspath, path, scratch_pool));
>
> SVN_ERR(svn_wc__db_read_kind(&kind, db, local_abspath, TRUE, scratch_pool));
> if (kind == svn_wc__db_kind_unknown)
> return SVN_NO_ERROR; /* deleted/absent. (?) ... nothing to do. */
> + else if (kind == svn_wc__db_kind_dir)
with a 'return' statement on the above line, there is no need for an 'else'
>...
> @@ -719,17 +734,27 @@ svn_wc_process_committed_queue(svn_wc_co
>
> for (i = 0; i < queue->queue->nelts; i++)
> {
> + const char *local_abspath, *adm_abspath;
> + svn_wc__db_kind_t kind;
> const committed_queue_item_t *cqi
> = APR_ARRAY_IDX(queue->queue, i, const committed_queue_item_t *);
>
> svn_pool_clear(iterpool);
>
> + SVN_ERR(svn_dirent_get_absolute(&local_abspath, cqi->path, iterpool));
> + SVN_ERR(svn_wc__db_read_kind(&kind, wc_ctx->db, local_abspath, TRUE,
> + iterpool));
> + if (kind != svn_wc__db_kind_dir)
> + adm_abspath = svn_dirent_dirname(local_abspath, iterpool);
> + else
> + adm_abspath = local_abspath;
Please compute adm_abspath further down in the loop, right before its
actual usage. This will also *avoid* computing it (often?) because of
the if/continue block.
>...
> @@ -748,6 +773,24 @@ svn_wc_process_committed_queue(svn_wc_co
> }
>
> svn_error_t *
> +svn_wc_process_committed_queue(svn_wc_committed_queue_t *queue,
> + svn_wc_adm_access_t *adm_access,
> + svn_revnum_t new_revnum,
> + const char *rev_date,
> + const char *rev_author,
> + apr_pool_t *pool)
> +{
> + svn_wc_context_t *wc_ctx;
> +
> + SVN_ERR(svn_wc__context_create_with_db(&wc_ctx, NULL,
> + svn_wc__adm_get_db(adm_access),
> + pool));
> + SVN_ERR(svn_wc_process_committed_queue2(queue, wc_ctx, new_revnum,
> + rev_date, rev_author, pool));
> + return SVN_NO_ERROR;
You can explicitly destroy the temporary context before exit. See some
of the other deprecated functions.
>...
Cheers,
-g
Received on 2010-03-09 20:30:02 CET