Thanks, Kamesh.
Questions for the other Subversion developers:
1. Do we really want this to throw an error? People may have scripts that rely
on it succeeding.
2. Where are we going to write down our policy on whether a command like this
should throw an error or a warning? It's obvious that we need to, since the
question crops up over and over again.
Kamesh Jayachandran wrote:
>
> [[[
> Fix issue #2440, svn rm nonexistent returns success.
>
> * subversion/libsvn_wc/adm_ops.c
> (erase_unversioned_from_wc): Return an error in the case where the
> file is not found.
>
>
> * Fix tests
> subversion/tests/svn/basic_tests.py
> (basic_delete): Deleting non-existant unversioned item returns the error so expecting the error.
> subversion/tests/svn/schedule_tests.py
> (unschedule_missing_added):svntest.main.run_svn(None, 'rm', file1_path, dir1_path) fails while deleting file1_path and stops immedeately not trying to delete the dir1_path because of the fix, so seperating the deletion to 2 operations.
>
> ]]]
> Index: subversion/libsvn_wc/adm_ops.c
> ===================================================================
> --- subversion/libsvn_wc/adm_ops.c (revision 17664)
> +++ subversion/libsvn_wc/adm_ops.c (working copy)
> @@ -667,7 +667,8 @@
> switch (kind)
> {
> case svn_node_none:
> - /* Nothing to do. */
> + return svn_error_createf (SVN_ERR_WC_NOT_FILE, NULL,
> + _("'%s' not existing in the working copy and repository"), path);
> break;
>
> default:
> Index: subversion/tests/svn/basic_tests.py
> ===================================================================
> --- subversion/tests/svn/basic_tests.py (revision 17664)
> +++ subversion/tests/svn/basic_tests.py (working copy)
> @@ -1093,7 +1093,7 @@
> verify_file_deleted("Failed to remove unversioned file foo", foo_path)
>
> # Deleting non-existant unversioned item
> - svntest.actions.run_and_verify_svn(None, None, [],
> + svntest.actions.run_and_verify_svn(None, None, SVNAnyOutput,
> 'rm', '--force', foo_path)
What's the point of that action? It no longer tests anything, really. Might
as well remove it.
>
> # At one stage deleting an URL dumped core
> Index: subversion/tests/svn/schedule_tests.py
> ===================================================================
> --- subversion/tests/svn/schedule_tests.py (revision 17664)
> +++ subversion/tests/svn/schedule_tests.py (working copy)
> @@ -484,7 +484,8 @@
> svntest.main.safe_rmtree(dir2_path)
>
> # Unschedule the additions, using 'svn rm' and 'svn revert'.
> - svntest.main.run_svn(None, 'rm', file1_path, dir1_path)
> + svntest.main.run_svn(SVNAnyOutput, 'rm', file1_path)
> + svntest.main.run_svn(SVNAnyOutput, 'rm', dir1_path)
> svntest.main.run_svn(None, 'revert', file2_path, dir2_path)
That seems to contradict the comment at the top of the test that says we should
be able to do this with no errors. Maybe the comment is now deemed wrong ...
or something is.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Dec 12 17:34:41 2005