You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2010/11/10 02:44:36 UTC
svn commit: r1033320 - in /subversion/trunk/subversion:
include/svn_error_codes.h svn/cl.h svn/export-cmd.c svn/notify.c
svn/switch-cmd.c svn/update-cmd.c tests/cmdline/externals_tests.py
Author: cmpilato
Date: Wed Nov 10 01:44:35 2010
New Revision: 1033320
URL: http://svn.apache.org/viewvc?rev=1033320&view=rev
Log:
Fix issue #3622 ("svn should exit with exit code 1 if updating
externals fails").
* subversion/include/svn_error_codes.h
(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS): New error code.
* subversion/svn/cl.h
(struct svn_cl__check_externals_failed_notify_baton): New baton.
(svn_cl__check_externals_failed_notify_wrapper): New function.
* subversion/svn/notify.c
(svn_cl__check_externals_failed_notify_wrapper): New function.
* subversion/svn/update-cmd.c
(svn_cl__update): Use the new wrapper baton and function to track
externals processing failures, and return an error after all else
is finished if such failures occured.
* subversion/svn/export-cmd.c
(svn_cl__export): Use the new wrapper baton and function to track
externals processing failures, and return an error after all else
is finished if such failures occured.
* subversion/svn/switch-cmd.c
(svn_cl__switch): Use the new wrapper baton and function to track
externals processing failures, and return an error after all else
is finished if such failures occured.
* subversion/tests/cmdline/externals_tests.py
(old_style_externals_ignore_peg_reg,
can_place_file_external_into_dir_external): Change expected exit code.
Modified:
subversion/trunk/subversion/include/svn_error_codes.h
subversion/trunk/subversion/svn/cl.h
subversion/trunk/subversion/svn/export-cmd.c
subversion/trunk/subversion/svn/notify.c
subversion/trunk/subversion/svn/switch-cmd.c
subversion/trunk/subversion/svn/update-cmd.c
subversion/trunk/subversion/tests/cmdline/externals_tests.py
Modified: subversion/trunk/subversion/include/svn_error_codes.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_error_codes.h?rev=1033320&r1=1033319&r2=1033320&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_error_codes.h (original)
+++ subversion/trunk/subversion/include/svn_error_codes.h Wed Nov 10 01:44:35 2010
@@ -1391,6 +1391,10 @@ SVN_ERROR_START
SVN_ERR_CL_CATEGORY_START + 10,
"No external merge tool available")
+ SVN_ERRDEF(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS,
+ SVN_ERR_CL_CATEGORY_START + 11,
+ "Failed processing one or more externals definitions")
+
/* malfunctions such as assertion failures */
SVN_ERRDEF(SVN_ERR_ASSERTION_FAIL,
Modified: subversion/trunk/subversion/svn/cl.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/cl.h?rev=1033320&r1=1033319&r2=1033320&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/cl.h (original)
+++ subversion/trunk/subversion/svn/cl.h Wed Nov 10 01:44:35 2010
@@ -569,6 +569,21 @@ svn_cl__notifier_mark_checkout(void *bat
svn_error_t *
svn_cl__notifier_mark_export(void *baton);
+/* Baton for use with svn_cl__check_externals_failed_notify_wrapper(). */
+struct svn_cl__check_externals_failed_notify_baton
+{
+ void *wrapped_baton; /* The "real" notify_func2. */
+ svn_wc_notify_func2_t wrapped_func; /* The "real" notify_func2 baton. */
+ svn_boolean_t had_externals_error; /* Did something fail in an external? */
+};
+
+/* Notification function wrapper (implements `svn_wc_notify_func2_t').
+ Use with an svn_cl__check_externals_failed_notify_baton BATON. */
+void
+svn_cl__check_externals_failed_notify_wrapper(void *baton,
+ const svn_wc_notify_t *n,
+ apr_pool_t *pool);
+
/* Print conflict stats accumulated in NOTIFY_BATON.
* Return any error encountered during printing.
* Do all allocations in POOL.*/
Modified: subversion/trunk/subversion/svn/export-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/export-cmd.c?rev=1033320&r1=1033319&r2=1033320&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/export-cmd.c (original)
+++ subversion/trunk/subversion/svn/export-cmd.c Wed Nov 10 01:44:35 2010
@@ -52,6 +52,7 @@ svn_cl__export(apr_getopt_t *os,
svn_error_t *err;
svn_opt_revision_t peg_revision;
const char *truefrom;
+ struct svn_cl__check_externals_failed_notify_baton nwb;
SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
opt_state->targets,
@@ -95,6 +96,12 @@ svn_cl__export(apr_getopt_t *os,
if (opt_state->depth == svn_depth_unknown)
opt_state->depth = svn_depth_infinity;
+ nwb.wrapped_func = ctx->notify_func2;
+ nwb.wrapped_baton = ctx->notify_baton2;
+ nwb.had_externals_error = FALSE;
+ ctx->notify_func2 = svn_cl__check_externals_failed_notify_wrapper;
+ ctx->notify_baton2 = &nwb;
+
/* Do the export. */
err = svn_client_export5(NULL, truefrom, to, &peg_revision,
&(opt_state->start_revision),
@@ -106,5 +113,10 @@ svn_cl__export(apr_getopt_t *os,
_("Destination directory exists; please remove "
"the directory or use --force to overwrite"));
+ if (nwb.had_externals_error)
+ return svn_error_create(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS, NULL,
+ _("Failure occured processing one or more "
+ "externals definitions"));
+
return svn_error_return(err);
}
Modified: subversion/trunk/subversion/svn/notify.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/notify.c?rev=1033320&r1=1033319&r2=1033320&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/notify.c (original)
+++ subversion/trunk/subversion/svn/notify.c Wed Nov 10 01:44:35 2010
@@ -952,3 +952,18 @@ svn_cl__notifier_mark_export(void *baton
nb->is_export = TRUE;
return SVN_NO_ERROR;
}
+
+void
+svn_cl__check_externals_failed_notify_wrapper(void *baton,
+ const svn_wc_notify_t *n,
+ apr_pool_t *pool)
+{
+ struct svn_cl__check_externals_failed_notify_baton *nwb = baton;
+
+ if (n->action == svn_wc_notify_failed_external)
+ nwb->had_externals_error = TRUE;
+
+ if (nwb->wrapped_func)
+ nwb->wrapped_func(nwb->wrapped_baton, n, pool);
+}
+
Modified: subversion/trunk/subversion/svn/switch-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/switch-cmd.c?rev=1033320&r1=1033319&r2=1033320&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/switch-cmd.c (original)
+++ subversion/trunk/subversion/svn/switch-cmd.c Wed Nov 10 01:44:35 2010
@@ -101,6 +101,7 @@ svn_cl__switch(apr_getopt_t *os,
svn_opt_revision_t peg_revision;
svn_depth_t depth;
svn_boolean_t depth_is_sticky;
+ struct svn_cl__check_externals_failed_notify_baton nwb;
/* This command should discover (or derive) exactly two cmdline
arguments: a local path to update ("target"), and a new url to
@@ -158,6 +159,12 @@ svn_cl__switch(apr_getopt_t *os,
depth_is_sticky = FALSE;
}
+ nwb.wrapped_func = ctx->notify_func2;
+ nwb.wrapped_baton = ctx->notify_baton2;
+ nwb.had_externals_error = FALSE;
+ ctx->notify_func2 = svn_cl__check_externals_failed_notify_wrapper;
+ ctx->notify_baton2 = &nwb;
+
/* Do the 'switch' update. */
SVN_ERR(svn_client_switch2(NULL, target, switch_url, &peg_revision,
&(opt_state->start_revision), depth,
@@ -165,7 +172,12 @@ svn_cl__switch(apr_getopt_t *os,
opt_state->force, ctx, scratch_pool));
if (! opt_state->quiet)
- SVN_ERR(svn_cl__print_conflict_stats(ctx->notify_baton2, scratch_pool));
+ SVN_ERR(svn_cl__print_conflict_stats(nwb.wrapped_baton, scratch_pool));
+
+ if (nwb.had_externals_error)
+ return svn_error_create(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS, NULL,
+ _("Failure occured processing one or more "
+ "externals definitions"));
return SVN_NO_ERROR;
}
Modified: subversion/trunk/subversion/svn/update-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/update-cmd.c?rev=1033320&r1=1033319&r2=1033320&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/update-cmd.c (original)
+++ subversion/trunk/subversion/svn/update-cmd.c Wed Nov 10 01:44:35 2010
@@ -50,6 +50,7 @@ svn_cl__update(apr_getopt_t *os,
apr_array_header_t *targets;
svn_depth_t depth;
svn_boolean_t depth_is_sticky;
+ struct svn_cl__check_externals_failed_notify_baton nwb;
SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
opt_state->targets,
@@ -85,6 +86,12 @@ svn_cl__update(apr_getopt_t *os,
depth_is_sticky = FALSE;
}
+ nwb.wrapped_func = ctx->notify_func2;
+ nwb.wrapped_baton = ctx->notify_baton2;
+ nwb.had_externals_error = FALSE;
+ ctx->notify_func2 = svn_cl__check_externals_failed_notify_wrapper;
+ ctx->notify_baton2 = &nwb;
+
SVN_ERR(svn_client_update3(NULL, targets,
&(opt_state->start_revision),
depth, depth_is_sticky,
@@ -93,7 +100,12 @@ svn_cl__update(apr_getopt_t *os,
ctx, scratch_pool));
if (! opt_state->quiet)
- SVN_ERR(svn_cl__print_conflict_stats(ctx->notify_baton2, scratch_pool));
+ SVN_ERR(svn_cl__print_conflict_stats(nwb.wrapped_baton, scratch_pool));
+
+ if (nwb.had_externals_error)
+ return svn_error_create(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS, NULL,
+ _("Failure occured processing one or more "
+ "externals definitions"));
return SVN_NO_ERROR;
}
Modified: subversion/trunk/subversion/tests/cmdline/externals_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/externals_tests.py?rev=1033320&r1=1033319&r2=1033320&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/externals_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/externals_tests.py Wed Nov 10 01:44:35 2010
@@ -947,7 +947,7 @@ def old_style_externals_ignore_peg_reg(s
svntest.actions.run_and_verify_svn2("External '%s' used pegs" % ext.strip(),
None,
expected_error,
- 0,
+ 1,
'up',
wc_dir)
@@ -1063,7 +1063,7 @@ def can_place_file_external_into_dir_ext
svntest.actions.run_and_verify_svn2("Able to put file external in foreign wc",
None,
expected_error,
- 0,
+ 1,
'up',
repo_url, wc_dir)
Re: svn commit: r1033320 - in /subversion/trunk/subversion: include/svn_error_codes.h
svn/cl.h svn/export-cmd.c svn/notify.c svn/switch-cmd.c svn/update-cmd.c
tests/cmdline/externals_tests.py
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/10/2010 05:21 AM, Julian Foad wrote:
> I'm wondering about the wrapper function approach - did you feel it's
> important to separate this error detection from the notification
> function? I wonder if it would be simpler overall to get svn's existing
> notifier code to track the presence of errors for you, instead.
I actually took this route first, only to find failure in various ways. Not
all functions use the generic notification system -- notably, *none* of them
use it if you pass --quiet. In the end , I used the paradigm employed by
"propdel" with the custom notification wrapper.
>> +/* Baton for use with svn_cl__check_externals_failed_notify_wrapper(). */
>> +struct svn_cl__check_externals_failed_notify_baton
>> +{
>> + void *wrapped_baton; /* The "real" notify_func2. */
>> + svn_wc_notify_func2_t wrapped_func; /* The "real" notify_func2 baton. */
>
> Those two are swapped relative to their comments.
Doh! Will fix.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1033320 - in /subversion/trunk/subversion:
include/svn_error_codes.h svn/cl.h svn/export-cmd.c svn/notify.c
svn/switch-cmd.c svn/update-cmd.c tests/cmdline/externals_tests.py
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Nov 24, 2010 at 08:57:22AM -0500, C. Michael Pilato wrote:
> Gavin, you're a volunteer, offering a service we very much appreciate but
> which we realize is offered only out of your free and good will. We've
> already established the minimum expectations of someone serving in the Patch
> Manager role. So IMO, if you *want* to contribute to threads such as this
> one (between committers) with a *poke* or additional information or
> whatever, please do so.
+1 Poking never hurts! (pun on purpose :)
Re: svn commit: r1033320 - in /subversion/trunk/subversion: include/svn_error_codes.h
svn/cl.h svn/export-cmd.c svn/notify.c svn/switch-cmd.c svn/update-cmd.c
tests/cmdline/externals_tests.py
Posted by "C. Michael Pilato" <cm...@collab.net>.
Gavin, you're a volunteer, offering a service we very much appreciate but
which we realize is offered only out of your free and good will. We've
already established the minimum expectations of someone serving in the Patch
Manager role. So IMO, if you *want* to contribute to threads such as this
one (between committers) with a *poke* or additional information or
whatever, please do so.
(In this particular case, there's nothing to do. I fixed the code review
issues Julian mentioned, and shot down his concerns about the wrapper
function usage.)
On 11/24/2010 07:00 AM, Gavin Beau Baumanis wrote:
> Hi Everyone,
>
> I thought I would use this thread to solicit a few opinions about threads like this one and the Patch Manager's role with them.
>
> In a previous thread Hyrum noted that because the discussion was between full-committers there wasn't necessarily a role for the PM to play within the scope of that topic / thread.
> I.e. there is a discussion happening about development work - perhaps there is even a patch submission (or commit) being discussed on list by developers who have commit privileges.
>
>
> So it would seem sensible that I shouldn't "bother" to keep a watch on this specific one either.
>
> But is it so clear-cut?
> Should I only advocate submissions by non-committers, or is it appropriate, if I note a discussion missing a conclusion to bring that back to the lists attention, too?
> I don't mind either way - though my job (while not particularly demanding in the first place), would be "easier" if I simply ignored threads where the "mantle" was taken up by full-committers.
>
> I'm happy to do the work - but at the same time I'm mindful of creating unnecessary noise on the list.
>
>
> Gavin "Beau" Baumanis
>
>
> On 10/11/2010, at 9:21 PM, Julian Foad wrote:
>
>> On Wed, 2010-11-10, cmpilato@apache.org wrote:
>>> Author: cmpilato
>>> Date: Wed Nov 10 01:44:35 2010
>>> New Revision: 1033320
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1033320&view=rev
>>> Log:
>>> Fix issue #3622 ("svn should exit with exit code 1 if updating
>>> externals fails").
>>>
>>> * subversion/include/svn_error_codes.h
>>> (SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS): New error code.
>>>
>>> * subversion/svn/cl.h
>>> (struct svn_cl__check_externals_failed_notify_baton): New baton.
>>> (svn_cl__check_externals_failed_notify_wrapper): New function.
>>>
>>> * subversion/svn/notify.c
>>> (svn_cl__check_externals_failed_notify_wrapper): New function.
>>>
>>> * subversion/svn/update-cmd.c
>>> (svn_cl__update): Use the new wrapper baton and function to track
>>> externals processing failures, and return an error after all else
>>> is finished if such failures occured.
>>>
>>> * subversion/svn/export-cmd.c
>>> (svn_cl__export): Use the new wrapper baton and function to track
>>> externals processing failures, and return an error after all else
>>> is finished if such failures occured.
>>>
>>> * subversion/svn/switch-cmd.c
>>> (svn_cl__switch): Use the new wrapper baton and function to track
>>> externals processing failures, and return an error after all else
>>> is finished if such failures occured.
>>>
>>> * subversion/tests/cmdline/externals_tests.py
>>> (old_style_externals_ignore_peg_reg,
>>> can_place_file_external_into_dir_external): Change expected exit code.
>>
>> Hi Mike. Useful fix.
>>
>> I see that as well as reporting errors in externals, svn_wc_notify_t can
>> also report errors in locking:
>>
>> /** Points to an error describing the reason for the failure when @c
>> * action is one of the following: #svn_wc_notify_failed_lock,
>> * #svn_wc_notify_failed_unlock, #svn_wc_notify_failed_external.
>> * Is @c NULL otherwise. */
>> svn_error_t *err;
>>
>> Without investigating further, it seems to me that we will want to do
>> something similar for locking errors too. With this in mind, it might
>> be worth generalizing the wrapper stuff to "had an error" rather than
>> "had an externals error". (It can check nb->err != SVN_NO_ERROR rather
>> than nb->status==...)
>>
>>
>> I'm wondering about the wrapper function approach - did you feel it's
>> important to separate this error detection from the notification
>> function? I wonder if it would be simpler overall to get svn's existing
>> notifier code to track the presence of errors for you, instead. The
>> basic approach would be: add the flag
>>
>> svn_boolean_t had_error;
>>
>> to the private "struct notify_baton"; add a getter such as
>>
>> svn_boolean_t svn_cl__notifier_had_error(void *baton);
>>
>> that gets this flag; and make notify() do
>>
>> if (n->err)
>> nb->had_error = TRUE;
>>
>> Then the various subcommands would only need to call this function to
>> read the "had_error" indication. What do you think?
>>
>>
>>> Modified: subversion/trunk/subversion/include/svn_error_codes.h
>>> ==============================================================================
>>> --- subversion/trunk/subversion/include/svn_error_codes.h (original)
>>> +++ subversion/trunk/subversion/include/svn_error_codes.h Wed Nov 10 01:44:35 2010
>>> @@ -1391,6 +1391,10 @@ SVN_ERROR_START
>>> SVN_ERR_CL_CATEGORY_START + 10,
>>> "No external merge tool available")
>>>
>>> + SVN_ERRDEF(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS,
>>> + SVN_ERR_CL_CATEGORY_START + 11,
>>> + "Failed processing one or more externals definitions")
>>> +
>>> /* malfunctions such as assertion failures */
>>>
>>> SVN_ERRDEF(SVN_ERR_ASSERTION_FAIL,
>>>
>>> Modified: subversion/trunk/subversion/svn/cl.h
>>> ==============================================================================
>>> --- subversion/trunk/subversion/svn/cl.h (original)
>>> +++ subversion/trunk/subversion/svn/cl.h Wed Nov 10 01:44:35 2010
>>> @@ -569,6 +569,21 @@ svn_cl__notifier_mark_checkout(void *bat
>>> svn_error_t *
>>> svn_cl__notifier_mark_export(void *baton);
>>>
>>> +/* Baton for use with svn_cl__check_externals_failed_notify_wrapper(). */
>>> +struct svn_cl__check_externals_failed_notify_baton
>>> +{
>>> + void *wrapped_baton; /* The "real" notify_func2. */
>>> + svn_wc_notify_func2_t wrapped_func; /* The "real" notify_func2 baton. */
>>
>> Those two are swapped relative to their comments.
>>
>>> + svn_boolean_t had_externals_error; /* Did something fail in an external? */
>>> +};
>>> +
>>> +/* Notification function wrapper (implements `svn_wc_notify_func2_t').
>>> + Use with an svn_cl__check_externals_failed_notify_baton BATON. */
>>> +void
>>> +svn_cl__check_externals_failed_notify_wrapper(void *baton,
>>> + const svn_wc_notify_t *n,
>>> + apr_pool_t *pool);
>>> +
>>> /* Print conflict stats accumulated in NOTIFY_BATON.
>>> * Return any error encountered during printing.
>>> * Do all allocations in POOL.*/
>>>
>>> Modified: subversion/trunk/subversion/svn/export-cmd.c
>>> ==============================================================================
>>> --- subversion/trunk/subversion/svn/export-cmd.c (original)
>>> +++ subversion/trunk/subversion/svn/export-cmd.c Wed Nov 10 01:44:35 2010
>>> @@ -52,6 +52,7 @@ svn_cl__export(apr_getopt_t *os,
>>> svn_error_t *err;
>>> svn_opt_revision_t peg_revision;
>>> const char *truefrom;
>>> + struct svn_cl__check_externals_failed_notify_baton nwb;
>>>
>>> SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
>>> opt_state->targets,
>>> @@ -95,6 +96,12 @@ svn_cl__export(apr_getopt_t *os,
>>> if (opt_state->depth == svn_depth_unknown)
>>> opt_state->depth = svn_depth_infinity;
>>>
>>> + nwb.wrapped_func = ctx->notify_func2;
>>> + nwb.wrapped_baton = ctx->notify_baton2;
>>> + nwb.had_externals_error = FALSE;
>>> + ctx->notify_func2 = svn_cl__check_externals_failed_notify_wrapper;
>>> + ctx->notify_baton2 = &nwb;
>>> +
>>> /* Do the export. */
>>> err = svn_client_export5(NULL, truefrom, to, &peg_revision,
>>> &(opt_state->start_revision),
>>> @@ -106,5 +113,10 @@ svn_cl__export(apr_getopt_t *os,
>>> _("Destination directory exists; please remove "
>>> "the directory or use --force to overwrite"));
>>>
>>> + if (nwb.had_externals_error)
>>> + return svn_error_create(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS, NULL,
>>> + _("Failure occured processing one or more "
>>> + "externals definitions"));
>>> +
>>> return svn_error_return(err);
>>> }
>>>
>>> Modified: subversion/trunk/subversion/svn/notify.c
>>> ==============================================================================
>>> --- subversion/trunk/subversion/svn/notify.c (original)
>>> +++ subversion/trunk/subversion/svn/notify.c Wed Nov 10 01:44:35 2010
>>> @@ -952,3 +952,18 @@ svn_cl__notifier_mark_export(void *baton
>>> nb->is_export = TRUE;
>>> return SVN_NO_ERROR;
>>> }
>>> +
>>> +void
>>> +svn_cl__check_externals_failed_notify_wrapper(void *baton,
>>> + const svn_wc_notify_t *n,
>>> + apr_pool_t *pool)
>>> +{
>>> + struct svn_cl__check_externals_failed_notify_baton *nwb = baton;
>>> +
>>> + if (n->action == svn_wc_notify_failed_external)
>>> + nwb->had_externals_error = TRUE;
>>> +
>>> + if (nwb->wrapped_func)
>>> + nwb->wrapped_func(nwb->wrapped_baton, n, pool);
>>> +}
>>
>> All the rest looks fine.
>>
>> - Julian
>>
>>
>>
>
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1033320 - in /subversion/trunk/subversion: include/svn_error_codes.h svn/cl.h svn/export-cmd.c svn/notify.c svn/switch-cmd.c svn/update-cmd.c tests/cmdline/externals_tests.py
Posted by Gavin Beau Baumanis <ga...@thespidernet.com>.
Hi Everyone,
I thought I would use this thread to solicit a few opinions about threads like this one and the Patch Manager's role with them.
In a previous thread Hyrum noted that because the discussion was between full-committers there wasn't necessarily a role for the PM to play within the scope of that topic / thread.
I.e. there is a discussion happening about development work - perhaps there is even a patch submission (or commit) being discussed on list by developers who have commit privileges.
So it would seem sensible that I shouldn't "bother" to keep a watch on this specific one either.
But is it so clear-cut?
Should I only advocate submissions by non-committers, or is it appropriate, if I note a discussion missing a conclusion to bring that back to the lists attention, too?
I don't mind either way - though my job (while not particularly demanding in the first place), would be "easier" if I simply ignored threads where the "mantle" was taken up by full-committers.
I'm happy to do the work - but at the same time I'm mindful of creating unnecessary noise on the list.
Gavin "Beau" Baumanis
On 10/11/2010, at 9:21 PM, Julian Foad wrote:
> On Wed, 2010-11-10, cmpilato@apache.org wrote:
>> Author: cmpilato
>> Date: Wed Nov 10 01:44:35 2010
>> New Revision: 1033320
>>
>> URL: http://svn.apache.org/viewvc?rev=1033320&view=rev
>> Log:
>> Fix issue #3622 ("svn should exit with exit code 1 if updating
>> externals fails").
>>
>> * subversion/include/svn_error_codes.h
>> (SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS): New error code.
>>
>> * subversion/svn/cl.h
>> (struct svn_cl__check_externals_failed_notify_baton): New baton.
>> (svn_cl__check_externals_failed_notify_wrapper): New function.
>>
>> * subversion/svn/notify.c
>> (svn_cl__check_externals_failed_notify_wrapper): New function.
>>
>> * subversion/svn/update-cmd.c
>> (svn_cl__update): Use the new wrapper baton and function to track
>> externals processing failures, and return an error after all else
>> is finished if such failures occured.
>>
>> * subversion/svn/export-cmd.c
>> (svn_cl__export): Use the new wrapper baton and function to track
>> externals processing failures, and return an error after all else
>> is finished if such failures occured.
>>
>> * subversion/svn/switch-cmd.c
>> (svn_cl__switch): Use the new wrapper baton and function to track
>> externals processing failures, and return an error after all else
>> is finished if such failures occured.
>>
>> * subversion/tests/cmdline/externals_tests.py
>> (old_style_externals_ignore_peg_reg,
>> can_place_file_external_into_dir_external): Change expected exit code.
>
> Hi Mike. Useful fix.
>
> I see that as well as reporting errors in externals, svn_wc_notify_t can
> also report errors in locking:
>
> /** Points to an error describing the reason for the failure when @c
> * action is one of the following: #svn_wc_notify_failed_lock,
> * #svn_wc_notify_failed_unlock, #svn_wc_notify_failed_external.
> * Is @c NULL otherwise. */
> svn_error_t *err;
>
> Without investigating further, it seems to me that we will want to do
> something similar for locking errors too. With this in mind, it might
> be worth generalizing the wrapper stuff to "had an error" rather than
> "had an externals error". (It can check nb->err != SVN_NO_ERROR rather
> than nb->status==...)
>
>
> I'm wondering about the wrapper function approach - did you feel it's
> important to separate this error detection from the notification
> function? I wonder if it would be simpler overall to get svn's existing
> notifier code to track the presence of errors for you, instead. The
> basic approach would be: add the flag
>
> svn_boolean_t had_error;
>
> to the private "struct notify_baton"; add a getter such as
>
> svn_boolean_t svn_cl__notifier_had_error(void *baton);
>
> that gets this flag; and make notify() do
>
> if (n->err)
> nb->had_error = TRUE;
>
> Then the various subcommands would only need to call this function to
> read the "had_error" indication. What do you think?
>
>
>> Modified: subversion/trunk/subversion/include/svn_error_codes.h
>> ==============================================================================
>> --- subversion/trunk/subversion/include/svn_error_codes.h (original)
>> +++ subversion/trunk/subversion/include/svn_error_codes.h Wed Nov 10 01:44:35 2010
>> @@ -1391,6 +1391,10 @@ SVN_ERROR_START
>> SVN_ERR_CL_CATEGORY_START + 10,
>> "No external merge tool available")
>>
>> + SVN_ERRDEF(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS,
>> + SVN_ERR_CL_CATEGORY_START + 11,
>> + "Failed processing one or more externals definitions")
>> +
>> /* malfunctions such as assertion failures */
>>
>> SVN_ERRDEF(SVN_ERR_ASSERTION_FAIL,
>>
>> Modified: subversion/trunk/subversion/svn/cl.h
>> ==============================================================================
>> --- subversion/trunk/subversion/svn/cl.h (original)
>> +++ subversion/trunk/subversion/svn/cl.h Wed Nov 10 01:44:35 2010
>> @@ -569,6 +569,21 @@ svn_cl__notifier_mark_checkout(void *bat
>> svn_error_t *
>> svn_cl__notifier_mark_export(void *baton);
>>
>> +/* Baton for use with svn_cl__check_externals_failed_notify_wrapper(). */
>> +struct svn_cl__check_externals_failed_notify_baton
>> +{
>> + void *wrapped_baton; /* The "real" notify_func2. */
>> + svn_wc_notify_func2_t wrapped_func; /* The "real" notify_func2 baton. */
>
> Those two are swapped relative to their comments.
>
>> + svn_boolean_t had_externals_error; /* Did something fail in an external? */
>> +};
>> +
>> +/* Notification function wrapper (implements `svn_wc_notify_func2_t').
>> + Use with an svn_cl__check_externals_failed_notify_baton BATON. */
>> +void
>> +svn_cl__check_externals_failed_notify_wrapper(void *baton,
>> + const svn_wc_notify_t *n,
>> + apr_pool_t *pool);
>> +
>> /* Print conflict stats accumulated in NOTIFY_BATON.
>> * Return any error encountered during printing.
>> * Do all allocations in POOL.*/
>>
>> Modified: subversion/trunk/subversion/svn/export-cmd.c
>> ==============================================================================
>> --- subversion/trunk/subversion/svn/export-cmd.c (original)
>> +++ subversion/trunk/subversion/svn/export-cmd.c Wed Nov 10 01:44:35 2010
>> @@ -52,6 +52,7 @@ svn_cl__export(apr_getopt_t *os,
>> svn_error_t *err;
>> svn_opt_revision_t peg_revision;
>> const char *truefrom;
>> + struct svn_cl__check_externals_failed_notify_baton nwb;
>>
>> SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
>> opt_state->targets,
>> @@ -95,6 +96,12 @@ svn_cl__export(apr_getopt_t *os,
>> if (opt_state->depth == svn_depth_unknown)
>> opt_state->depth = svn_depth_infinity;
>>
>> + nwb.wrapped_func = ctx->notify_func2;
>> + nwb.wrapped_baton = ctx->notify_baton2;
>> + nwb.had_externals_error = FALSE;
>> + ctx->notify_func2 = svn_cl__check_externals_failed_notify_wrapper;
>> + ctx->notify_baton2 = &nwb;
>> +
>> /* Do the export. */
>> err = svn_client_export5(NULL, truefrom, to, &peg_revision,
>> &(opt_state->start_revision),
>> @@ -106,5 +113,10 @@ svn_cl__export(apr_getopt_t *os,
>> _("Destination directory exists; please remove "
>> "the directory or use --force to overwrite"));
>>
>> + if (nwb.had_externals_error)
>> + return svn_error_create(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS, NULL,
>> + _("Failure occured processing one or more "
>> + "externals definitions"));
>> +
>> return svn_error_return(err);
>> }
>>
>> Modified: subversion/trunk/subversion/svn/notify.c
>> ==============================================================================
>> --- subversion/trunk/subversion/svn/notify.c (original)
>> +++ subversion/trunk/subversion/svn/notify.c Wed Nov 10 01:44:35 2010
>> @@ -952,3 +952,18 @@ svn_cl__notifier_mark_export(void *baton
>> nb->is_export = TRUE;
>> return SVN_NO_ERROR;
>> }
>> +
>> +void
>> +svn_cl__check_externals_failed_notify_wrapper(void *baton,
>> + const svn_wc_notify_t *n,
>> + apr_pool_t *pool)
>> +{
>> + struct svn_cl__check_externals_failed_notify_baton *nwb = baton;
>> +
>> + if (n->action == svn_wc_notify_failed_external)
>> + nwb->had_externals_error = TRUE;
>> +
>> + if (nwb->wrapped_func)
>> + nwb->wrapped_func(nwb->wrapped_baton, n, pool);
>> +}
>
> All the rest looks fine.
>
> - Julian
>
>
>
Re: svn commit: r1033320 - in /subversion/trunk/subversion: include/svn_error_codes.h
svn/cl.h svn/export-cmd.c svn/notify.c svn/switch-cmd.c svn/update-cmd.c
tests/cmdline/externals_tests.py
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/10/2010 05:21 AM, Julian Foad wrote:
> I'm wondering about the wrapper function approach - did you feel it's
> important to separate this error detection from the notification
> function? I wonder if it would be simpler overall to get svn's existing
> notifier code to track the presence of errors for you, instead.
I actually took this route first, only to find failure in various ways. Not
all functions use the generic notification system -- notably, *none* of them
use it if you pass --quiet. In the end , I used the paradigm employed by
"propdel" with the custom notification wrapper.
>> +/* Baton for use with svn_cl__check_externals_failed_notify_wrapper(). */
>> +struct svn_cl__check_externals_failed_notify_baton
>> +{
>> + void *wrapped_baton; /* The "real" notify_func2. */
>> + svn_wc_notify_func2_t wrapped_func; /* The "real" notify_func2 baton. */
>
> Those two are swapped relative to their comments.
Doh! Will fix.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1033320 - in /subversion/trunk/subversion:
include/svn_error_codes.h svn/cl.h svn/export-cmd.c svn/notify.c
svn/switch-cmd.c svn/update-cmd.c tests/cmdline/externals_tests.py
Posted by Julian Foad <ju...@wandisco.com>.
On Wed, 2010-11-10, cmpilato@apache.org wrote:
> Author: cmpilato
> Date: Wed Nov 10 01:44:35 2010
> New Revision: 1033320
>
> URL: http://svn.apache.org/viewvc?rev=1033320&view=rev
> Log:
> Fix issue #3622 ("svn should exit with exit code 1 if updating
> externals fails").
>
> * subversion/include/svn_error_codes.h
> (SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS): New error code.
>
> * subversion/svn/cl.h
> (struct svn_cl__check_externals_failed_notify_baton): New baton.
> (svn_cl__check_externals_failed_notify_wrapper): New function.
>
> * subversion/svn/notify.c
> (svn_cl__check_externals_failed_notify_wrapper): New function.
>
> * subversion/svn/update-cmd.c
> (svn_cl__update): Use the new wrapper baton and function to track
> externals processing failures, and return an error after all else
> is finished if such failures occured.
>
> * subversion/svn/export-cmd.c
> (svn_cl__export): Use the new wrapper baton and function to track
> externals processing failures, and return an error after all else
> is finished if such failures occured.
>
> * subversion/svn/switch-cmd.c
> (svn_cl__switch): Use the new wrapper baton and function to track
> externals processing failures, and return an error after all else
> is finished if such failures occured.
>
> * subversion/tests/cmdline/externals_tests.py
> (old_style_externals_ignore_peg_reg,
> can_place_file_external_into_dir_external): Change expected exit code.
Hi Mike. Useful fix.
I see that as well as reporting errors in externals, svn_wc_notify_t can
also report errors in locking:
/** Points to an error describing the reason for the failure when @c
* action is one of the following: #svn_wc_notify_failed_lock,
* #svn_wc_notify_failed_unlock, #svn_wc_notify_failed_external.
* Is @c NULL otherwise. */
svn_error_t *err;
Without investigating further, it seems to me that we will want to do
something similar for locking errors too. With this in mind, it might
be worth generalizing the wrapper stuff to "had an error" rather than
"had an externals error". (It can check nb->err != SVN_NO_ERROR rather
than nb->status==...)
I'm wondering about the wrapper function approach - did you feel it's
important to separate this error detection from the notification
function? I wonder if it would be simpler overall to get svn's existing
notifier code to track the presence of errors for you, instead. The
basic approach would be: add the flag
svn_boolean_t had_error;
to the private "struct notify_baton"; add a getter such as
svn_boolean_t svn_cl__notifier_had_error(void *baton);
that gets this flag; and make notify() do
if (n->err)
nb->had_error = TRUE;
Then the various subcommands would only need to call this function to
read the "had_error" indication. What do you think?
> Modified: subversion/trunk/subversion/include/svn_error_codes.h
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_error_codes.h (original)
> +++ subversion/trunk/subversion/include/svn_error_codes.h Wed Nov 10 01:44:35 2010
> @@ -1391,6 +1391,10 @@ SVN_ERROR_START
> SVN_ERR_CL_CATEGORY_START + 10,
> "No external merge tool available")
>
> + SVN_ERRDEF(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS,
> + SVN_ERR_CL_CATEGORY_START + 11,
> + "Failed processing one or more externals definitions")
> +
> /* malfunctions such as assertion failures */
>
> SVN_ERRDEF(SVN_ERR_ASSERTION_FAIL,
>
> Modified: subversion/trunk/subversion/svn/cl.h
> ==============================================================================
> --- subversion/trunk/subversion/svn/cl.h (original)
> +++ subversion/trunk/subversion/svn/cl.h Wed Nov 10 01:44:35 2010
> @@ -569,6 +569,21 @@ svn_cl__notifier_mark_checkout(void *bat
> svn_error_t *
> svn_cl__notifier_mark_export(void *baton);
>
> +/* Baton for use with svn_cl__check_externals_failed_notify_wrapper(). */
> +struct svn_cl__check_externals_failed_notify_baton
> +{
> + void *wrapped_baton; /* The "real" notify_func2. */
> + svn_wc_notify_func2_t wrapped_func; /* The "real" notify_func2 baton. */
Those two are swapped relative to their comments.
> + svn_boolean_t had_externals_error; /* Did something fail in an external? */
> +};
> +
> +/* Notification function wrapper (implements `svn_wc_notify_func2_t').
> + Use with an svn_cl__check_externals_failed_notify_baton BATON. */
> +void
> +svn_cl__check_externals_failed_notify_wrapper(void *baton,
> + const svn_wc_notify_t *n,
> + apr_pool_t *pool);
> +
> /* Print conflict stats accumulated in NOTIFY_BATON.
> * Return any error encountered during printing.
> * Do all allocations in POOL.*/
>
> Modified: subversion/trunk/subversion/svn/export-cmd.c
> ==============================================================================
> --- subversion/trunk/subversion/svn/export-cmd.c (original)
> +++ subversion/trunk/subversion/svn/export-cmd.c Wed Nov 10 01:44:35 2010
> @@ -52,6 +52,7 @@ svn_cl__export(apr_getopt_t *os,
> svn_error_t *err;
> svn_opt_revision_t peg_revision;
> const char *truefrom;
> + struct svn_cl__check_externals_failed_notify_baton nwb;
>
> SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
> opt_state->targets,
> @@ -95,6 +96,12 @@ svn_cl__export(apr_getopt_t *os,
> if (opt_state->depth == svn_depth_unknown)
> opt_state->depth = svn_depth_infinity;
>
> + nwb.wrapped_func = ctx->notify_func2;
> + nwb.wrapped_baton = ctx->notify_baton2;
> + nwb.had_externals_error = FALSE;
> + ctx->notify_func2 = svn_cl__check_externals_failed_notify_wrapper;
> + ctx->notify_baton2 = &nwb;
> +
> /* Do the export. */
> err = svn_client_export5(NULL, truefrom, to, &peg_revision,
> &(opt_state->start_revision),
> @@ -106,5 +113,10 @@ svn_cl__export(apr_getopt_t *os,
> _("Destination directory exists; please remove "
> "the directory or use --force to overwrite"));
>
> + if (nwb.had_externals_error)
> + return svn_error_create(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS, NULL,
> + _("Failure occured processing one or more "
> + "externals definitions"));
> +
> return svn_error_return(err);
> }
>
> Modified: subversion/trunk/subversion/svn/notify.c
> ==============================================================================
> --- subversion/trunk/subversion/svn/notify.c (original)
> +++ subversion/trunk/subversion/svn/notify.c Wed Nov 10 01:44:35 2010
> @@ -952,3 +952,18 @@ svn_cl__notifier_mark_export(void *baton
> nb->is_export = TRUE;
> return SVN_NO_ERROR;
> }
> +
> +void
> +svn_cl__check_externals_failed_notify_wrapper(void *baton,
> + const svn_wc_notify_t *n,
> + apr_pool_t *pool)
> +{
> + struct svn_cl__check_externals_failed_notify_baton *nwb = baton;
> +
> + if (n->action == svn_wc_notify_failed_external)
> + nwb->had_externals_error = TRUE;
> +
> + if (nwb->wrapped_func)
> + nwb->wrapped_func(nwb->wrapped_baton, n, pool);
> +}
All the rest looks fine.
- Julian
Re: svn commit: r1033320 - in /subversion/trunk/subversion:
include/svn_error_codes.h svn/cl.h svn/export-cmd.c svn/notify.c
svn/switch-cmd.c svn/update-cmd.c tests/cmdline/externals_tests.py
Posted by Julian Foad <ju...@wandisco.com>.
On Wed, 2010-11-10, cmpilato@apache.org wrote:
> Author: cmpilato
> Date: Wed Nov 10 01:44:35 2010
> New Revision: 1033320
>
> URL: http://svn.apache.org/viewvc?rev=1033320&view=rev
> Log:
> Fix issue #3622 ("svn should exit with exit code 1 if updating
> externals fails").
>
> * subversion/include/svn_error_codes.h
> (SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS): New error code.
>
> * subversion/svn/cl.h
> (struct svn_cl__check_externals_failed_notify_baton): New baton.
> (svn_cl__check_externals_failed_notify_wrapper): New function.
>
> * subversion/svn/notify.c
> (svn_cl__check_externals_failed_notify_wrapper): New function.
>
> * subversion/svn/update-cmd.c
> (svn_cl__update): Use the new wrapper baton and function to track
> externals processing failures, and return an error after all else
> is finished if such failures occured.
>
> * subversion/svn/export-cmd.c
> (svn_cl__export): Use the new wrapper baton and function to track
> externals processing failures, and return an error after all else
> is finished if such failures occured.
>
> * subversion/svn/switch-cmd.c
> (svn_cl__switch): Use the new wrapper baton and function to track
> externals processing failures, and return an error after all else
> is finished if such failures occured.
>
> * subversion/tests/cmdline/externals_tests.py
> (old_style_externals_ignore_peg_reg,
> can_place_file_external_into_dir_external): Change expected exit code.
Hi Mike. Useful fix.
I see that as well as reporting errors in externals, svn_wc_notify_t can
also report errors in locking:
/** Points to an error describing the reason for the failure when @c
* action is one of the following: #svn_wc_notify_failed_lock,
* #svn_wc_notify_failed_unlock, #svn_wc_notify_failed_external.
* Is @c NULL otherwise. */
svn_error_t *err;
Without investigating further, it seems to me that we will want to do
something similar for locking errors too. With this in mind, it might
be worth generalizing the wrapper stuff to "had an error" rather than
"had an externals error". (It can check nb->err != SVN_NO_ERROR rather
than nb->status==...)
I'm wondering about the wrapper function approach - did you feel it's
important to separate this error detection from the notification
function? I wonder if it would be simpler overall to get svn's existing
notifier code to track the presence of errors for you, instead. The
basic approach would be: add the flag
svn_boolean_t had_error;
to the private "struct notify_baton"; add a getter such as
svn_boolean_t svn_cl__notifier_had_error(void *baton);
that gets this flag; and make notify() do
if (n->err)
nb->had_error = TRUE;
Then the various subcommands would only need to call this function to
read the "had_error" indication. What do you think?
> Modified: subversion/trunk/subversion/include/svn_error_codes.h
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_error_codes.h (original)
> +++ subversion/trunk/subversion/include/svn_error_codes.h Wed Nov 10 01:44:35 2010
> @@ -1391,6 +1391,10 @@ SVN_ERROR_START
> SVN_ERR_CL_CATEGORY_START + 10,
> "No external merge tool available")
>
> + SVN_ERRDEF(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS,
> + SVN_ERR_CL_CATEGORY_START + 11,
> + "Failed processing one or more externals definitions")
> +
> /* malfunctions such as assertion failures */
>
> SVN_ERRDEF(SVN_ERR_ASSERTION_FAIL,
>
> Modified: subversion/trunk/subversion/svn/cl.h
> ==============================================================================
> --- subversion/trunk/subversion/svn/cl.h (original)
> +++ subversion/trunk/subversion/svn/cl.h Wed Nov 10 01:44:35 2010
> @@ -569,6 +569,21 @@ svn_cl__notifier_mark_checkout(void *bat
> svn_error_t *
> svn_cl__notifier_mark_export(void *baton);
>
> +/* Baton for use with svn_cl__check_externals_failed_notify_wrapper(). */
> +struct svn_cl__check_externals_failed_notify_baton
> +{
> + void *wrapped_baton; /* The "real" notify_func2. */
> + svn_wc_notify_func2_t wrapped_func; /* The "real" notify_func2 baton. */
Those two are swapped relative to their comments.
> + svn_boolean_t had_externals_error; /* Did something fail in an external? */
> +};
> +
> +/* Notification function wrapper (implements `svn_wc_notify_func2_t').
> + Use with an svn_cl__check_externals_failed_notify_baton BATON. */
> +void
> +svn_cl__check_externals_failed_notify_wrapper(void *baton,
> + const svn_wc_notify_t *n,
> + apr_pool_t *pool);
> +
> /* Print conflict stats accumulated in NOTIFY_BATON.
> * Return any error encountered during printing.
> * Do all allocations in POOL.*/
>
> Modified: subversion/trunk/subversion/svn/export-cmd.c
> ==============================================================================
> --- subversion/trunk/subversion/svn/export-cmd.c (original)
> +++ subversion/trunk/subversion/svn/export-cmd.c Wed Nov 10 01:44:35 2010
> @@ -52,6 +52,7 @@ svn_cl__export(apr_getopt_t *os,
> svn_error_t *err;
> svn_opt_revision_t peg_revision;
> const char *truefrom;
> + struct svn_cl__check_externals_failed_notify_baton nwb;
>
> SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
> opt_state->targets,
> @@ -95,6 +96,12 @@ svn_cl__export(apr_getopt_t *os,
> if (opt_state->depth == svn_depth_unknown)
> opt_state->depth = svn_depth_infinity;
>
> + nwb.wrapped_func = ctx->notify_func2;
> + nwb.wrapped_baton = ctx->notify_baton2;
> + nwb.had_externals_error = FALSE;
> + ctx->notify_func2 = svn_cl__check_externals_failed_notify_wrapper;
> + ctx->notify_baton2 = &nwb;
> +
> /* Do the export. */
> err = svn_client_export5(NULL, truefrom, to, &peg_revision,
> &(opt_state->start_revision),
> @@ -106,5 +113,10 @@ svn_cl__export(apr_getopt_t *os,
> _("Destination directory exists; please remove "
> "the directory or use --force to overwrite"));
>
> + if (nwb.had_externals_error)
> + return svn_error_create(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS, NULL,
> + _("Failure occured processing one or more "
> + "externals definitions"));
> +
> return svn_error_return(err);
> }
>
> Modified: subversion/trunk/subversion/svn/notify.c
> ==============================================================================
> --- subversion/trunk/subversion/svn/notify.c (original)
> +++ subversion/trunk/subversion/svn/notify.c Wed Nov 10 01:44:35 2010
> @@ -952,3 +952,18 @@ svn_cl__notifier_mark_export(void *baton
> nb->is_export = TRUE;
> return SVN_NO_ERROR;
> }
> +
> +void
> +svn_cl__check_externals_failed_notify_wrapper(void *baton,
> + const svn_wc_notify_t *n,
> + apr_pool_t *pool)
> +{
> + struct svn_cl__check_externals_failed_notify_baton *nwb = baton;
> +
> + if (n->action == svn_wc_notify_failed_external)
> + nwb->had_externals_error = TRUE;
> +
> + if (nwb->wrapped_func)
> + nwb->wrapped_func(nwb->wrapped_baton, n, pool);
> +}
All the rest looks fine.
- Julian
Re: svn commit: r1033320 - in /subversion/trunk/subversion: include/svn_error_codes.h
svn/cl.h svn/export-cmd.c svn/notify.c svn/switch-cmd.c svn/update-cmd.c
tests/cmdline/externals_tests.py
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/09/2010 10:30 PM, Noorul Islam K M wrote:
> I think I can have similar approach for issue 3713. If you are not
> already on it then I will try implementing the same for 3713.
Nope, I'm not "on it".
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: svn commit: r1033320 - in /subversion/trunk/subversion: include/svn_error_codes.h svn/cl.h svn/export-cmd.c svn/notify.c svn/switch-cmd.c svn/update-cmd.c tests/cmdline/externals_tests.py
Posted by Noorul Islam K M <no...@collab.net>.
cmpilato@apache.org writes:
> Author: cmpilato
> Date: Wed Nov 10 01:44:35 2010
> New Revision: 1033320
>
> URL: http://svn.apache.org/viewvc?rev=1033320&view=rev
> Log:
> Fix issue #3622 ("svn should exit with exit code 1 if updating
> externals fails").
>
> * subversion/include/svn_error_codes.h
> (SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS): New error code.
>
> * subversion/svn/cl.h
> (struct svn_cl__check_externals_failed_notify_baton): New baton.
> (svn_cl__check_externals_failed_notify_wrapper): New function.
>
> * subversion/svn/notify.c
> (svn_cl__check_externals_failed_notify_wrapper): New function.
>
> * subversion/svn/update-cmd.c
> (svn_cl__update): Use the new wrapper baton and function to track
> externals processing failures, and return an error after all else
> is finished if such failures occured.
>
> * subversion/svn/export-cmd.c
> (svn_cl__export): Use the new wrapper baton and function to track
> externals processing failures, and return an error after all else
> is finished if such failures occured.
>
> * subversion/svn/switch-cmd.c
> (svn_cl__switch): Use the new wrapper baton and function to track
> externals processing failures, and return an error after all else
> is finished if such failures occured.
>
> * subversion/tests/cmdline/externals_tests.py
> (old_style_externals_ignore_peg_reg,
> can_place_file_external_into_dir_external): Change expected exit code.
>
> Modified:
> subversion/trunk/subversion/include/svn_error_codes.h
> subversion/trunk/subversion/svn/cl.h
> subversion/trunk/subversion/svn/export-cmd.c
> subversion/trunk/subversion/svn/notify.c
> subversion/trunk/subversion/svn/switch-cmd.c
> subversion/trunk/subversion/svn/update-cmd.c
> subversion/trunk/subversion/tests/cmdline/externals_tests.py
>
I think I can have similar approach for issue 3713. If you are not
already on it then I will try implementing the same for 3713.
Thanks and Regards
Noorul