You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ph...@apache.org on 2010/03/09 14:53:39 UTC

svn commit: r920875 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/commit.c libsvn_wc/adm_ops.c

Author: philip
Date: Tue Mar  9 13:53:38 2010
New Revision: 920875

URL: http://svn.apache.org/viewvc?rev=920875&view=rev
Log:
Remove some access batons from post-commit processing.

* subversion/include/svn_wc.h
  (svn_wc_queue_committed3, svn_wc_process_committed_queue2): New.
  (svn_wc_queue_committed2, svn_wc_process_committed_queue): Deprecate.

* subversion/libsvn_wc/adm_ops.c
  (struct committed_queue_item_t): Remove adm_abspath.
  (process_committed_internal): Remove adm_abspath parameter, derive
   abspath from path.
  (svn_wc_queue_committed3): Renamed from svn_wc_process_committed_queue2
   with access baton parameter removed.
  (svn_wc_queue_committed2): Call svn_wc_queue_committed3.
  (svn_wc_process_committed_queue2): Renamed svn_wc_process_committed_queue
   with access baton parameter changed to wc context.
  (svn_wc_process_committed_queue): Call svn_wc_process_committed_queue2.

* subversion/libsvn_client/commit.c
  (svn_client_commit4): Call svn_wc_queue_committed3 and
   svn_wc_process_committed_queue2.

Modified:
    subversion/trunk/subversion/include/svn_wc.h
    subversion/trunk/subversion/libsvn_client/commit.c
    subversion/trunk/subversion/libsvn_wc/adm_ops.c

Modified: subversion/trunk/subversion/include/svn_wc.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc.h?rev=920875&r1=920874&r2=920875&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_wc.h (original)
+++ 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);
+
+/** @see svn_wc_queue_committed3()
+ *
+ * @a adm_access is unused.
+ *
  * @since New in 1.6.
+ *
+ * @deprecated Provided for backwards compatibility with the 1.6 API.
  */
 svn_error_t *
 svn_wc_queue_committed2(svn_wc_committed_queue_t *queue,
@@ -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);
+
+/** @see svn_wc_process_committed_queue2()
  *
  * @since New in 1.5.
+ *
+ * @deprecated Provided for backwards compatibility with the 1.5 API.
  */
