[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r22599 - in branches/multiple-moves/subversion: include libsvn_client svn

From: Daniel Rall <dlr_at_collab.net>
Date: 2006-12-07 20:39:27 CET

On Thu, 07 Dec 2006, hwright@tigris.org wrote:
...
> On the multiple moves branch:
> Merge the svn_client_[copy,move] and svn_client_[copy,move]_into APIs. The
> idea of the new APIs is to Do The Right Thing when given a list of source paths
> and a destination path. The new APIs also have a flag to preserve compatibility
> with the changes introduced in r18315 to fix issue 2188.

Bravo! I think this addressed issue #2188 well. Stefan, can you
confirm?

...
> --- branches/multiple-moves/subversion/include/svn_client.h (original)
> +++ branches/multiple-moves/subversion/include/svn_client.h Thu Dec 7 10:53:16 2006
> @@ -1898,28 +1898,32 @@
...
> + * If multiple @a src_paths are given, @a dst_path must be a directory,
> + * and @a src_paths will be copied as children of @a dst_path.
> + *
> + * @a src_paths must be files or directories under version control, or
> + * URLs of a versioned item in the repository. If @a src_paths has multiple
> + * items, they must @a src_revision is used to choose the revision from which
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This text needs a wording fix.

> + * to copy the @a src_path.
> + *
> + * The parent of @a dst_path must already exist. If @a src_paths has only
> + * one item, and @a copy_as_child is TRUE, @a dst_path must be a non-existent
                                                                    ^^^^
Wouldn't the destination need to exist, if you wanted to copy a path
as a child of it?

What happens for the COPY_AS_CHILD case if you have more than one path
in SRC_PATHS? I'd still expect this to work (similarly to a vanilla
command-line 'cp' command).

> + * WC path or URL. If @a src_paths has only one item, @a copy_as_child is
> + * FALSE, and @a dst_path already exists, fail with @c SVN_ERR_ENTRY_EXISTS
> + * if @a dst_path is a working copy path and @c SVN_ERR_FS_ALREADY_EXISTS if
> + * @a dst_path is an URL.

I think this maintains compatibility with Stefan's desired behavior
for TortoiseSVN.

...
> +svn_error_t *
> +svn_client_copy4(svn_commit_info_t **commit_info_p,
> + apr_array_header_t *src_paths,
> + const svn_opt_revision_t *src_revision,
> + const char *dst_path,
> + svn_boolean_t copy_as_child,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);
...
> + * @a src_paths must be files or directories under version control, or
> + * URLs of versioned items in the repository. All @a src_paths must be of
> + * the same type. If multiple @a src_paths are given, @a dst_path must be
> + * a directory and @src_paths will be moved as children of @a dst_path.
...
> * - This is a scheduling operation. No changes will happen to the
> * repository until a commit occurs. This scheduling can be removed
...
> + * with svn_client_revert(). If one of @a src_paths is a file it is
> + * removed from the working copy immediately. If one of @a src_path
                                                                   ^^^^^^^^^
Parameter name should be plural.

> + * is a directory it will remain in the working copy but all the files,
> + * and unversioned items, it contains will be removed.
> + *
> + * - If one of @a src_paths contains locally modified and/or unversioned
> + * items and @a force is not set, the move will fail. If @a force is set
> + * such items will be removed.
> + *
> + * The parent of @a dst_path must already exist. If @a src_paths has only
> + * one item, and @a move_as_child is TRUE, @a dst_path must be a non-existent

Ditto as for copy (what if there are multiple source paths?).

