Author: rhuijben
Date: Thu Jun 16 13:27:21 2011
New Revision: 1136429

URL: http://svn.apache.org/viewvc?rev=1136429&view=rev
Log:
Provide users of our apis and bindings more information on why a commit has
failed, without falling back to scraping a possibly localized error message for
paths and other details.

* subversion/include/svn_wc.h
  (svn_wc_notify_action_t): Add four notification actions.

* subversion/libsvn_client/commit_util.c
  (fixup_out_of_date_error): Extend arguments to allow providing the absolute
    path to the notification handler and error messages.
  (bail_on_tree_conflicted_children,
   bail_on_tree_conflicted_ancestor): Forward conflict failures to the
    notification handler before returning an error.
  (harvest_committables): Notify nodes before returning an error on them.
    Update caller.
  (validate_dangler): Remove function. Fold into its only caller
    svn_client__harvest_committables.

  (svn_client__harvest_committables): Update caller. Notify errors. Check for
    danglers with a simple hash walk.
  (do_item_commit): Update caller. Use fixup_out_of_date_error in a few more
    places.

Modified:
    subversion/trunk/subversion/include/svn_wc.h
    subversion/trunk/subversion/libsvn_client/commit_util.c

Modified: subversion/trunk/subversion/include/svn_wc.h
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc.h?rev=1136429&r1=1136428&r2=1136429&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_wc.h (original)
+++ subversion/trunk/subversion/include/svn_wc.h Thu Jun 16 13:27:21 2011
@@ -1175,7 +1175,23 @@ typedef enum svn_wc_notify_action_t
 
   /** Removing a path by excluding it.
    * @since New in 1.7. */
-  svn_wc_notify_exclude
+  svn_wc_notify_exclude,
+
+  /** Operation failed because the node remains in conflict
+   * @since New in 1.7. */
+  svn_wc_notify_failed_conflict,
+
+  /** Operation failed because an added node is missing
+   * @since New in 1.7. */
+  svn_wc_notify_failed_missing,
+
+  /** Operation failed because a node is out of date
+   * @since New in 1.7. */
+  svn_wc_notify_failed_out_of_date,
+
+  /** Operation failed because an added parent is not selected
+   * @since New in 1.7. */
+  svn_wc_notify_failed_no_parent
 
 } svn_wc_notify_action_t;
 

Modified: subversion/trunk/subversion/libsvn_client/commit_util.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/commit_util.c?rev=1136429&r1=1136428&r2=1136429&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/commit_util.c (original)
+++ subversion/trunk/subversion/libsvn_client/commit_util.c Thu Jun 16 13:27:21 
2011
@@ -52,17 +52,35 @@
 
 /* Wrap an RA error in an out-of-date error if warranted. */
 static svn_error_t *
-fixup_out_of_date_error(const char *path,
+fixup_out_of_date_error(const char *local_abspath,
+                        const char *path,
                         svn_node_kind_t kind,
-                        svn_error_t *err)
+                        svn_error_t *err,
+                        svn_client_ctx_t *ctx,
+                        apr_pool_t *scratch_pool)
 {
   if (err->apr_err == SVN_ERR_FS_NOT_FOUND
       || err->apr_err == SVN_ERR_RA_DAV_PATH_NOT_FOUND)
-    return  svn_error_createf(SVN_ERR_WC_NOT_UP_TO_DATE, err,
-                              (kind == svn_node_dir
-                               ? _("Directory '%s' is out of date")
-                               : _("File '%s' is out of date")),
-                              path);
+    {
+      if (ctx->notify_func2)
+        {
+          svn_wc_notify_t *notify;
+
+          notify = svn_wc_create_notify(local_abspath ? local_abspath : path,
+                                        svn_wc_notify_failed_out_of_date,
+                                        scratch_pool);
+
+          ctx->notify_func2(ctx->notify_baton2, notify, scratch_pool);
+        }
+
+      return svn_error_createf(SVN_ERR_WC_NOT_UP_TO_DATE, err,
+                               (kind == svn_node_dir
+                                 ? _("Directory '%s' is out of date")
+                                 : _("File '%s' is out of date")),
+                                svn_dirent_local_style(
+                                    local_abspath ? local_abspath : path,
+                                    scratch_pool));
+    }
   else
     return err;
 }
