You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by David James <ja...@cs.toronto.edu> on 2007/09/13 00:13:19 UTC
Re: svn commit: r26561 - trunk/subversion/libsvn_wc
On 9/12/07, dionisos@tigris.org <di...@tigris.org> wrote:
> + /* Then maybe it was a directory? */
> + svn_error_clear(err);
> if (cancel_func)
> SVN_ERR(cancel_func(cancel_baton));
>
> - SVN_ERR(svn_io_remove_dir2(path, FALSE, pool));
> + err = svn_io_remove_dir2(path, FALSE, pool);
>
> - if (cancel_func)
> - SVN_ERR(cancel_func(cancel_baton));
> + if (err)
> + {
> + /* We're unlikely to end up here. But we need this fallback
> + to make sure we report the right error *and* try the
> + correct deletion at least once. */
> + svn_node_kind_t kind;
>
> - break;
> + svn_error_clear(err);
> + SVN_ERR(svn_io_check_path(path, &kind, pool));
> + if (kind == svn_node_file)
> + SVN_ERR(svn_io_remove_file(path, pool));
> + else if (kind == svn_node_dir)
> + {
> + if (cancel_func)
> + SVN_ERR(cancel_func(cancel_baton));
It looks like you changed the cancel func semantics slightly here. Was
this intentional?
Previously, we used the following algorithm:
- call the cancel func
- delete the directory
- if the directory was deleted successfully, call the cancel func
again (presumably to check if the user pressed the cancel button while
we were off deleting a big directory of files)
Now, we use the following algorithm instead:
- call the cancel func
- delete the directory
- if the directory was NOT deleted successfully, call the cancel func again
Cheers,
David
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r26561 - trunk/subversion/libsvn_wc
Posted by Erik Huelsmann <eh...@gmail.com>.
On 9/13/07, David James <ja...@cs.toronto.edu> wrote:
> On 9/12/07, dionisos@tigris.org <di...@tigris.org> wrote:
> > + /* Then maybe it was a directory? */
> > + svn_error_clear(err);
> > if (cancel_func)
> > SVN_ERR(cancel_func(cancel_baton));
> >
> > - SVN_ERR(svn_io_remove_dir2(path, FALSE, pool));
> > + err = svn_io_remove_dir2(path, FALSE, pool);
> >
> > - if (cancel_func)
> > - SVN_ERR(cancel_func(cancel_baton));
> > + if (err)
> > + {
> > + /* We're unlikely to end up here. But we need this fallback
> > + to make sure we report the right error *and* try the
> > + correct deletion at least once. */
> > + svn_node_kind_t kind;
> >
> > - break;
> > + svn_error_clear(err);
> > + SVN_ERR(svn_io_check_path(path, &kind, pool));
> > + if (kind == svn_node_file)
> > + SVN_ERR(svn_io_remove_file(path, pool));
> > + else if (kind == svn_node_dir)
> > + {
> > + if (cancel_func)
> > + SVN_ERR(cancel_func(cancel_baton));
>
> It looks like you changed the cancel func semantics slightly here. Was
> this intentional?
>
> Previously, we used the following algorithm:
> - call the cancel func
> - delete the directory
> - if the directory was deleted successfully, call the cancel func
> again (presumably to check if the user pressed the cancel button while
> we were off deleting a big directory of files)
>
> Now, we use the following algorithm instead:
> - call the cancel func
> - delete the directory
> - if the directory was NOT deleted successfully, call the cancel func again
Right. No, that was not intentional. But, having a look at it, I think
it's better to pass the cancel func into svn_io_remove_dir() anyway. I
also see this function is new in 1.5, so it's not a reason to an
interface rev. All in all I think it's a good moment to allow this
long-running function to start to be cancelable (especially on Windows
with its retry loop, this function can potentially take a **loooong**
time to complete)
Thanks for the heads up.
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org