On Tue, Jan 09, 2007 at 02:20:48AM -0800, zhakov@tigris.org wrote:
> Add option "--keep-local" to svn rm command to keep deleted path in
> current working copy.
>
Nice! I made a followup commit in r22963 to clarify some of the
comments (and document the new attribute in the README file in
libsvn_wc). I've got a few comments on the implementation, see below:
> --- trunk/subversion/libsvn_wc/adm_ops.c (original)
> +++ trunk/subversion/libsvn_wc/adm_ops.c Tue Jan 9 02:20:46 2007
> @@ -1095,12 +1114,13 @@
>
>
> svn_error_t *
> -svn_wc_delete2(const char *path,
> +svn_wc_delete3(const char *path,
> svn_wc_adm_access_t *adm_access,
> svn_cancel_func_t cancel_func,
> void *cancel_baton,
> svn_wc_notify_func2_t notify_func,
> void *notify_baton,
> + svn_boolean_t keep_local,
> apr_pool_t *pool)
> {
> svn_wc_adm_access_t *dir_access;
> @@ -1175,8 +1195,10 @@
> if (dir_access != adm_access)
> {
> /* Recursively mark a whole tree for deletion. */
> - SVN_ERR(mark_tree(dir_access, SVN_WC__ENTRY_MODIFY_SCHEDULE,
> - svn_wc_schedule_delete, FALSE,
> + SVN_ERR(mark_tree(dir_access,
> + SVN_WC__ENTRY_MODIFY_SCHEDULE
> + | SVN_WC__ENTRY_MODIFY_KEEP_LOCAL,
> + svn_wc_schedule_delete, FALSE, keep_local,
> cancel_func, cancel_baton,
> notify_func, notify_baton,
> pool));
According to the comments, we're trying to delete a directory that's
missing. Why might we want to set the keep-local attribute in this
case?
> --- trunk/subversion/libsvn_wc/entries.c (original)
> +++ trunk/subversion/libsvn_wc/entries.c Tue Jan 9 02:20:46 2007
> @@ -646,6 +651,10 @@
> modify_flags, SVN_WC__ENTRY_MODIFY_INCOMPLETE,
> atts, SVN_WC__ENTRY_ATTR_INCOMPLETE, name));
>
> + /* Is this item should be kept in working copy after deletion? */
> + SVN_ERR(do_bool_attr(&entry->keep_local,
> + modify_flags, SVN_WC__ENTRY_MODIFY_KEEP_LOCAL,
> + atts, SVN_WC__ENTRY_ATTR_KEEP_LOCAL, name));
>
(I wondered whether we should only copy the attribute across when the
entry was schedule-deleted, but it doesn't look like we do that kind of
checking here).
> @@ -2178,6 +2197,13 @@
> cur_entry->copyfrom_url = NULL;
> }
>
> + /* keep_local makes sense only when we are going to delete directory. */
> + if (modify_flags & SVN_WC__ENTRY_MODIFY_SCHEDULE
> + && entry->schedule != svn_wc_schedule_delete)
> + {
> + cur_entry->keep_local = FALSE;
> + }
> +
This is where we check for valid combinations. Should we also check
that the entry we're setting keep-local for is the this_dir entry?
> --- trunk/subversion/svn/main.c (original)
> +++ trunk/subversion/svn/main.c Tue Jan 9 02:20:46 2007
> @@ -337,14 +339,16 @@
> "\n"
> " 1. Each item specified by a PATH is scheduled for deletion upon\n"
> " the next commit. Files, and directories that have not been\n"
> - " committed, are immediately removed from the working copy.\n"
> + " committed, are immediately removed from the working copy\n"
> + " unless --keep-local option is given.\n"
> " PATHs that are, or contain, unversioned or modified items will\n"
> " not be removed unless the --force option is given.\n"
> "\n"
> " 2. Each item specified by a URL is deleted from the repository\n"
> " via an immediate commit.\n"),
This is incorrect: it implies that --keep-local only works for uncommitted
(schedule-add) directories.
Regards,
Malcolm
- application/pgp-signature attachment: stored
Received on Wed Jan 10 21:57:03 2007