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

Re: [PATCH]#2440 svn rm nonexistent exits with success

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: 2005-12-12 07:42:57 CET

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

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