Kamesh Jayachandran <firstname.lastname@example.org> 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.
www.collab.net <> CollabNet | Distributed Development On Demand
To unsubscribe, e-mail: email@example.com
For additional commands, e-mail: firstname.lastname@example.org
Received on Fri Jan 6 01:30:24 2006