Thanks for the patch for issue #1806, Ramaswamy. Here's a review:
> Fix Issue #1806 -- 'svn up' stops when it doesn't have to
>
> *subversion/libsvn_wc/log.c
> (log_do_delete_entry): Change 'instant_error' to TRUE to allow
> unversioning of files/dirs even if there are local modifications.
> Trap error SVN_ERR_WC_LEFT_LOCAL_MOD so that updates can continue.
> *subversion/libsvn_wc/update_editor.c
> (do_entry_deletion): Remove section that does not allow updates to
> proceed if there are local modifications.
Minor nit: we usually put a space after the asterisks.
Major nit: the log message says "Change 'instant_error' to TRUE", but
this patch actually changes it to FALSE :-).
> Index: subversion/libsvn_wc/log.c
> ===================================================================
> --- subversion/libsvn_wc/log.c (revision 9295)
> +++ subversion/libsvn_wc/log.c (working copy)
> @@ -601,7 +601,7 @@
> err = svn_wc_remove_from_revision_control (adm_access,
> SVN_WC_ENTRY_THIS_DIR,
> TRUE, /* destroy */
> - TRUE, /* instant_error */
> + FALSE, /* instant_error */
> NULL, NULL,
> loggy->pool);
> }
> @@ -610,12 +610,17 @@
> {
> err = svn_wc_remove_from_revision_control (loggy->adm_access, name,
> TRUE, /* destroy */
> - TRUE, /* instant_error */
> + FALSE, /* instant_error */
> NULL, NULL,
> loggy->pool);
> }
>
> - return err;
> + if ((err) && (err->apr_err == SVN_ERR_WC_LEFT_LOCAL_MOD))
> + {
> + return SVN_NO_ERROR;
> + }
> + else
> + return err;
> }
A comment explaining why we trap the error would probably be good.
> /* Note: assuming that svn_wc__log_commit() is what created all of
> Index: subversion/libsvn_wc/update_editor.c
> ===================================================================
> --- subversion/libsvn_wc/update_editor.c (revision 9295)
> +++ subversion/libsvn_wc/update_editor.c (working copy)
> @@ -809,23 +809,6 @@
> logfile_path = svn_wc__adm_path (parent_path, FALSE, pool,
> SVN_WC__ADM_LOG, NULL);
>
> - /* If trying to delete a locally-modified file, throw an 'obstructed
> - update' error. */
> - if (kind == svn_node_file)
> - {
> - svn_boolean_t tmodified_p, pmodified_p;
> - SVN_ERR (svn_wc_text_modified_p (&tmodified_p, full_path, FALSE,
> - adm_access, pool));
> - SVN_ERR (svn_wc_props_modified_p (&pmodified_p, full_path,
> - adm_access, pool));
> -
> - if (tmodified_p || pmodified_p)
> - return svn_error_createf
> - (SVN_ERR_WC_OBSTRUCTED_UPDATE, NULL,
> - "Won't delete locally modified file '%s'",
> - path);
> - }
> -
> SVN_ERR (svn_wc__open_adm_file (&log_fp,
> parent_path,
> SVN_WC__ADM_LOG,
Yup, looks right to me (and Ben).
In the issue, you ask:
> Also removing the section from update_editor.c that checks for local
> file modifications results in regression test failure (update tests)
>
> Any suggestions ?
Yup -- change the test to expect the new behavior :-). Basically,
update_tests.py 7 tests that we get an 'obstructed update' error in
this circumstance. Now we're changing (fixing) Subversion so we won't
get an error anymore. Thus, the test should be changed to expect no
error now.
I'm doing this right now. If I have time to run the tests afterwards,
I'll just commit your patch + my tweaks to the regression test. If
not, I'll attach the whole thing to the issue so you can review it.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Apr 12 23:49:00 2004