Author: stsp
Date: Tue Oct 18 16:37:59 2011
New Revision: 1185746

URL: http://svn.apache.org/viewvc?rev=1185746&view=rev
Log:
Fix up some erroneous "Could not frob some targets because..." warnings.

E.g. 'svn add existing-versioned-file' would report "Could not add some
targets because some targets don't exist" -- which is obviously bogus.

The warnings reported are now more precise, and can contain multiple
reasons. E.g. 'svn cat' reports both nonexistent targets and targets
which are directories in its "Could not cat all targets because ..."
messages.

* subversion/tests/cmdline/cat_tests.py
  (cat_skip_uncattable): Adjust expected warnings.

* subversion/svn/cl.h, subversion/svn/util.c
  (svn_cl__try): Instead of returning a single boolean SUCCESS, return
    the list ERRORS_SEEN, which contains all ignored error codes which
    appear in the varargs list and were returned as part of the ERR chain.

* subversion/svn/add-cmd.c
  (svn_cl__cat): Check all errors reported back from svn_cl__try(),
    and print a proper warning message for each.

* subversion/svn/cat-cmd.c
  (svn_cl__cat): As previous.

* subversion/svn/proplist-cmd.c
  (svn_cl__proplist): As previous, and also fix up warnings to say
   "display properties" instead of "display info" (probably a copy-paste
   from info-cmd.c when these messages were originally added).

* subversion/svn/changelist-cmd.c
  (svn_cl__changelist): As previous, but fix up warnings to say "display
   changelist" instead of "display info".

Modified:
    subversion/trunk/subversion/svn/add-cmd.c
    subversion/trunk/subversion/svn/cat-cmd.c
    subversion/trunk/subversion/svn/changelist-cmd.c
    subversion/trunk/subversion/svn/cl.h
    subversion/trunk/subversion/svn/proplist-cmd.c
    subversion/trunk/subversion/svn/util.c
    subversion/trunk/subversion/tests/cmdline/cat_tests.py

Modified: subversion/trunk/subversion/svn/add-cmd.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/add-cmd.c?rev=1185746&r1=1185745&r2=1185746&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/add-cmd.c (original)
+++ subversion/trunk/subversion/svn/add-cmd.c Tue Oct 18 16:37:59 2011
@@ -51,7 +51,7 @@ svn_cl__add(apr_getopt_t *os,
   apr_array_header_t *targets;
   int i;
   apr_pool_t *iterpool;
-  svn_boolean_t seen_nonexistent_target = FALSE;
+  apr_array_header_t *errors = apr_array_make(pool, 0, sizeof(apr_status_t));
 
   SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
                                                       opt_state->targets,
@@ -71,7 +71,6 @@ svn_cl__add(apr_getopt_t *os,
   for (i = 0; i < targets->nelts; i++)
     {
       const char *target = APR_ARRAY_IDX(targets, i, const char *);
-      svn_boolean_t success;
 
       svn_pool_clear(iterpool);
       SVN_ERR(svn_cl__check_cancel(ctx->cancel_baton));
@@ -80,21 +79,34 @@ svn_cl__add(apr_getopt_t *os,
                                opt_state->depth,
                                opt_state->force, opt_state->no_ignore,
                                opt_state->parents, ctx, iterpool),
-               &success, opt_state->quiet,
+               errors, opt_state->quiet,
                SVN_ERR_ENTRY_EXISTS,
                SVN_ERR_WC_PATH_NOT_FOUND,
                SVN_NO_ERROR));
-
-      if (! success)
-        seen_nonexistent_target = TRUE;
     }
 
   svn_pool_destroy(iterpool);
 
