[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1330058 - in /subversion/trunk/subversion: include/svn_error_codes.h include/svn_fs.h libsvn_fs/editor.c libsvn_repos/commit.c

From: Daniel Shahaf <danielsh_at_elego.de>
Date: Sat, 28 Apr 2012 12:07:30 +0300

Greg Stein wrote on Fri, Apr 27, 2012 at 17:33:18 -0400:
> I'm growing weary of this.
>
> Daniel and I spent a couple hours discussing calling patterns, error
> states, and everything. This felt the best approach.
>
> If you have a concrete suggestion, and a patch, then I'll take a look.
>

Index: subversion/include/svn_fs.h
===================================================================
--- subversion/include/svn_fs.h (revision 1331716)
+++ subversion/include/svn_fs.h (working copy)
@@ -1090,19 +1090,19 @@ svn_fs_editor_create_for(svn_editor_t **editor,
  * reason, then @a *revision will be set to #SVN_INVALID_REVNUM.
  *
  * If a conflict occurs during the commit, then @a *conflict_path will
- * be set to a path that caused the conflict. #SVN_NO_ERROR will be returned.
- * Callers may want to construct an #SVN_ERR_FS_CONFLICT error with a
- * message that incorporates @a *conflict_path.
+ * be set to a path that caused the conflict, and @a *copc_err will be
+ * be set to an error chain describing that error in a human-readable manner.
+ * #SVN_NO_ERROR will be returned.
  *
  * If a non-conflict error occurs during the commit, then that error will
  * be returned.
- * As is standard with any Subversion API, @a revision, @a post_commit_err,
+ * As is standard with any Subversion API, @a revision, @a copc_err,
  * and @a conflict_path (the OUT parameters) have an indeterminate value if
  * an error is returned.
  *
  * If the commit completes (and a revision is returned in @a *revision), then
  * it is still possible for an error to occur during the cleanup process.
- * Any such error will be returned in @a *post_commit_err. The caller must
+ * Any such error will be returned in @a *copc_err. The caller must
  * properly use or clear that error.
  *
  * If svn_editor_complete() has already been called on @a editor, then
@@ -1117,15 +1117,16 @@ svn_fs_editor_create_for(svn_editor_t **editor,
  * @a scratch_pool will be used for all temporary allocations.
  *
  * @note To summarize, there are three possible outcomes of this function:
- * successful commit (with or without an associated @a *post_commit_err);
- * failed commit due to a conflict (reported via @a *conflict_path); and
- * failed commit for some other reason (reported via the returned error.)
+ * successful commit (with or without an associated @a *copc_err);
+ * failed commit due to a conflict (reported via both @a *conflict_path
+ * and @a *copc_err); and failed commit for some other reason (reported via
+ * the returned error, with undefined values for the OUT parameters.)
  *
  * @since New in 1.8.
  */
 svn_error_t *
 svn_fs_editor_commit(svn_revnum_t *revision,
- svn_error_t **post_commit_err,
+ svn_error_t **copc_err, /* conflict or post-commit */
                      const char **conflict_path,
                      svn_editor_t *editor,
                      apr_pool_t *result_pool,
Index: subversion/libsvn_fs/editor.c
===================================================================
--- subversion/libsvn_fs/editor.c (revision 1331716)
+++ subversion/libsvn_fs/editor.c (working copy)
@@ -450,7 +450,7 @@ svn_fs_editor_create_for(svn_editor_t **editor,
 
 svn_error_t *
 svn_fs_editor_commit(svn_revnum_t *revision,
- svn_error_t **post_commit_err,
+ svn_error_t **copc_err,
                      const char **conflict_path,
                      svn_editor_t *editor,
                      apr_pool_t *result_pool,
@@ -466,7 +466,7 @@ svn_fs_editor_commit(svn_revnum_t *revision,
                             NULL, NULL);
 
   *revision = SVN_INVALID_REVNUM;
- *post_commit_err = NULL;
+ *copc_err = NULL;
   *conflict_path = NULL;
 
   /* Clean up internal resources (eg. eb->root). This also allows the
@@ -489,7 +489,7 @@ svn_fs_editor_commit(svn_revnum_t *revision,
           /* Case 3. ERR is a post-commit (cleanup) error. */
 
           /* Pass responsibility via POST_COMMIT_ERR. */
- *post_commit_err = err;
+ *copc_err = err;
           err = SVN_NO_ERROR;
         }
       /* else: Case 1. */
@@ -504,9 +504,8 @@ svn_fs_editor_commit(svn_revnum_t *revision,
           /* Copy this into the correct pool (see note above). */
           *conflict_path = apr_pstrdup(result_pool, inner_conflict_path);
 
- /* Return sucess. The caller should inspect CONFLICT_PATH to
- determine this particular case. */
- svn_error_clear(err);
+ /* Pass responsibility via POST_COMMIT_ERR. */
+ *copc_err = err;
           err = SVN_NO_ERROR;
         }
       /* else: Case 4. */
Index: subversion/libsvn_repos/commit.c
===================================================================
--- subversion/libsvn_repos/commit.c (revision 1331716)
+++ subversion/libsvn_repos/commit.c (working copy)
@@ -1214,6 +1214,7 @@ complete_cb(void *baton,
 {
   struct ev2_baton *eb = baton;
   svn_revnum_t revision;
+ svn_error_t *copc_err;
   svn_error_t *post_commit_err;
   const char *conflict_path;
   svn_error_t *err;
@@ -1224,18 +1225,19 @@ complete_cb(void *baton,
   SVN_ERR(svn_repos__hooks_pre_commit(eb->repos, eb->txn_name, scratch_pool));
 
   /* Hook is done. Let's do the actual commit. */
- SVN_ERR(svn_fs_editor_commit(&revision, &post_commit_err, &conflict_path,
+ SVN_ERR(svn_fs_editor_commit(&revision, &copc_err, &conflict_path,
                                eb->inner, scratch_pool, scratch_pool));
 
   /* Did a conflict occur during the commit process? */
   if (conflict_path != NULL)
- return svn_error_createf(SVN_ERR_FS_CONFLICT, NULL,
+ return svn_error_createf(SVN_ERR_FS_CONFLICT, copc_err,,
                              _("Conflict at '%s'"), conflict_path);
 
   /* Since did not receive an error during the commit process, and no
      conflict was specified... we committed a revision. Run the hooks.
      Other errors may have occurred within the FS (specified by the
      POST_COMMIT_ERR localvar), but we need to run the hooks. */
+ post_commit_err = copc_err;
   SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(revision));
   err = svn_repos__hooks_post_commit(eb->repos, revision, eb->txn_name,
                                      scratch_pool);
Received on 2012-04-28 11:08:11 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.