Paul Burba <paulb@softlanding.com> writes:
> The original proposal was for svn co to tolerate obstructions by default,
> but eventually I decided to require the --force option.
I think "checkout --force" is acceptable but I think the behaviour
should be extended to "update --force" as well, after all, checkout
and update share a lot of code.
> Note: When a file or dir exists prior to a forced checkout and obstructs
> during the co, it is reported as
>
> "T WC_PATH/PATH"
>
> rather than
>
> "A WC_PATH/PATH"
>
> "T" of course stands for takenover. This applies to use cases C, D, E,
> and H.
We already use 'T' for a "sTolen" lock during update. If update can
also takeover then we really need to use some other letter, how about
'V' for "takenoVer"? Even if we don't change update I think it would
be a good idea to avoid reusing 'T'.
> Open Questions
> --------------
>
> I'm uncertain what the ideal behavior is in use cases E and G.
>
> For use case E, this patch has problems with property mods. Is it best to
> simply fail whenever a versioned directory obstructs?
It solves the basic "takeover an unversioned tree" problem so it's a
good starting point.
> Should it be more like an update, including potential conflicts?
I think that could get very complicated, I'd be tempted to leave it as
an obstruction error.
> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h (revision 19507)
> +++ subversion/include/svn_client.h (working copy)
> @@ -691,6 +691,13 @@
> * just the directory represented by @a URL and its immediate
> * non-directory children, but none of its child directories (if any).
> *
> + * If @a force is @c TRUE then permit the checkout to tolerate an existing
> + * local tree rooted at @a PATH even if some paths in the local tree
> + * are identical to those in @a URL. File paths that collide will
> + * effectively treat the local file as a user modification to the pristine
> + * checkout. If @a force is @c FALSE then the checkout will abort if there
> + * are any colliding paths.
> + *
> * If @a URL refers to a file rather than a directory, return the
> * error SVN_ERR_UNSUPPORTED_FEATURE. If @a URL does not exist,
> * return the error SVN_ERR_RA_ILLEGAL_URL.
> @@ -700,6 +707,25 @@
> * @since New in 1.2.
Not 1.2
> */
> svn_error_t *
> +svn_client_checkout3(svn_revnum_t *result_rev,
> + const char *URL,
> + const char *path,
> + const svn_opt_revision_t *peg_revision,
> + const svn_opt_revision_t *revision,
> + svn_boolean_t recurse,
> + svn_boolean_t ignore_externals,
> + svn_boolean_t force,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);
> +
> +
> +/**
> + * Similar to svn_client_checkout3() but with the force parameter always
> + * set to @c FALSE.
> + *
> + * @deprecated Provided for backward compatibility with the 1.3 API.
> + */
> +svn_error_t *
> svn_client_checkout2(svn_revnum_t *result_rev,
> const char *URL,
> const char *path,
> Index: subversion/include/svn_wc.h
> ===================================================================
> --- subversion/include/svn_wc.h (revision 19507)
> +++ subversion/include/svn_wc.h (working copy)
> @@ -640,7 +640,11 @@
> svn_wc_notify_failed_lock,
>
> /** Failed to unlock a path. @since New in 1.2. */
> - svn_wc_notify_failed_unlock
> + svn_wc_notify_failed_unlock,
> +
> + /** @since New in 1.4. Took over a path during a forced checkout. */
> + svn_wc_notify_co_takeover
Perhaps a bit late for 1.4? Drop the _co if supporting update,
perhaps drop it even if not supporting update?
> +
> } svn_wc_notify_action_t;
>
>
> @@ -2472,8 +2476,34 @@
> * have their working timestamp set to the last-committed-time. If
> * FALSE, the working files will be touched with the 'now' time.
> *
> - * @since New in 1.2.
> + * If @a forced_checkout is TRUE, then the @a *editor and @a *edit_baton
> + * returned are for a checkout with the --force option, if @c FALSE these
> + * are for a co without --force or an update.
> + *
> + * @since New in 1.4.
> */
> +svn_error_t *svn_wc_get_update_editor3(svn_revnum_t *target_revision,
> + svn_wc_adm_access_t *anchor,
> + const char *target,
> + svn_boolean_t use_commit_times,
> + svn_boolean_t recurse,
> + svn_boolean_t forced_checkout,
'force' rather than 'forced_checkout' would be consistent with the
other APIs, and is obviously better if it applies to updates as well.
[I now think reusing "--force" was a mistake in the client, we should
have used "--force xxx" or "--force-xxx" from the beginning.]
> + svn_wc_notify_func2_t notify_func,
> + void *notify_baton,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + const char *diff3_cmd,
> + const svn_delta_editor_t **editor,
> + void **edit_baton,
> + svn_wc_traversal_info_t *ti,
> + apr_pool_t *pool);
> +
> +
> +/* Similar to svn_wc_get_update_editor3() but with the forced_checkout
> + * parameter always set to @c FALSE.
> + *
> + * @deprecated Provided for backward compatibility with the 1.3 API.
> + */
> svn_error_t *svn_wc_get_update_editor2(svn_revnum_t *target_revision,
> svn_wc_adm_access_t *anchor,
> const char *target,
> Index: subversion/libsvn_wc/update_editor.c
> ===================================================================
> --- subversion/libsvn_wc/update_editor.c (revision 19507)
> +++ subversion/libsvn_wc/update_editor.c (working copy)
> @@ -84,6 +84,11 @@
>
> /* Was the update-target deleted? This is a special situation. */
> svn_boolean_t target_deleted;
> +
> + /* Is this an edit_baton for a checkout with the --force option? If so,
In general avoid referring to the command line client in the wc code.
> + existing unversioned files will be accepted as is and not obstruct the
> + update process. */
> + svn_boolean_t forced_checkout;
>
> /* Non-null if this is a 'switch' operation. */
> const char *switch_url;
> @@ -2250,7 +2384,8 @@
> {
> svn_wc_notify_t *notify
> = svn_wc_create_notify(fb->path,
> - fb->added ? svn_wc_notify_update_add
> + fb->taken_over ? svn_wc_notify_co_takeover
> + : fb->added ? svn_wc_notify_update_add
> : svn_wc_notify_update_update, pool);
Looks a bit hard to read, how about
(fb->taken_over
? svn_wc_notify_co_takeover
: (fb->added
? svn_wc_notify_update_add
: svn_wc_notify_update_update)),
> notify->kind = svn_node_file;
> notify->content_state = content_state;
> @@ -2341,6 +2476,7 @@
> svn_boolean_t use_commit_times,
> const char *switch_url,
> svn_boolean_t recurse,
> + svn_boolean_t forced_checkout,
> svn_wc_notify_func2_t notify_func,
> void *notify_baton,
> svn_cancel_func_t cancel_func,
> @@ -2386,6 +2522,7 @@
> eb->diff3_cmd = diff3_cmd;
> eb->cancel_func = cancel_func;
> eb->cancel_baton = cancel_baton;
> + eb->forced_checkout = forced_checkout;
>
> /* Construct an editor. */
> tree_editor->set_target_revision = set_target_revision;
> @@ -2417,6 +2554,31 @@
>
>
> svn_error_t *
> +svn_wc_get_update_editor3(svn_revnum_t *target_revision,
> + svn_wc_adm_access_t *anchor,
> + const char *target,
> + svn_boolean_t use_commit_times,
> + svn_boolean_t recurse,
> + svn_boolean_t forced_checkout,
> + svn_wc_notify_func2_t notify_func,
> + void *notify_baton,
> + svn_cancel_func_t cancel_func,
> + void *cancel_baton,
> + const char *diff3_cmd,
> + const svn_delta_editor_t **editor,
> + void **edit_baton,
> + svn_wc_traversal_info_t *traversal_info,
> + apr_pool_t *pool)
> +{
> + return make_editor(target_revision, anchor, svn_wc_adm_access_path(anchor),
> + target, use_commit_times, NULL, recurse,
> + forced_checkout, notify_func, notify_baton, cancel_func,
> + cancel_baton, diff3_cmd, editor, edit_baton,
> + traversal_info, pool);
> +}
> +
> +
> +svn_error_t *
> svn_wc_get_update_editor2(svn_revnum_t *target_revision,
> svn_wc_adm_access_t *anchor,
> const char *target,
> @@ -2433,9 +2595,9 @@
> apr_pool_t *pool)
> {
> return make_editor(target_revision, anchor, svn_wc_adm_access_path(anchor),
> - target, use_commit_times, NULL, recurse, notify_func,
> - notify_baton, cancel_func, cancel_baton, diff3_cmd,
> - editor, edit_baton, traversal_info, pool);
> + target, use_commit_times, NULL, recurse, FALSE,
> + notify_func, notify_baton, cancel_func, cancel_baton,
> + diff3_cmd, editor, edit_baton, traversal_info, pool);
> }
>
> svn_error_t *
> @@ -2485,7 +2647,7 @@
> assert(switch_url);
>
> return make_editor(target_revision, anchor, svn_wc_adm_access_path(anchor),
> - target, use_commit_times, switch_url, recurse,
> + target, use_commit_times, switch_url, recurse, FALSE,
> notify_func, notify_baton, cancel_func, cancel_baton,
> diff3_cmd, editor, edit_baton, traversal_info, pool);
> }
> Index: subversion/svn/checkout-cmd.c
> ===================================================================
> --- subversion/svn/checkout-cmd.c (revision 19507)
> +++ subversion/svn/checkout-cmd.c (working copy)
> @@ -156,12 +156,24 @@
> revision.kind = svn_opt_revision_head;
> }
>
> - SVN_ERR(svn_client_checkout2(NULL, true_url, target_dir,
> + SVN_ERR(svn_client_checkout3(NULL, true_url, target_dir,
> &peg_revision,
> &revision,
> opt_state->nonrecursive ? FALSE : TRUE,
> opt_state->ignore_externals,
> + opt_state->force,
> ctx, subpool));
> +
> + /* If this is a checkout with the --force option it was likely used
> + to create a WC over an existing local tree. If any of the file
> + paths in the intial tree collided with paths coming from the
> + repository then the text-time field for this file in entries is
> + equal to the committed-date field. This identifies the file as
> + *potentially* modified. Running cleanup scrubs the entries file
> + so that WC files with no actual modifications have their text-time
> + field set the the file's last modified time. */
> + if (opt_state->force)
> + SVN_ERR(svn_client_cleanup(target_dir, ctx, subpool));
That's ugly: it involves an extra wc pass, extra disk IO, sleeping
twice, etc. Far better to get checkout to do the right thing, which
also means that other clients don't have to implement the same trick.
> }
> svn_pool_destroy(subpool);
>
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat May 6 00:19:14 2006