@@ -174,6 +192,8 @@ bail_on_tree_conflicted_children(svn_wc_
                                  svn_node_kind_t kind,
                                  svn_depth_t depth,
                                  apr_hash_t *changelists,
+                                 svn_wc_notify_func2_t notify_func,
+                                 void *notify_baton,
                                  apr_pool_t *pool)
 {
   apr_hash_t *conflicts;
@@ -206,6 +226,16 @@ bail_on_tree_conflicted_children(svn_wc_
 
       /* At this point, a conflict was found, and either there were no
          changelists, or the changelists matched. Bail out already! */
+
+      if (notify_func != NULL)
+        {
+          notify_func(notify_baton,
+                      svn_wc_create_notify(local_abspath,
+                                           svn_wc_notify_failed_conflict,
+                                           pool),
+                      pool);
+        }
+
       return svn_error_createf(
                SVN_ERR_WC_FOUND_CONFLICT, NULL,
                _("Aborting commit: '%s' remains in conflict"),
@@ -221,6 +251,8 @@ bail_on_tree_conflicted_children(svn_wc_
 static svn_error_t *
 bail_on_tree_conflicted_ancestor(svn_wc_context_t *wc_ctx,
                                  const char *local_abspath,
+                                 svn_wc_notify_func2_t notify_func,
+                                 void *notify_baton,
                                  apr_pool_t *scratch_pool)
 {
   const char *wcroot_abspath;
@@ -239,6 +271,15 @@ bail_on_tree_conflicted_ancestor(svn_wc_
                                    wc_ctx, local_abspath, scratch_pool));
       if (tree_conflicted)
         {
+          if (notify_func != NULL)
+            {
+              notify_func(notify_baton,
+                          svn_wc_create_notify(local_abspath,
+                                               svn_wc_notify_failed_conflict,
+                                               scratch_pool),
+                          scratch_pool);
+            }
+
           return svn_error_createf(
                    SVN_ERR_WC_FOUND_CONFLICT, NULL,
                    _("Aborting commit: '%s' remains in tree-conflict"),
@@ -301,6 +342,8 @@ harvest_committables(svn_wc_context_t *w
                      void *check_url_baton,
                      svn_cancel_func_t cancel_func,
                      void *cancel_baton,
+                     svn_wc_notify_func2_t notify_func,
+                     void *notify_baton,
                      apr_pool_t *result_pool,
                      apr_pool_t *scratch_pool)
 {
@@ -428,6 +471,15 @@ harvest_committables(svn_wc_context_t *w
                                    local_abspath, scratch_pool));
       if (tc || pc || treec)
         {
+          if (notify_func != NULL)
+            {
+              notify_func(notify_baton,
+                          svn_wc_create_notify(local_abspath,
+                                               svn_wc_notify_failed_conflict,
+                                               scratch_pool),
+                          scratch_pool);
+            }
+
           return svn_error_createf(
             SVN_ERR_WC_FOUND_CONFLICT, NULL,
             _("Aborting commit: '%s' remains in conflict"),
@@ -538,8 +590,16 @@ harvest_committables(svn_wc_context_t *w
          See issue #3198. */
       if (working_kind == svn_node_none)
         {
-          return svn_error_createf
-            (SVN_ERR_WC_PATH_NOT_FOUND, NULL,
+          if (notify_func != NULL)
+            {
+              notify_func(notify_baton,
+                          svn_wc_create_notify(local_abspath,
+                                               svn_wc_notify_failed_missing,
+                                               scratch_pool),
+                          scratch_pool);
+            }
+          return svn_error_createf(
+             SVN_ERR_WC_PATH_NOT_FOUND, NULL,
              _("'%s' is scheduled for addition, but is missing"),
              svn_dirent_local_style(local_abspath, scratch_pool));
         }
@@ -648,6 +708,7 @@ harvest_committables(svn_wc_context_t *w
 
   SVN_ERR(bail_on_tree_conflicted_children(wc_ctx, local_abspath,
                                            db_kind, depth, changelists,
+                                           notify_func, notify_baton,
                                            scratch_pool));
 
   /* Recursively handle each node according to depth, except when the
@@ -692,6 +753,7 @@ harvest_committables(svn_wc_context_t *w
                                        (depth < svn_depth_immediates),
                                        check_url_func, check_url_baton,
                                        cancel_func, cancel_baton,
+                                       notify_func, notify_baton,
                                        result_pool,
                                        iterpool));
         }
@@ -815,34 +877,6 @@ handle_descendants(void *baton,
   return SVN_NO_ERROR;
 }
 
-
-
-/* BATON is an apr_hash_t * of harvested committables. */
-static svn_error_t *
-validate_dangler(void *baton,
-                 const void *key, apr_ssize_t klen, void *val,
-                 apr_pool_t *pool)
-{
-  const char *dangling_parent = key;
-  const char *dangling_child = val;
-
-  /* The baton points to the committables hash */
-  if (! look_up_committable(baton, dangling_parent, pool))
-    {
-      return svn_error_createf
-        (SVN_ERR_ILLEGAL_TARGET, NULL,
-         _("'%s' is not under version control "
-           "and is not part of the commit, "
-           "yet its child '%s' is part of the commit"),
-         /* Probably one or both of these is an entry, but
-            safest to local_stylize just in case. */
-         svn_dirent_local_style(dangling_parent, pool),
-         svn_dirent_local_style(dangling_child, pool));
-    }
-
-  return SVN_NO_ERROR;
-}
-
 /* Allocate and initialize the COMMITTABLES structure from POOL.
  */
 static void
@@ -874,6 +908,7 @@ svn_client__harvest_committables(svn_cli
   apr_hash_t *changelist_hash = NULL;
   svn_wc_context_t *wc_ctx = ctx->wc_ctx;
   struct handle_descendants_baton hdb;
+  apr_hash_index_t *hi;
 
   /* It's possible that one of the named targets has a parent that is
    * itself scheduled for addition or replacement -- that is, the
@@ -998,6 +1033,8 @@ svn_client__harvest_committables(svn_cli
       /* Make sure this isn't inside a working copy subtree that is
        * marked as tree-conflicted. */
       SVN_ERR(bail_on_tree_conflicted_ancestor(ctx->wc_ctx, target_abspath,
+                                               ctx->notify_func2,
+                                               ctx->notify_baton2,
                                                iterpool));
 
       SVN_ERR(harvest_committables(ctx->wc_ctx, target_abspath,
@@ -1009,6 +1046,7 @@ svn_client__harvest_committables(svn_cli
                                    FALSE, FALSE,
                                    check_url_func, check_url_baton,
                                    ctx->cancel_func, ctx->cancel_baton,
+                                   ctx->notify_func2, ctx->notify_baton2,
                                    result_pool, iterpool));
     }
 
@@ -1022,9 +1060,38 @@ svn_client__harvest_committables(svn_cli
                             handle_descendants, &hdb, iterpool));
 
   /* Make sure that every path in danglers is part of the commit. */
-  SVN_ERR(svn_iter_apr_hash(NULL,
-                            danglers, validate_dangler, *committables,
-                            iterpool));
+  for (hi = apr_hash_first(scratch_pool, danglers); hi; hi = apr_hash_next(hi))
+    {
+      const char *dangling_parent = svn__apr_hash_index_key(hi);
+
+      svn_pool_clear(iterpool);
+
+       if (! look_up_committable(*committables, dangling_parent, iterpool))
+         {
+           const char *dangling_child = svn__apr_hash_index_val(hi);
+
+           if (ctx->notify_func2 != NULL)
+             {
+               svn_wc_notify_t *notify;
+
+               notify = svn_wc_create_notify(dangling_child,
+                                             svn_wc_notify_failed_no_parent,
+                                             scratch_pool);
+
+               ctx->notify_func2(ctx->notify_baton2, notify, iterpool);
+             }
+
+           return svn_error_createf(
+                            SVN_ERR_ILLEGAL_TARGET, NULL,
+                            _("'%s' is not under version control "
+                              "and is not part of the commit, "
+                              "yet its child '%s' is part of the commit"),
+                            /* Probably one or both of these is an entry, but
+                               safest to local_stylize just in case. */
+                            svn_dirent_local_style(dangling_parent, iterpool),
+                            svn_dirent_local_style(dangling_child, iterpool));
+         }
+    }
 
   svn_pool_destroy(iterpool);
 
@@ -1076,6 +1143,8 @@ harvest_copy_committables(void *baton, v
                                btn->check_url_baton,
                                btn->ctx->cancel_func,
                                btn->ctx->cancel_baton,
+                               btn->ctx->notify_func2,
+                               btn->ctx->notify_baton2,
                                btn->result_pool, pool));
 
   hdb.wc_ctx = btn->ctx->wc_ctx;
@@ -1403,8 +1472,9 @@ do_item_commit(void **dir_baton,
                                  parent_baton, pool);
 
       if (err)
-        return svn_error_return(fixup_out_of_date_error(path, item->kind,
-                                                        err));
+        return svn_error_return(fixup_out_of_date_error(local_abspath,
+                                                        path, item->kind,
+                                                        err, ctx, pool));
     }
 
   /* If this item is supposed to be added, do so. */
@@ -1463,8 +1533,10 @@ do_item_commit(void **dir_baton,
                                       file_pool, &file_baton);
 
               if (err)
-                return svn_error_return(fixup_out_of_date_error(path, kind,
-                                                                err));
+                return svn_error_return(fixup_out_of_date_error(local_abspath,
+                                                                path, kind,
+                                                                err, ctx,
+                                                                pool));
             }
         }
       else
@@ -1473,16 +1545,20 @@ do_item_commit(void **dir_baton,
             {
               if (! parent_baton)
                 {
-                  SVN_ERR(editor->open_root
-                          (cb_baton->edit_baton, item->revision,
-                           pool, dir_baton));
+                  err = editor->open_root(cb_baton->edit_baton, item->revision,
+                                          pool, dir_baton);
                 }
               else
                 {
-                  SVN_ERR(editor->open_directory
-                          (path, parent_baton, item->revision,
-                           pool, dir_baton));
+                  err = editor->open_directory(path, parent_baton,
+                                               item->revision,
+                                               pool, dir_baton);
                 }
+
+              if (err)
+                return svn_error_return(fixup_out_of_date_error(
+                                                local_abspath, path, kind,
+                                                err, ctx, pool));
             }
         }
 
@@ -1495,7 +1571,9 @@ do_item_commit(void **dir_baton,
               (kind == svn_node_dir) ? *dir_baton : file_baton, pool);
 
       if (err)
-        return svn_error_return(fixup_out_of_date_error(path, kind, err));
+        return svn_error_return(fixup_out_of_date_error(local_abspath,
+                                                        path, kind, err,
+                                                        ctx, pool));
 
       /* Make any additional client -> repository prop changes. */
       if (item->outgoing_prop_changes)
@@ -1537,8 +1615,9 @@ do_item_commit(void **dir_baton,
                                     file_pool, &file_baton);
 
           if (err)
-            return svn_error_return(fixup_out_of_date_error(path, item->kind,
-                                                            err));
+            return svn_error_return(fixup_out_of_date_error(local_abspath,
+                                                            path, item->kind,
+                                                            err, ctx, pool));
         }
 
       /* Add this file mod to the FILE_MODS hash. */


Reply via email to