[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r32790 - in trunk/subversion: libsvn_wc tests/cmdline

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Thu, 28 Aug 2008 12:02:49 -0400

danielsh_at_tigris.org writes:
> Log:
> Fix issue #2505: make switch continue after deleting locally modified
> directories, as it update and merge do.
>
> Patch by: Danil Shopyrin <danil.shopyrin_at_gmail.com>
> (Log message tweaked by me.)
>
> * subversion/libsvn_wc/update_editor.c
> (leftmod_error_chain): Trap and ignore SVN_ERR_WC_LEFT_LOCAL_MOD, fixing
> a potential error leak. Don't delete logfile in that case.
> (do_entry_deletion): Call svn_wc_remove_from_revision_control()
> with instant_error = FALSE.
>
> * subversion/tests/cmdline/switch_tests.py
> (tolerate_local_mods): New test.
> (test_list): Run the new test.

Sorry not to have offered the review below before the patch was
applied -- I didn't get a chance to look until now.

> --- trunk/subversion/libsvn_wc/update_editor.c
> +++ trunk/subversion/libsvn_wc/update_editor.c
> @@ -1092,19 +1092,16 @@ leftmod_error_chain(svn_error_t *err,
> if (tmp_err->apr_err == SVN_ERR_WC_LEFT_LOCAL_MOD)
> break;
>
> - /* If we found a "left a local mod" error, wrap and return it.
> - Otherwise, we just return our top-most error. */
> + /* If we found a "left a local mod" error, tolerate it
> + and clear the whole error. In that case we continue with
> + modified files left on the disk. */
> if (tmp_err)
> {
> - /* Remove the LOGFILE (and eat up errors from this process). */
> - svn_error_clear(svn_io_remove_file(logfile, pool));
> -
> - return svn_error_createf
> - (SVN_ERR_WC_OBSTRUCTED_UPDATE, tmp_err,
> - _("Won't delete locally modified directory '%s'"),
> - svn_path_local_style(path, pool));
> + svn_error_clear(err);
> + return SVN_NO_ERROR;
> }
>
> + /* Otherwise, we just return our top-most error. */
> return err;
> }

The doc string for leftmod_error_chain() should explain that it can
result in the 'err' argument being cleared.

But also, 'err' is just the top of a possibly multi-error chain. Here
we're clear 'err', but not any of the other down errors (which could be
before or after 'err' in the chain). Shouldn't we clear everyone in the
chain?

See the definition of svn_error_clear(), by the way, which behaves very
differently when SVN_DEBUG is defined, for reasons I don't understand.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-28 18:03:02 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.