One small issue I have with the log message is that it still says the
changes against the old path not the new ones.
I am not sure how can I give the patch for the property changes like
I feel the following log message will be the correct one.
Fix issue #2440 'svn rm nonexistent-item' wasn't erroring.
Patch by: Kamesh Jayachandran <firstname.lastname@example.org>
(erase_unversioned_from_wc): Return an error in the case where the
file is not found.
(basic_delete): Deleting non-existant unversioned item returns the error
so moving the testcase to subversion/tests/svn/schedule_tests.py.
(unschedule_missing_added): Deleting set of working copy files,
fails on first unversioned file and subsequent files are not
deleted at all, so seperating the deletion to 2 delete operations.
(delete_non_existent): New test, for issue #2440.
(test_list): Run the new test.
> Kamesh Jayachandran <email@example.com> writes:
>> 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
>> moving the testcase to subversion/tests/svn/schedule_tests.py.
>> * subversion/tests/svn/schedule_tests.py
>> (unschedule_missing_added): Deleting set of working copy files,
>> fails on first unversioned file and subsequent files are not deleted
>> at all,
>> so seperating the deletion to 2 delete operations.
>> (delete_non_existent): Regression test for #2440
> Looks good to me. Watch out for slightly inconsistent formatting, and
> remember to mention the change to 'test_list' in schedule_tests.py. I
> took care of these issues when committing, just mentioning for next
>> Index: libsvn_wc/adm_ops.c
>> --- libsvn_wc/adm_ops.c (revision 17664)
>> +++ libsvn_wc/adm_ops.c (working copy)
> Ooops, the patch should apply from top of trunk, that is, that should
> be 'subversion/libsvn_wc/adm_ops.c' above. Same for the other files.
> No big deal, I adjusted it. Just letting you know for next time.
>> @@ -667,7 +667,8 @@
>> switch (kind)
>> case svn_node_none:
>> - /* Nothing to do. */
>> + return svn_error_createf (SVN_ERR_BAD_FILENAME, NULL,
>> + _("'%s' does not exist"), svn_path_local_style(path, pool));
> The code looks fine. The indentation is off, and there should be a
> space before the paren in the svn_path_local_style call. I fixed
>> Index: tests/svn/basic_tests.py
>> --- tests/svn/basic_tests.py (revision 17664)
>> +++ tests/svn/basic_tests.py (working copy)
>> @@ -1092,10 +1092,6 @@
>> 'rm', '--force', foo_path)
>> verify_file_deleted("Failed to remove unversioned file foo", foo_path)
>> - # Deleting non-existant unversioned item
>> - svntest.actions.run_and_verify_svn(None, None, ,
>> - 'rm', '--force', foo_path)
>> # At one stage deleting an URL dumped core
>> iota_URL = svntest.main.current_repo_url + '/iota'
> Heh, r17860 changed it to "non-existent", making the patch not apply.
> I didn't think of that when I applied all those spelling changes. Not
> your fault, of course.
> Rest looks great -- applied in r17993.
Received on Fri Jan 6 08:53:52 2006