> + * WC path or URL. If @a src_paths has only one item, @a copy_as_child is
> + * FALSE, and @a dst_path already exists, fail with @c SVN_ERR_ENTRY_EXISTS
> + * if @a dst_path is a working copy path and @c SVN_ERR_FS_ALREADY_EXISTS if
> + * @a dst_path is an URL.
...
> +svn_error_t *
> +svn_client_move5(svn_commit_info_t **commit_info_p,
> + apr_array_header_t *src_paths,
> + const char *dst_path,
> + svn_boolean_t force,
> + svn_boolean_t move_as_child,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);
...
> --- branches/multiple-moves/subversion/libsvn_client/copy.c (original)
> +++ branches/multiple-moves/subversion/libsvn_client/copy.c Thu Dec 7 10:53:16 2006
> @@ -1363,6 +1363,45 @@
>
>
> /* Public Interfaces */
> +svn_error_t *
> +svn_client_copy4(svn_commit_info_t **commit_info_p,
> + apr_array_header_t *src_paths,
> + const svn_opt_revision_t *src_revision,
> + const char *dst_path,
> + svn_boolean_t copy_as_child,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> +{
> + svn_error_t *err;
> +
> + err = setup_copy(commit_info_p,
> + src_paths, src_revision, dst_path,
> + FALSE /* is_move */,
> + TRUE /* force, set to avoid deletion check */,
> + ctx,
> + pool);
> +
> + /* If the destination exists, try to copy the sources as children of the
> + destination. */
> + if (copy_as_child && err && (src_paths->nelts == 1)
> + && (err->apr_err == SVN_ERR_ENTRY_EXISTS
> + || err->apr_err == SVN_ERR_FS_ALREADY_EXISTS))
> + {
> + const char *src_path = ((const char **) (src_paths->elts))[0];
> + const char *src_basename = svn_path_basename(src_path, pool);
> +
> + return setup_copy(commit_info_p,
> + src_paths, src_revision,
> + svn_path_join(dst_path, src_basename, pool),
> + FALSE /* is_move */,
> + TRUE /* force, set to avoid deletion check */,
> + ctx,
> + pool);
> + }
> +
> + return err;
> +}
...
> +svn_client_move5(svn_commit_info_t **commit_info_p,
> + apr_array_header_t *src_paths,
> + const char *dst_path,
> + svn_boolean_t force,
> + svn_boolean_t move_as_child,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> +{
> + const svn_opt_revision_t src_revision
> + = { svn_opt_revision_unspecified, { 0 } };
> + svn_error_t *err;
> +
> + err = setup_copy(commit_info_p, src_paths, &src_revision, dst_path,
> + TRUE /* is_move */,
> + force,
> + ctx,
> + pool);
> +
> + /* If the destination exists, try to copy the sources as children of the
> + destination. */
> + if (move_as_child && err && (src_paths->nelts == 1)
> + && (err->apr_err == SVN_ERR_ENTRY_EXISTS
> + || err->apr_err == SVN_ERR_FS_ALREADY_EXISTS))
> {
> const char *src_path = ((const char **) (src_paths->elts))[0];
> const char *src_basename = svn_path_basename(src_path, pool);
> - dst_path = svn_path_join(dst_dir, src_basename, pool);
> +
> + return setup_copy(commit_info_p, src_paths, &src_revision,
> + svn_path_join(dst_path, src_basename, pool),
> + TRUE /* is_move */,
> + force,
> + ctx,
> + pool);
> }
...
> + return err;
> }
...
> --- branches/multiple-moves/subversion/svn/copy-cmd.c (original)
> +++ branches/multiple-moves/subversion/svn/copy-cmd.c Thu Dec 7 10:53:16 2006
> @@ -111,33 +111,8 @@
> SVN_ERR(svn_cl__make_log_msg_baton(&(ctx->log_msg_baton2), opt_state,
> NULL, ctx->config, pool));
>
> - if (targets->nelts == 1)
> - {
> - /* Standard copy, try to copy the source onto the destination. */
> - src_path = ((const char **) (targets->elts))[0];
> -
> - err = svn_client_copy3(&commit_info, src_path,
> - &(opt_state->start_revision),
> - dst_path, ctx, pool);
> -
> - /* If dst_path already exists, try to copy src_path as a child of it,
> - * by using copy_into. */
> - if (err && (err->apr_err == SVN_ERR_ENTRY_EXISTS
> - || err->apr_err == SVN_ERR_FS_ALREADY_EXISTS))
> - {
> - svn_error_clear(err);
> -
> - err = svn_client_copy_into(&commit_info, targets,
> - &(opt_state->start_revision),
> - dst_path, ctx, pool);
> - }
> - }
> - else
> - {
> - err = svn_client_copy_into(&commit_info, targets,
> - &(opt_state->start_revision),
> - dst_path, ctx, pool);
> - }
> + err = svn_client_copy4(&commit_info, targets, &(opt_state->start_revision),
> + dst_path, TRUE, ctx, pool);
...
> --- branches/multiple-moves/subversion/svn/move-cmd.c (original)
> +++ branches/multiple-moves/subversion/svn/move-cmd.c Thu Dec 7 10:53:16 2006
> @@ -70,31 +70,8 @@
> dst_path = ((const char **) (targets->elts))[targets->nelts - 1];
> apr_array_pop(targets);
>
> - if (targets->nelts == 1)
> - {
> - /* A simple move, move the source to the destination. */
> - src_path = ((const char **) (targets->elts))[0];
> -
> - err = svn_client_move4(&commit_info, src_path, dst_path,
> - opt_state->force, ctx, pool);
> -
> - /* If dst_path already exists, try to move src_path as a child of it, by
> - * using move_into. */
> - if (err && (err->apr_err == SVN_ERR_ENTRY_EXISTS
> - || err->apr_err == SVN_ERR_FS_ALREADY_EXISTS))
> - {
> - svn_error_clear(err);
> -
> - err = svn_client_move_into(&commit_info, targets, dst_path,
> - opt_state->force, ctx, pool);
> - }
> - }
> - else
> - {
> - /* If there are multiple sources, move the targets into dst_path. */
> - err = svn_client_move_into(&commit_info, targets, dst_path,
> - opt_state->force, ctx, pool);
> - }
> + err = svn_client_move5(&commit_info, targets, dst_path, opt_state->force,
> + TRUE, ctx, pool);
...

  • application/pgp-signature attachment: stored
Received on Thu Dec 7 20:41:15 2006

This is an archived mail posted to the Subversion Dev mailing list.