Thanks Karl.
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
svn:log.
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 <kamesh@collab.net>
* subversion/libsvn_wc/adm_ops.c
(erase_unversioned_from_wc): Return an error in the case where the
file is not found.
* subversion/tests/*cmdline*/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/*cmdline*/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.
]]
With regards
Kamesh Jayachandran
kfogel@collab.net wrote:
> Kamesh Jayachandran <kamesh@collab.net> 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
> time.
>
>
>> 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));
>> break;
>>
>
> 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
> those.
>
>
>> default:
>> 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.
>
> Thanks,
> -Karl
>
>
Received on Fri Jan 6 08:53:52 2006