You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2010/08/10 15:46:57 UTC
svn commit: r984007 - /subversion/trunk/subversion/libsvn_wc/update_editor.c
Author: rhuijben
Date: Tue Aug 10 13:46:57 2010
New Revision: 984007
URL: http://svn.apache.org/viewvc?rev=984007&view=rev
Log:
* subversion/libsvn_wc/update_editor.c
(cleanup_dir_baton): When calling wq_run() from pool cleanup, don't pass
a cancel func as this will also be called on canceling the operation.
And in that case you don't want to cancel running the wq items.
Modified:
subversion/trunk/subversion/libsvn_wc/update_editor.c
Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=984007&r1=984006&r2=984007&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
+++ subversion/trunk/subversion/libsvn_wc/update_editor.c Tue Aug 10 13:46:57 2010
@@ -496,7 +496,7 @@ cleanup_dir_baton(void *dir_baton)
apr_pool_t *pool = apr_pool_parent_get(db->pool);
err = svn_wc__wq_run(eb->db, db->local_abspath,
- eb->cancel_func, eb->cancel_baton,
+ NULL /* cancel_func */, NULL /* cancel_baton */,
pool);
if (err)
Re: svn commit: r984007 -
/subversion/trunk/subversion/libsvn_wc/update_editor.c
Posted by Julian Foad <ju...@wandisco.com>.
On Tue, 2010-08-10, rhuijben@apache.org wrote:
> Author: rhuijben
> Date: Tue Aug 10 13:46:57 2010
> New Revision: 984007
>
> URL: http://svn.apache.org/viewvc?rev=984007&view=rev
> Log:
> * subversion/libsvn_wc/update_editor.c
> (cleanup_dir_baton): When calling wq_run() from pool cleanup, don't pass
> a cancel func as this will also be called on canceling the operation.
> And in that case you don't want to cancel running the wq items.
This needs a comment in the code!
- Julian
> Modified:
> subversion/trunk/subversion/libsvn_wc/update_editor.c
>
> Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=984007&r1=984006&r2=984007&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/update_editor.c Tue Aug 10 13:46:57 2010
> @@ -496,7 +496,7 @@ cleanup_dir_baton(void *dir_baton)
> apr_pool_t *pool = apr_pool_parent_get(db->pool);
>
> err = svn_wc__wq_run(eb->db, db->local_abspath,
> - eb->cancel_func, eb->cancel_baton,
> + NULL /* cancel_func */, NULL /* cancel_baton */,
> pool);
>
> if (err)
>
>
Re: svn commit: r984007 - /subversion/trunk/subversion/libsvn_wc/update_editor.c
Posted by Greg Stein <gs...@gmail.com>.
On Wed, Aug 11, 2010 at 15:47, Bert Huijben <be...@vmoo.com> wrote:
>> -----Original Message-----
>> From: Greg Stein [mailto:gstein@gmail.com]
>> Sent: woensdag 11 augustus 2010 21:30
>> To: dev@subversion.apache.org
>> Subject: Re: svn commit: r984007 -
>> /subversion/trunk/subversion/libsvn_wc/update_editor.c
>>
>> I think you *always* want wq operations to be cancelable. That simply
>> means that wq items will be left behind and "svn cleanup" will fix it.
>> I see no problem with that.
>
> The problem here is that once you press ^C all future calls to the global
> cancel handler (in svn and other code) return SVN_ERR_CANCELED..
>
> So when you press ^C during update, none of the wq operations are handled,
> while before we introduced the WQ all loggy operations were just handled.
>
> So pressing ^C likely locks your working copy in 1.7 (requiring cleanup),
> while older versions did not.
> (I found this issue when I added some additional tracing to detect where in
> ra_serf we leaked these cancel errors)
That seems okay to me.
Remember that the cancellation func/baton is also passed (in wq) to
things like svn_stream_copy(). That allows for stopping a 5G
copy/translation and resuming it later. (and imagine if one/both of
the targets is on a network drive!)
Removing the callback means the client will be less responsive and may
get locked into a very slow, very long operation.
We've started passing cancel func/baton through a lot more places in
wc, compared to 1.6. That gives us a much more responsive client. I'd
like to see that continue, even if it could mean a locked working
copy. People should just normally expect ^C to leave a working copy in
a locked state. I see no issue with that.
Cheers,
-g
RE: svn commit: r984007 - /subversion/trunk/subversion/libsvn_wc/update_editor.c
Posted by Bert Huijben <be...@vmoo.com>.
> -----Original Message-----
> From: Greg Stein [mailto:gstein@gmail.com]
> Sent: woensdag 11 augustus 2010 21:30
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r984007 -
> /subversion/trunk/subversion/libsvn_wc/update_editor.c
>
> I think you *always* want wq operations to be cancelable. That simply
> means that wq items will be left behind and "svn cleanup" will fix it.
> I see no problem with that.
The problem here is that once you press ^C all future calls to the global
cancel handler (in svn and other code) return SVN_ERR_CANCELED..
So when you press ^C during update, none of the wq operations are handled,
while before we introduced the WQ all loggy operations were just handled.
So pressing ^C likely locks your working copy in 1.7 (requiring cleanup),
while older versions did not.
(I found this issue when I added some additional tracing to detect where in
ra_serf we leaked these cancel errors)
Bert
Re: svn commit: r984007 - /subversion/trunk/subversion/libsvn_wc/update_editor.c
Posted by Greg Stein <gs...@gmail.com>.
I think you *always* want wq operations to be cancelable. That simply
means that wq items will be left behind and "svn cleanup" will fix it.
I see no problem with that.
Cheers,
-g
On Tue, Aug 10, 2010 at 09:46, <rh...@apache.org> wrote:
> Author: rhuijben
> Date: Tue Aug 10 13:46:57 2010
> New Revision: 984007
>
> URL: http://svn.apache.org/viewvc?rev=984007&view=rev
> Log:
> * subversion/libsvn_wc/update_editor.c
> (cleanup_dir_baton): When calling wq_run() from pool cleanup, don't pass
> a cancel func as this will also be called on canceling the operation.
> And in that case you don't want to cancel running the wq items.
>
> Modified:
> subversion/trunk/subversion/libsvn_wc/update_editor.c
>
> Modified: subversion/trunk/subversion/libsvn_wc/update_editor.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/update_editor.c?rev=984007&r1=984006&r2=984007&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_wc/update_editor.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/update_editor.c Tue Aug 10 13:46:57 2010
> @@ -496,7 +496,7 @@ cleanup_dir_baton(void *dir_baton)
> apr_pool_t *pool = apr_pool_parent_get(db->pool);
>
> err = svn_wc__wq_run(eb->db, db->local_abspath,
> - eb->cancel_func, eb->cancel_baton,
> + NULL /* cancel_func */, NULL /* cancel_baton */,
> pool);
>
> if (err)
>
>
>