You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Noorul Islam K M <no...@collab.net> on 2011/07/14 07:17:51 UTC
Re: svn commit: r1145982 - in /subversion/trunk/subversion: svn/proplist-cmd.c tests/cmdline/tree_conflict_tests.py
rhuijben@apache.org writes:
> Author: rhuijben
> Date: Wed Jul 13 12:47:26 2011
> New Revision: 1145982
>
> URL: http://svn.apache.org/viewvc?rev=1145982&view=rev
> Log:
> Resolve another review todo from issue #3779 "how to handle actual only nodes"
> by making 'svn proplist' return a non zero exit code when one or more errors
> occurred.
>
> * subversion/svn/proplist-cmd.c
> (svn_cl__proplist): Check if all operations completed successfully and if
> not return an error.
>
> * subversion/tests/cmdline/tree_conflict_tests.py
> (actual_only_node_behaviour): Update expected result.
>
> Modified:
> subversion/trunk/subversion/svn/proplist-cmd.c
> subversion/trunk/subversion/tests/cmdline/tree_conflict_tests.py
>
> Modified: subversion/trunk/subversion/svn/proplist-cmd.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/proplist-cmd.c?rev=1145982&r1=1145981&r2=1145982&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/svn/proplist-cmd.c (original)
> +++ subversion/trunk/subversion/svn/proplist-cmd.c Wed Jul 13 12:47:26 2011
> @@ -168,6 +168,7 @@ 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)
> {
> @@ -189,6 +190,7 @@ 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));
> @@ -200,22 +202,31 @@ svn_cl__proplist(apr_getopt_t *os,
> SVN_ERR(svn_opt_parse_path(&peg_revision, &truepath, target,
> iterpool));
>
> - SVN_ERR(svn_cl__try
> - (svn_client_proplist3(truepath, &peg_revision,
> + SVN_ERR(svn_cl__try(
> + svn_client_proplist3(truepath, &peg_revision,
> &(opt_state->start_revision),
> opt_state->depth,
> opt_state->changelists,
> pl_receiver, &pl_baton,
> ctx, iterpool),
> - NULL, opt_state->quiet,
> + &success, opt_state->quiet,
> SVN_ERR_UNVERSIONED_RESOURCE,
> SVN_ERR_ENTRY_NOT_FOUND,
> SVN_NO_ERROR));
> +
> + if (!success)
> + had_errors = TRUE;
> }
> svn_pool_destroy(iterpool);
>
> if (opt_state->xml)
> 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"));
I think here also the error message seems to incorrect. copy-pasto?
Thanks and Regards
Noorul
Re: svn commit: r1145982 - in /subversion/trunk/subversion: svn/proplist-cmd.c tests/cmdline/tree_conflict_tests.py
Posted by Noorul Islam K M <no...@collab.net>.
Noorul Islam K M <no...@collab.net> writes:
> "Bert Huijben" <be...@qqmail.nl> writes:
>
>>> -----Original Message-----
>>> From: Noorul Islam K M [mailto:noorul@collab.net]
>>> Sent: donderdag 14 juli 2011 7:18
>>> To: dev@subversion.apache.org
>>> Cc: commits@subversion.apache.org
>>> Subject: Re: svn commit: r1145982 - in /subversion/trunk/subversion:
>>> svn/proplist-cmd.c tests/cmdline/tree_conflict_tests.py
>>>
>>> rhuijben@apache.org writes:
>>>
>>> > Author: rhuijben
>>> > Date: Wed Jul 13 12:47:26 2011
>>> > New Revision: 1145982
>>> >
>>> > URL: http://svn.apache.org/viewvc?rev=1145982&view=rev
>>> > Log:
>>> > Resolve another review todo from issue #3779 "how to handle actual only
>>> nodes"
>>> > by making 'svn proplist' return a non zero exit code when one or more
>>> errors
>>> > occurred.
>>
>>> > if (opt_state->xml)
>>> > 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"));
>>>
>>> I think here also the error message seems to incorrect. copy-pasto?
>>
>> Noorul,
>>
>> How is this incorrect?
>>
>> Maybe it can be improved, but that is not a bug.
>
> I think I should not have used the work incorrect. I agree that it could
> be improved.
s/work/word.
- Noorul
>
>>
>> Like in the other patch I just added the generic error text as it is not
>> easy to provide a correct specific error.
>>
>
> Why is it not easy to provide correct specific error? Is it because we
> do not have translated message?
>
> Thanks and Regards
> Noorul
Re: svn commit: r1145982 - in /subversion/trunk/subversion: svn/proplist-cmd.c tests/cmdline/tree_conflict_tests.py
Posted by Noorul Islam K M <no...@collab.net>.
"Bert Huijben" <be...@qqmail.nl> writes:
>> -----Original Message-----
>> From: Noorul Islam K M [mailto:noorul@collab.net]
>> Sent: donderdag 14 juli 2011 7:18
>> To: dev@subversion.apache.org
>> Cc: commits@subversion.apache.org
>> Subject: Re: svn commit: r1145982 - in /subversion/trunk/subversion:
>> svn/proplist-cmd.c tests/cmdline/tree_conflict_tests.py
>>
>> rhuijben@apache.org writes:
>>
>> > Author: rhuijben
>> > Date: Wed Jul 13 12:47:26 2011
>> > New Revision: 1145982
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1145982&view=rev
>> > Log:
>> > Resolve another review todo from issue #3779 "how to handle actual only
>> nodes"
>> > by making 'svn proplist' return a non zero exit code when one or more
>> errors
>> > occurred.
>
>> > if (opt_state->xml)
>> > 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"));
>>
>> I think here also the error message seems to incorrect. copy-pasto?
>
> Noorul,
>
> How is this incorrect?
>
> Maybe it can be improved, but that is not a bug.
I think I should not have used the work incorrect. I agree that it could
be improved.
>
> Like in the other patch I just added the generic error text as it is not
> easy to provide a correct specific error.
>
Why is it not easy to provide correct specific error? Is it because we
do not have translated message?
Thanks and Regards
Noorul
RE: svn commit: r1145982 - in /subversion/trunk/subversion: svn/proplist-cmd.c tests/cmdline/tree_conflict_tests.py
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Noorul Islam K M [mailto:noorul@collab.net]
> Sent: donderdag 14 juli 2011 7:18
> To: dev@subversion.apache.org
> Cc: commits@subversion.apache.org
> Subject: Re: svn commit: r1145982 - in /subversion/trunk/subversion:
> svn/proplist-cmd.c tests/cmdline/tree_conflict_tests.py
>
> rhuijben@apache.org writes:
>
> > Author: rhuijben
> > Date: Wed Jul 13 12:47:26 2011
> > New Revision: 1145982
> >
> > URL: http://svn.apache.org/viewvc?rev=1145982&view=rev
> > Log:
> > Resolve another review todo from issue #3779 "how to handle actual only
> nodes"
> > by making 'svn proplist' return a non zero exit code when one or more
> errors
> > occurred.
> > if (opt_state->xml)
> > 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"));
>
> I think here also the error message seems to incorrect. copy-pasto?
Noorul,
How is this incorrect?
Maybe it can be improved, but that is not a bug.
Like in the other patch I just added the generic error text as it is not
easy to provide a correct specific error.
Bert