-  if (seen_nonexistent_target)
-    return svn_error_create(
-      SVN_ERR_ILLEGAL_TARGET, NULL,
-      _("Could not add all targets because some targets don't exist"));
-  else
-    return SVN_NO_ERROR;
+  if (errors->nelts > 0)
+    {
+      svn_error_t *err;
+
+      err = svn_error_create(SVN_ERR_ILLEGAL_TARGET, NULL, NULL);
+      for (i = 0; i < errors->nelts; i++)
+        {
+          apr_status_t status = APR_ARRAY_IDX(errors, i, apr_status_t);
+          if (status == SVN_ERR_ENTRY_NOT_FOUND)
+            err = svn_error_quick_wrap(err,
+                                       _("Could not add all targets because "
+                                         "some targets don't exist"));
+          else if (status == SVN_ERR_UNVERSIONED_RESOURCE)
+            err = svn_error_quick_wrap(err,
+                                       _("Could not add all targets because "
+                                         "some targets are already 
versioned"));
+        }
+
+      return svn_error_trace(err);
+    }
+
+  return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/svn/cat-cmd.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/cat-cmd.c?rev=1185746&r1=1185745&r2=1185746&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/cat-cmd.c (original)
+++ subversion/trunk/subversion/svn/cat-cmd.c Tue Oct 18 16:37:59 2011
@@ -50,7 +50,8 @@ svn_cl__cat(apr_getopt_t *os,
   int i;
   svn_stream_t *out;
   apr_pool_t *subpool = svn_pool_create(pool);
-  svn_boolean_t seen_nonexistent_target = FALSE;
+  apr_array_header_t *errors = apr_array_make(pool, 0, sizeof(apr_status_t));
+  svn_error_t *err;
 
   SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
                                                       opt_state->targets,
@@ -67,7 +68,6 @@ svn_cl__cat(apr_getopt_t *os,
       const char *target = APR_ARRAY_IDX(targets, i, const char *);
       const char *truepath;
       svn_opt_revision_t peg_revision;
-      svn_boolean_t success;
 
       svn_pool_clear(subpool);
       SVN_ERR(svn_cl__check_cancel(ctx->cancel_baton));
@@ -79,21 +79,40 @@ svn_cl__cat(apr_getopt_t *os,
       SVN_ERR(svn_cl__try(svn_client_cat2(out, truepath, &peg_revision,
                                           &(opt_state->start_revision),
                                           ctx, subpool),
-                           &success, opt_state->quiet,
+                           errors, opt_state->quiet,
                            SVN_ERR_UNVERSIONED_RESOURCE,
                            SVN_ERR_ENTRY_NOT_FOUND,
                            SVN_ERR_CLIENT_IS_DIRECTORY,
                            SVN_ERR_FS_NOT_FOUND,
                            SVN_NO_ERROR));
-      if (! success)
-        seen_nonexistent_target = TRUE;
     }
   svn_pool_destroy(subpool);
 
-  if (seen_nonexistent_target)
-    return svn_error_create(
-      SVN_ERR_ILLEGAL_TARGET, NULL,
-      _("Could not cat all targets because some targets don't exist"));
-  else
-    return SVN_NO_ERROR;
+  if (errors->nelts > 0)
+    {
+      err = svn_error_create(SVN_ERR_ILLEGAL_TARGET, NULL, NULL);
+
+      for (i = 0; i < errors->nelts; i++)
+        {
+          apr_status_t status = APR_ARRAY_IDX(errors, i, apr_status_t);
+
+          if (status == SVN_ERR_ENTRY_NOT_FOUND ||
+              status == SVN_ERR_FS_NOT_FOUND)
+            err = svn_error_quick_wrap(err, 
+                                       _("Could not cat all targets because "
+                                         "some targets don't exist"));
+          else if (status == SVN_ERR_UNVERSIONED_RESOURCE)
+            err = svn_error_quick_wrap(err,
+                                       _("Could not cat all targets because "
+                                         "some targets are not versioned"));
+          else if (status == SVN_ERR_CLIENT_IS_DIRECTORY)
+            err = svn_error_quick_wrap(err,
+                                       _("Could not cat all targets because "
+                                         "some targets are directories"));
+        }
+
+      return svn_error_trace(err);
+    }
+  
+  return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/svn/changelist-cmd.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/changelist-cmd.c?rev=1185746&r1=1185745&r2=1185746&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/changelist-cmd.c (original)
+++ subversion/trunk/subversion/svn/changelist-cmd.c Tue Oct 18 16:37:59 2011
@@ -45,7 +45,7 @@ svn_cl__changelist(apr_getopt_t *os,
   svn_client_ctx_t *ctx = ((svn_cl__cmd_baton_t *) baton)->ctx;
   apr_array_header_t *targets;
   svn_depth_t depth = opt_state->depth;
-  svn_boolean_t success = TRUE;
+  apr_array_header_t *errors = apr_array_make(pool, 0, sizeof(apr_status_t));
 
   /* If we're not removing changelists, then our first argument should
      be the name of a changelist. */
@@ -103,7 +103,7 @@ svn_cl__changelist(apr_getopt_t *os,
                svn_client_add_to_changelist(targets, changelist_name,
                                             depth, opt_state->changelists,
                                             ctx, pool),
-               &success, opt_state->quiet,
+               errors, opt_state->quiet,
                SVN_ERR_UNVERSIONED_RESOURCE,
                SVN_ERR_WC_PATH_NOT_FOUND,
                SVN_NO_ERROR));
@@ -114,16 +114,36 @@ svn_cl__changelist(apr_getopt_t *os,
                svn_client_remove_from_changelists(targets, depth,
                                                   opt_state->changelists,
                                                   ctx, pool),
-               &success, opt_state->quiet,
+               errors, opt_state->quiet,
                SVN_ERR_UNVERSIONED_RESOURCE,
                SVN_ERR_WC_PATH_NOT_FOUND,
                SVN_NO_ERROR));
     }
 
-  if (!success)
-    return svn_error_create(SVN_ERR_ILLEGAL_TARGET, NULL,
-                            _("Could not display info for all targets because "
-                              "some targets don't exist"));
-  else
-    return SVN_NO_ERROR;
+  if (errors->nelts > 0)
+    {
+      int i;
+      svn_error_t *err;
+
+      err = svn_error_create(SVN_ERR_ILLEGAL_TARGET, NULL, NULL);
+      for (i = 0; i < errors->nelts; i++)
+        {
+          apr_status_t status = APR_ARRAY_IDX(errors, i, apr_status_t);
+          
+          if (status == SVN_ERR_WC_PATH_NOT_FOUND)
+            err = svn_error_quick_wrap(err,
+                                       _("Could not display changelists of "
+                                         "all targets because some targets "
+                                         "don't exist"));
+          else if (status == SVN_ERR_UNVERSIONED_RESOURCE)
+            err = svn_error_quick_wrap(err,
+                                       _("Could not display changelists of "
+                                         "all targets because some targets "
+                                         "are not versioned"));
+        }
+
+      return svn_error_trace(err);
+    }
+
+  return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/svn/cl.h
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/cl.h?rev=1185746&r1=1185745&r2=1185746&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/cl.h (original)
+++ subversion/trunk/subversion/svn/cl.h Tue Oct 18 16:37:59 2011
@@ -294,15 +294,14 @@ extern const apr_getopt_option_t svn_cl_
  * invoked on an unversioned, nonexistent, or otherwise innocuously
  * errorful resource.  Meant to be wrapped with SVN_ERR().
  *
- * If ERR is null, return SVN_NO_ERROR, setting *SUCCESS to TRUE
- * if SUCCESS is not NULL.
+ * If ERR is null, return SVN_NO_ERROR.
  *
  * Else if ERR->apr_err is one of the error codes supplied in varargs,
  * then handle ERR as a warning (unless QUIET is true), clear ERR, and
- * return SVN_NO_ERROR, setting *SUCCESS to FALSE if SUCCESS is not
- * NULL.
+ * return SVN_NO_ERROR, and push the value of ERR->apr_err into the
+ * ERRORS_SEEN array, if ERRORS_SEEN is not NULL.
  *
- * Else return ERR, setting *SUCCESS to FALSE if SUCCESS is not NULL.
+ * Else return ERR.
  *
  * Typically, error codes like SVN_ERR_UNVERSIONED_RESOURCE,
  * SVN_ERR_ENTRY_NOT_FOUND, etc, are supplied in varargs.  Don't
@@ -310,7 +309,7 @@ extern const apr_getopt_option_t svn_cl_
  */
 svn_error_t *
 svn_cl__try(svn_error_t *err,
-            svn_boolean_t *success,
+            apr_array_header_t *errors_seen,
             svn_boolean_t quiet,
             ...);
 

Modified: subversion/trunk/subversion/svn/proplist-cmd.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/proplist-cmd.c?rev=1185746&r1=1185745&r2=1185746&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/proplist-cmd.c (original)
+++ subversion/trunk/subversion/svn/proplist-cmd.c Tue Oct 18 16:37:59 2011
@@ -112,6 +112,8 @@ svn_cl__proplist(apr_getopt_t *os,
   svn_cl__opt_state_t *opt_state = ((svn_cl__cmd_baton_t *) baton)->opt_state;
   svn_client_ctx_t *ctx = ((svn_cl__cmd_baton_t *) baton)->ctx;
   apr_array_header_t *targets;
+  apr_array_header_t *errors = apr_array_make(scratch_pool, 0,
+                                              sizeof(apr_status_t));
 
   SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
                                                       opt_state->targets,
@@ -168,7 +170,6 @@ svn_cl__proplist(apr_getopt_t *os,
       int i;
       apr_pool_t *iterpool;
       svn_proplist_receiver_t pl_receiver;
-      svn_boolean_t had_errors = FALSE;
 
       if (opt_state->xml)
         {
@@ -190,7 +191,6 @@ svn_cl__proplist(apr_getopt_t *os,
           proplist_baton_t pl_baton;
           const char *truepath;
           svn_opt_revision_t peg_revision;
-          svn_boolean_t success;
 
           svn_pool_clear(iterpool);
           SVN_ERR(svn_cl__check_cancel(ctx->cancel_baton));
@@ -209,13 +209,10 @@ svn_cl__proplist(apr_getopt_t *os,
                                         opt_state->changelists,
                                         pl_receiver, &pl_baton,
                                         ctx, iterpool),
-                   &success, opt_state->quiet,
+                   errors, opt_state->quiet,
                    SVN_ERR_UNVERSIONED_RESOURCE,
                    SVN_ERR_ENTRY_NOT_FOUND,
                    SVN_NO_ERROR));
-
-          if (!success)
-            had_errors = TRUE;
         }
       svn_pool_destroy(iterpool);
 
@@ -223,10 +220,29 @@ svn_cl__proplist(apr_getopt_t *os,
         SVN_ERR(svn_cl__xml_print_footer("properties", scratch_pool));
 
       /* Error out *after* we closed the XML element */
-      if (had_errors)
-        return svn_error_create(SVN_ERR_ILLEGAL_TARGET, NULL,
-                                _("Could not display info for all targets "
-                                  "because some targets don't exist"));
+      if (errors->nelts > 0)
+        {
+          svn_error_t *err;
+          
+          err = svn_error_create(SVN_ERR_ILLEGAL_TARGET, NULL, NULL);
+          for (i = 0; i < errors->nelts; i++)
+            {
+              apr_status_t status = APR_ARRAY_IDX(errors, i, apr_status_t);
+              
+              if (status == SVN_ERR_ENTRY_NOT_FOUND)
+                err = svn_error_quick_wrap(err,
+                                           _("Could not display properties "
+                                             "of all targets because some "
+                                             "targets don't exist"));
+              else if (status == SVN_ERR_UNVERSIONED_RESOURCE)
+                err = svn_error_quick_wrap(err,
+                                           _("Could not display properties "
+                                             "of all targets because some "
+                                             "targets are not versioned"));
+            }
+
+          return svn_error_trace(err);
+        }
     }
 
   return SVN_NO_ERROR;

Modified: subversion/trunk/subversion/svn/util.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/util.c?rev=1185746&r1=1185745&r2=1185746&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/util.c (original)
+++ subversion/trunk/subversion/svn/util.c Tue Oct 18 16:37:59 2011
@@ -937,7 +937,7 @@ svn_cl__error_checked_fputs(const char *
 
 svn_error_t *
 svn_cl__try(svn_error_t *err,
-            svn_boolean_t *success,
+            apr_array_header_t *errors_seen,
             svn_boolean_t quiet,
             ...)
 {
@@ -946,12 +946,27 @@ svn_cl__try(svn_error_t *err,
       apr_status_t apr_err;
       va_list ap;
 
-      if (success)
-        *success = FALSE;
-
       va_start(ap, quiet);
       while ((apr_err = va_arg(ap, apr_status_t)) != SVN_NO_ERROR)
         {
+          if (errors_seen)
+            {
+              int i;
+              svn_boolean_t add = TRUE;
+
+              /* Don't report duplicate error codes. */
+              for (i = 0; i < errors_seen->nelts; i++)
+                {
+                  if (APR_ARRAY_IDX(errors_seen, i,
+                                    apr_status_t) == err->apr_err)
+                    {
+                      add = FALSE;
+                      break;
+                    }
+                }
+              if (add)
+                APR_ARRAY_PUSH(errors_seen, apr_status_t) = err->apr_err;
+            }
           if (err->apr_err == apr_err)
             {
               if (! quiet)
@@ -962,10 +977,6 @@ svn_cl__try(svn_error_t *err,
         }
       va_end(ap);
     }
-  else if (success)
-    {
-      *success = TRUE;
-    }
 
   return svn_error_trace(err);
 }

Modified: subversion/trunk/subversion/tests/cmdline/cat_tests.py
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/cat_tests.py?rev=1185746&r1=1185745&r2=1185746&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/cat_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/cat_tests.py Tue Oct 18 16:37:59 
2011
@@ -159,8 +159,7 @@ def cat_skip_uncattable(sbox):
                                        'cat', rho_path, new_file_path)
 
   expected_err3 = expected_err1 + expected_err2 + \
-      ".*svn: E200009: Could not cat all targets because some targets " + \
-      "don't exist\n"
+      ".*svn: E200009: Could not cat all targets because some targets"
   expected_err_re = re.compile(expected_err3, re.DOTALL)
 
   exit_code, output, error = svntest.main.run_svn(1, 'cat', rho_path, G_path, 
new_file_path)


Reply via email to