+SVN_DEPRECATED
 svn_error_t *
 svn_wc_process_committed_queue(svn_wc_committed_queue_t *queue,
                                svn_wc_adm_access_t *adm_access,

Modified: subversion/trunk/subversion/libsvn_client/commit.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/commit.c?rev=920875&r1=920874&r2=920875&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/commit.c (original)
+++ subversion/trunk/subversion/libsvn_client/commit.c Tue Mar  9 13:53:38 2010
@@ -990,7 +990,7 @@ post_process_commit_item(void *baton, vo
 
   /* Allocate the queue in a longer-lived pool than (iter)pool:
      we want it to survive the next iteration. */
-  return svn_wc_queue_committed2(btn->queue, item->path, adm_access,
+  return svn_wc_queue_committed3(btn->queue, item->path,
                                  loop_recurse, item->incoming_prop_changes,
                                  remove_lock, !btn->keep_changelists,
                                  apr_hash_get(btn->checksums,
@@ -1298,7 +1298,7 @@ svn_client_commit4(svn_commit_info_t **c
 
       SVN_ERR_ASSERT(*commit_info_p);
       bump_err
-        = svn_wc_process_committed_queue(queue, base_dir_access,
+        = svn_wc_process_committed_queue2(queue, ctx->wc_ctx,
                                          (*commit_info_p)->revision,
                                          (*commit_info_p)->date,
                                          (*commit_info_p)->author,

Modified: subversion/trunk/subversion/libsvn_wc/adm_ops.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/adm_ops.c?rev=920875&r1=920874&r2=920875&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/adm_ops.c (original)
+++ subversion/trunk/subversion/libsvn_wc/adm_ops.c Tue Mar  9 13:53:38 2010
@@ -76,7 +76,6 @@ struct svn_wc_committed_queue_t
 typedef struct
 {
   const char *path;
-  const char *adm_abspath;
   svn_boolean_t recurse;
   svn_boolean_t no_unlock;
   svn_boolean_t keep_changelist;
@@ -448,7 +447,6 @@ process_committed_leaf(svn_wc__db_t *db,
 
 static svn_error_t *
 process_committed_internal(svn_wc__db_t *db,
-                           const char *adm_abspath,
                            const char *path,
                            svn_boolean_t recurse,
                            svn_revnum_t new_revnum,
@@ -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)
+    adm_abspath = local_abspath;
+  else
+    adm_abspath = svn_dirent_dirname(local_abspath, scratch_pool);
 
   SVN_ERR(process_committed_leaf(db, adm_abspath, path,
                                  new_revnum, new_date, rev_author,
@@ -530,7 +532,7 @@ process_committed_internal(svn_wc__db_t 
              this one committed item. */
           if (kind == svn_wc__db_kind_dir)
             {
-              SVN_ERR(process_committed_internal(db, this_abspath, this_path,
+              SVN_ERR(process_committed_internal(db, this_path,
                                                  TRUE /* recurse */,
                                                  new_revnum, new_date,
                                                  rev_author,
@@ -612,9 +614,8 @@ svn_wc_committed_queue_create(apr_pool_t
 }
 
 svn_error_t *
-svn_wc_queue_committed2(svn_wc_committed_queue_t *queue,
+svn_wc_queue_committed3(svn_wc_committed_queue_t *queue,
                         const char *path,
-                        svn_wc_adm_access_t *adm_access,
                         svn_boolean_t recurse,
                         const apr_array_header_t *wcprop_changes,
                         svn_boolean_t remove_lock,
@@ -633,7 +634,6 @@ svn_wc_queue_committed2(svn_wc_committed
   /* Add to the array with paths and options */
   cqi = apr_palloc(queue->pool, sizeof(*cqi));
   cqi->path = path;
-  cqi->adm_abspath = svn_wc__adm_access_abspath(adm_access);
   cqi->recurse = recurse;
   cqi->no_unlock = !remove_lock;
   cqi->keep_changelist = !remove_changelist;
@@ -645,6 +645,22 @@ svn_wc_queue_committed2(svn_wc_committed
   return SVN_NO_ERROR;
 }
 
+svn_error_t *
+svn_wc_queue_committed2(svn_wc_committed_queue_t *queue,
+                        const char *path,
+                        svn_wc_adm_access_t *adm_access,
+                        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)
+{
+  return svn_wc_queue_committed3(queue, path, recurse, wcprop_changes,
+                                 remove_lock, remove_changelist, checksum,
+                                 scratch_pool);
+}
+
 
 /* NOTE: this function doesn't move to deprecated.c because of its need
    for the internals of svn_wc_committed_queue_t.  */
@@ -700,14 +716,13 @@ have_recursive_parent(apr_array_header_t
 }
 
 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_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)
 {
-  svn_wc__db_t *db = svn_wc__adm_get_db(adm_access);
   int i;
   apr_pool_t *iterpool = svn_pool_create(pool);
   apr_time_t new_date;
@@ -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;
+
       /* If there are some recursive items, then see if this item is a
          child of one, and will (implicitly) be accounted for. */
       if (queue->have_recursive && have_recursive_parent(queue->queue, i))
         continue;
 
-      SVN_ERR(process_committed_internal(db, cqi->adm_abspath, cqi->path,
+      SVN_ERR(process_committed_internal(wc_ctx->db, cqi->path,
                                          cqi->recurse,
                                          new_revnum, new_date, rev_author,
                                          cqi->new_dav_cache,
@@ -737,7 +762,7 @@ svn_wc_process_committed_queue(svn_wc_co
                                          cqi->keep_changelist,
                                          cqi->checksum, queue, iterpool));
 
-      SVN_ERR(svn_wc__wq_run(db, cqi->adm_abspath, NULL, NULL, iterpool));
+      SVN_ERR(svn_wc__wq_run(wc_ctx->db, adm_abspath, NULL, NULL, iterpool));
     }
 
   queue->queue->nelts = 0;
@@ -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;
+}
+
+svn_error_t *
 svn_wc_process_committed4(const char *path,
                           svn_wc_adm_access_t *adm_access,
                           svn_boolean_t recurse,
@@ -775,7 +818,7 @@ svn_wc_process_committed4(const char *pa
   else
     checksum = NULL;
 
-  SVN_ERR(process_committed_internal(db, adm_abspath,
+  SVN_ERR(process_committed_internal(db,
                                      path, recurse,
                                      new_revnum, new_date, rev_author,
                                      convert_to_hash(wcprop_changes, pool),



Re: svn commit: r920875 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/commit.c libsvn_wc/adm_ops.c

Posted by Philip Martin <ph...@wandisco.com>.
Julian Foad <ju...@wandisco.com> writes:

> You could add "SVN_DEPRECATED" to svn_wc_queue_committed2(). (You added
> it to svn_wc_process_committed_queue().)
>
> Hyrum suggests the deprecated functions should be moved to
> 'deprecated.c'.

Will do.

-- 
Philip

Re: svn commit: r920875 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/commit.c libsvn_wc/adm_ops.c

Posted by Julian Foad <ju...@wandisco.com>.
On Tue, 2010-03-09, philip@apache.org wrote:
> Author: philip
> Date: Tue Mar  9 13:53:38 2010
> New Revision: 920875
> 
> URL: http://svn.apache.org/viewvc?rev=920875&view=rev
> Log:
> Remove some access batons from post-commit processing.

Hi Philip.

You could add "SVN_DEPRECATED" to svn_wc_queue_committed2(). (You added
it to svn_wc_process_committed_queue().)

Hyrum suggests the deprecated functions should be moved to
'deprecated.c'.

- Julian


> * subversion/include/svn_wc.h
>   (svn_wc_queue_committed3, svn_wc_process_committed_queue2): New.
>   (svn_wc_queue_committed2, svn_wc_process_committed_queue): Deprecate.
> 
> * subversion/libsvn_wc/adm_ops.c
>   (struct committed_queue_item_t): Remove adm_abspath.
>   (process_committed_internal): Remove adm_abspath parameter, derive
>    abspath from path.
>   (svn_wc_queue_committed3): Renamed from svn_wc_process_committed_queue2
>    with access baton parameter removed.
>   (svn_wc_queue_committed2): Call svn_wc_queue_committed3.
>   (svn_wc_process_committed_queue2): Renamed svn_wc_process_committed_queue
>    with access baton parameter changed to wc context.
>   (svn_wc_process_committed_queue): Call svn_wc_process_committed_queue2.
> 
> * subversion/libsvn_client/commit.c
>   (svn_client_commit4): Call svn_wc_queue_committed3 and
>    svn_wc_process_committed_queue2.
> 
> Modified:
>     subversion/trunk/subversion/include/svn_wc.h
>     subversion/trunk/subversion/libsvn_client/commit.c
>     subversion/trunk/subversion/libsvn_wc/adm_ops.c
> 
> Modified: subversion/trunk/subversion/include/svn_wc.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc.h?rev=920875&r1=920874&r2=920875&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_wc.h (original)
> +++ 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);
> +
> +/** @see svn_wc_queue_committed3()
> + *
> + * @a adm_access is unused.
> + *
>   * @since New in 1.6.
> + *
> + * @deprecated Provided for backwards compatibility with the 1.6 API.
>   */
>  svn_error_t *
>  svn_wc_queue_committed2(svn_wc_committed_queue_t *queue,
> @@ -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);
> +
> +/** @see svn_wc_process_committed_queue2()
>   *
>   * @since New in 1.5.
> + *
> + * @deprecated Provided for backwards compatibility with the 1.5 API.
>   */
> +SVN_DEPRECATED
>  svn_error_t *
>  svn_wc_process_committed_queue(svn_wc_committed_queue_t *queue,
>                                 svn_wc_adm_access_t *adm_access,
> 
[...]



Re: svn commit: r920875 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/commit.c libsvn_wc/adm_ops.c

Posted by Julian Foad <ju...@wandisco.com>.
On Tue, 2010-03-09, philip@apache.org wrote:
> Author: philip
> Date: Tue Mar  9 13:53:38 2010
> New Revision: 920875
> 
> URL: http://svn.apache.org/viewvc?rev=920875&view=rev
> Log:
> Remove some access batons from post-commit processing.

Hi Philip.

You could add "SVN_DEPRECATED" to svn_wc_queue_committed2(). (You added
it to svn_wc_process_committed_queue().)

Hyrum suggests the deprecated functions should be moved to
'deprecated.c'.

- Julian


> * subversion/include/svn_wc.h
>   (svn_wc_queue_committed3, svn_wc_process_committed_queue2): New.
>   (svn_wc_queue_committed2, svn_wc_process_committed_queue): Deprecate.
> 
> * subversion/libsvn_wc/adm_ops.c
>   (struct committed_queue_item_t): Remove adm_abspath.
>   (process_committed_internal): Remove adm_abspath parameter, derive
>    abspath from path.
>   (svn_wc_queue_committed3): Renamed from svn_wc_process_committed_queue2
>    with access baton parameter removed.
>   (svn_wc_queue_committed2): Call svn_wc_queue_committed3.
>   (svn_wc_process_committed_queue2): Renamed svn_wc_process_committed_queue
>    with access baton parameter changed to wc context.
>   (svn_wc_process_committed_queue): Call svn_wc_process_committed_queue2.
> 
> * subversion/libsvn_client/commit.c
>   (svn_client_commit4): Call svn_wc_queue_committed3 and
>    svn_wc_process_committed_queue2.
> 
> Modified:
>     subversion/trunk/subversion/include/svn_wc.h
>     subversion/trunk/subversion/libsvn_client/commit.c
>     subversion/trunk/subversion/libsvn_wc/adm_ops.c
> 
> Modified: subversion/trunk/subversion/include/svn_wc.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc.h?rev=920875&r1=920874&r2=920875&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_wc.h (original)
> +++ 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);
> +
> +/** @see svn_wc_queue_committed3()
> + *
> + * @a adm_access is unused.
> + *
>   * @since New in 1.6.
> + *
> + * @deprecated Provided for backwards compatibility with the 1.6 API.
>   */
>  svn_error_t *
>  svn_wc_queue_committed2(svn_wc_committed_queue_t *queue,
> @@ -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);
> +
> +/** @see svn_wc_process_committed_queue2()
>   *
>   * @since New in 1.5.
> + *
> + * @deprecated Provided for backwards compatibility with the 1.5 API.
>   */
> +SVN_DEPRECATED
>  svn_error_t *
>  svn_wc_process_committed_queue(svn_wc_committed_queue_t *queue,
>                                 svn_wc_adm_access_t *adm_access,
> 
[...]


Re: svn commit: r920875 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/commit.c libsvn_wc/adm_ops.c

Posted by Neels J Hofmeyr <ne...@elego.de>.
Greg Stein wrote:
> On Tue, Mar 9, 2010 at 08:53,  <ph...@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?

We could also change the argument order to get compile time errors.

~Neels


Re: svn commit: r920875 - in /subversion/trunk/subversion: include/svn_wc.h libsvn_client/commit.c libsvn_wc/adm_ops.c

Posted by Greg Stein <gs...@gmail.com>.
On Tue, Mar 9, 2010 at 08:53,  <ph...@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