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

Re: [PATCH] tolerate locally modified dirs on switch

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Sun, 17 Aug 2008 19:04:48 +0300 (Jerusalem Daylight Time)

(This is issue #2505, as noted elsewhere in the thread.)

Danil Shopyrin wrote on Thu, 14 Aug 2008 at 14:58 +0400:
> Hi all!
>
> There is an annoying behavior in the switch command when there are
> locally modified directories to delete. Use case is as follows:
> * user creates a development branch to do some experiments;
> * new folders are created and added to Subversion in the development
> branch. There are some unversioned (and ignored) items in
> the added folders (intermediate compiler output files, for example);
> * and when user wants to switch back to the trunk he/she gets the
> following error from the switch command:
> 'Won't delete locally modified directory';
> * working copy becomes broken and it is needed to perform long and
> nondeterministic (for me, at least) sequence of svn cleanup, svn up
> svn switch commands to heal the working copy.
>
> The problem can be reproduced by the following script:
> [[
> set REPOS_DIR=C:\test\db
> set REPOS=file:///C:/test/db
> set WC=C:\test\wc
>
> rd /q /s %REPOS_DIR%
> rd /q /s %WC%
>
> svnadmin create %REPOS_DIR%
> svn mkdir %REPOS%/branch1 -m ""
> svn cp %REPOS%/branch1 %REPOS%/branch2 -m ""
>
> svn co %REPOS%/branch1 %WC%
> mkdir %WC%\dir
> svn add %WC%\dir
> svn ci -m "r1" %WC%
>
> echo file >%WC%\dir\file
>
> svn sw %REPOS%/branch2 %WC%
> ]]
>
> The better behavior is to leave locally modified directory unversioned
> just like svn update command does. Here is a patch.
>
> Please note that the patch doesn't introduce really new behavior. It
> just makes switch command behavior more consistent with
> other commands behavior.
>
> The log message is as follows:
> [[[
> Make switch continue after deleting locally modified directories, as
> it update and merge do.
>
> * subversion/libsvn_wc/update_editor:
> (leftmod_error_chain): Remove unused helper.
> (do_entry_deletion): Ignore SVN_ERR_WC_LEFT_LOCAL_MOD error as it
> log.c:log_do_delete_entry() does.
>

Nits: s/it//, and (symbol_name)s should be indented two spaces, not one.

> * subversion/tests/cmdline/switch_tests.py:
> (tollerate_local_mods): New test.
> (test_list): Run the new test.

s/tollerate/tolerate/

> ]]]

> Index: subversion/libsvn_wc/update_editor.c
> ===================================================================
> --- subversion/libsvn_wc/update_editor.c (revision 32393)
> +++ subversion/libsvn_wc/update_editor.c (working copy)
> @@ -1061,47 +1061,7 @@
> }
>
>
> -/* Helper for delete_entry().
> -
> - Search an error chain (ERR) for evidence that a local mod was left.
> - If so, cleanup LOGFILE and return an appropriate error. Otherwise,
> - just return the original error chain.
> -*/
> static svn_error_t *
> -leftmod_error_chain(svn_error_t *err,
> - const char *logfile,
> - const char *path,
> - apr_pool_t *pool)
> -{
> - svn_error_t *tmp_err;
> -
> - if (! err)
> - return SVN_NO_ERROR;
> -
> - /* Advance TMP_ERR to the part of the error chain that reveals that
> - a local mod was left, or to the NULL end of the chain. */
> - for (tmp_err = err; tmp_err; tmp_err = tmp_err->child)
> - 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 (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,

(not related to the patch)
Doesn't this code leak the parent error of tmp_err?

> - _("Won't delete locally modified directory '%s'"),
> - svn_path_local_style(path, pool));
> - }
> -
> - return err;
> -}
> -
> -
> -static svn_error_t *
> do_entry_deletion(struct edit_baton *eb,
> const char *parent_path,
> const char *path,
> @@ -1163,6 +1123,7 @@
>
> if (entry->kind == svn_node_dir)
> {
> + svn_error_t *err;
> svn_wc_adm_access_t *child_access;
> const char *logfile_path
> = svn_wc__adm_path(parent_path, FALSE, pool,
> @@ -1172,16 +1133,25 @@
> (&child_access, eb->adm_access,
> full_path, pool));
>
> - SVN_ERR(leftmod_error_chain
> - (svn_wc_remove_from_revision_control
> - (child_access,
> - SVN_WC_ENTRY_THIS_DIR,
> - TRUE, /* destroy */
> - TRUE, /* instant error */
> - eb->cancel_func,
> - eb->cancel_baton,
> - pool),
> - logfile_path, parent_path, pool));
> + err = svn_wc_remove_from_revision_control
> + (child_access,
> + SVN_WC_ENTRY_THIS_DIR,
> + TRUE, /* destroy */
> + FALSE, /* instant error */
> + eb->cancel_func,
> + eb->cancel_baton,
> + pool);
> +
> + if (err && err->apr_err == SVN_ERR_WC_LEFT_LOCAL_MOD)
> + {
> + svn_error_clear(err);
> + }
> + else if (err)
> + {
> + /* Remove the LOGFILE (and eat up errors from this process). */
> + svn_error_clear(svn_io_remove_file(logfile_path, pool));

This has two differences from the previous code:

* This code removes the logfile when the error is *not*
  SVN_ERR_WC_LEFT_LOCAL_MOD, the existing code removes the logfile when
  the error *is* SVN_ERR_WC_LEFT_LOCAL_MOD.

* The existing code checks for SVN_ERR_WC_LEFT_LOCAL_MOD anywhere in the
  error chain, not just as the top-most error.
  
Any reason for these two changes? I expected -- and the log message
says -- the only change to be the handling of LEFT_LOCAL_MOD error
(ignoring it or not).

Thanks for the patch :)

Daniel

> + return err;
> + }
> }
> }
>
> Index: subversion/tests/cmdline/switch_tests.py
> ===================================================================
> --- subversion/tests/cmdline/switch_tests.py (revision 32393)
> +++ subversion/tests/cmdline/switch_tests.py (working copy)
> @@ -2108,6 +2108,53 @@
> expected_disk,
> expected_status)
>
> +#----------------------------------------------------------------------
> +# Make sure that switch continue after deleting locally modified
> +# directories, as it update and merge do.
> +
> +def tollerate_local_mods(sbox):
> + "tollerate deletion of a directory with local mods"
> +
> + sbox.build()
> + wc_dir = sbox.wc_dir
> +
> + A_path = os.path.join(wc_dir, 'A')
> + L_path = os.path.join(A_path, 'L')
> + LM_path = os.path.join(L_path, 'local_mod')
> + A_url = sbox.repo_url + '/A'
> + A2_url = sbox.repo_url + '/A2'
> +
> + svntest.actions.run_and_verify_svn(None,
> + ['\n', 'Committed revision 2.\n'], [],
> + 'cp', '-m', 'make copy', A_url, A2_url)
> +
> + os.mkdir(L_path)
> + svntest.main.run_svn(None, 'add', L_path)
> + svntest.main.run_svn(None, 'ci', '-m', 'Commit added folder', wc_dir)
> +
> + # locally modified unversioned file
> + svntest.main.file_write(LM_path, 'Locally modified file.\n', 'w+')
> +
> + expected_output = svntest.wc.State(wc_dir, {
> + 'A/L' : Item(status='D '),
> + })
> +
> + expected_disk = svntest.main.greek_state.copy()
> + expected_disk.add({
> + 'A/L' : Item(),
> + 'A/L/local_mod' : Item(contents='Locally modified file.\n'),
> + })
> +
> + expected_status = svntest.actions.get_virginal_state(wc_dir, 3)
> + expected_status.tweak('', 'iota', wc_rev=1)
> + expected_status.tweak('A', switched='S')
> +
> + # Used to fail with locally modified or unversioned files
> + svntest.actions.run_and_verify_switch(wc_dir, A_path, A2_url,
> + expected_output,
> + expected_disk,
> + expected_status)
> +
> ########################################################################
> # Run the tests
>
> @@ -2142,6 +2189,7 @@
> switch_urls_with_spaces,
> switch_to_dir_with_peg_rev2,
> switch_to_root,
> + tollerate_local_mods,
> ]
>
> if __name__ == '__main__':

---------------------------------------------------------------------
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-17 18:06:16 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.