Stefan Sperling <s...@elego.de> writes:

> On Thu, Jan 20, 2011 at 04:48:58PM +0530, Noorul Islam K M wrote:
>
>> 
>> This patch is a followup of the following thread. All tests pass with
>> this patch.
>> 
>> http://svn.haxx.se/dev/archive-2011-01/0210.shtml
>> 
>> Log
>> [[[
>> 
>> Make svn 'add' command to return 1 when one or more targets fail. Also
>> print error message at the end.
>> 
>> * subversion/svn/add-cmd.c
>>   (svn_cl__add): If one or more target fails to get processed then print
>>     error message at end of command execution. Also return 1 to shell.
>> 
>> Patch by: Noorul Islam K M <noorul{_AT_}collab.net>
>> ]]]
>> 
>
>> Index: subversion/svn/add-cmd.c
>> ===================================================================
>> --- subversion/svn/add-cmd.c (revision 1060693)
>> +++ subversion/svn/add-cmd.c (working copy)
>> @@ -51,6 +51,7 @@
>>    apr_array_header_t *targets;
>>    int i;
>>    apr_pool_t *iterpool;
>> +  svn_boolean_t saw_a_problem = FALSE;
>
> Like with your ls patch, I would prefer a more specific variable name,
> such as "seen_nonexistent_target".
>
>>  
>>    SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
>>                                                        opt_state->targets,
>> @@ -79,6 +80,7 @@
>>    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));
>> @@ -87,12 +89,19 @@
>>                                 opt_state->depth,
>>                                 opt_state->force, opt_state->no_ignore,
>>                                 opt_state->parents, ctx, iterpool),
>> -               NULL, opt_state->quiet,
>> +               &success, opt_state->quiet,
>>                 SVN_ERR_ENTRY_EXISTS,
>>                 SVN_ERR_WC_PATH_NOT_FOUND,
>>                 SVN_NO_ERROR));
>> +
>> +      if (! success)
>> +        saw_a_problem = TRUE;
>>      }
>>  
>>    svn_pool_destroy(iterpool);
>> -  return SVN_NO_ERROR;
>> +
>> +  if (saw_a_problem)
>> +    return svn_error_create(SVN_ERR_BASE, NULL, NULL);
>> +  else
>> +    return SVN_NO_ERROR;
>>  }
>
> My comment about use of SVN_ERR_BASE and no error message in your
> ls patch applies here as well.

Incorporated review comments and attached is the update patch.

Thanks and Regards
Noorul

Index: subversion/svn/add-cmd.c
===================================================================
--- subversion/svn/add-cmd.c    (revision 1070486)
+++ subversion/svn/add-cmd.c    (working copy)
@@ -51,6 +51,7 @@
   apr_array_header_t *targets;
   int i;
   apr_pool_t *iterpool;
+  svn_boolean_t seen_nonexistent_target = FALSE;
 
   SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
                                                       opt_state->targets,
@@ -79,6 +80,7 @@
   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));
@@ -87,12 +89,21 @@
                                opt_state->depth,
                                opt_state->force, opt_state->no_ignore,
                                opt_state->parents, ctx, iterpool),
-               NULL, opt_state->quiet,
+               &success, 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);
-  return SVN_NO_ERROR;
+
+  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;
 }

Reply via email to