Hi Garrett,
Thanks for your feedback.
When ran through *make check/make davautocheck *found 2 existing tests failing related to deleting a file failed, found and fixed those testcases to work fine with the new fix.
[[[
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.
]]]
With regards
Kamesh Jayachandran
Garrett Rooney wrote:
> On 12/9/05, Kamesh Jayachandran <kamesh@collab.net> wrote:
>
>> Hi All,
>> [[[
>> returning the error in case file does not exist in the working copy.
>> ]]]
>>
>
> Kamesh,
>
> This patch seems pretty reasonable, but the log message is not
> sufficient. Please look at
> http://subversion.tigris.org/hacking.html#log-messages for a
> description of the format the Subversion project uses for it's log
> messages. While your message does describe what the change does, it
> doesn't mention what files and functions were changed, and it doesn't
> mention the issue you're fixing, both of which are important
> information.
>
> A log message for this kind of change should look something like this:
>
> [[[
> 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.
> ]]]
>
> Also, I'm always kind of skeptical about changing low level stuff in
> libsvn_wc because there are often unexpected side effects. Did you
> run the test suite after making this change? Even if you can't add a
> test for this specific problem we should still verify that it doesn't
> break any existing tests.
>
> -garrett
>
>
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)
# 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)
# 'svn st' should now show absolutely zero local mods.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Dec 12 07:42:57 2005