You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Garrett Rooney <ro...@electricjellyfish.net> on 2006/02/01 22:20:37 UTC
Re: svn commit: r18315 - in trunk/subversion: include libsvn_client svn tests/cmdline
On 2/1/06, lundblad@tigris.org <lu...@tigris.org> wrote:
> Author: lundblad
> Date: Wed Feb 1 16:53:34 2006
> New Revision: 18315
>
> Modified:
> trunk/subversion/include/svn_client.h
> trunk/subversion/libsvn_client/copy.c
> trunk/subversion/svn/copy-cmd.c
> trunk/subversion/svn/move-cmd.c
> trunk/subversion/tests/cmdline/copy_tests.py
>
> Log:
> Fix issue #2188: svn_client_copy/move should error out if target exists.
>
> This change introduces new copy/move public functions with the same argument
> lists, but with the semantic change that they always fail if the destination
> exists. The command line client implements the old behaviour of copying the
> source as a child of an existing target.
Any reason this has the compat glue in the client instead of making
the behavior optional via a new parameter or something? I'm thinking
an enum that controls the behavior, leaving us with the option of
later implementing the ability to overwrite the existing item without
having to rev the API again...
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18315 - in trunk/subversion: include libsvn_client
svn tests/cmdline
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sun, 5 Feb 2006, Julian Foad wrote:
> (Peter, these doc string problems may not be your fault, it's just that I've
> noticed them after this change.)
>
I fixed these in r18534. Feel free to improve further if you like.
>
> > * Attempt to create any parent directories for @a dst_path that do not exist.
>
> I added that line in r17415, but, by experiment, it does not appear to be true.
> I wonder whether it was true even then. I think it was true at least for WC
> destinations, before you added the following code to wc_to_wc_copy() and
> repos_to_wc_copy():
>
> > /* Make sure the destination parent is a directory and produce a clear
> > error message if it is not. */
> > SVN_ERR (svn_io_check_path (dst_parent, &dst_parent_kind, pool));
> > if (dst_parent_kind != svn_node_none)
> > return svn_error_createf (SVN_ERR_WC_NOT_DIRECTORY, NULL,
>
> What do we want? Do create parents, don't create parents, or compatibility
> with how it was before (which may have been a mixture)?
>
This code just improves the error message. If there is no parent, the
operation would fail when the code would try to open an adm access baton
to the dst parent. What we want? Someone seems to want an option to
create parents:-) See issue #1776.
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18315 - in trunk/subversion: include libsvn_client
svn tests/cmdline
Posted by Julian Foad <ju...@btopenworld.com>.
(Peter, these doc string problems may not be your fault, it's just that I've
noticed them after this change.)
From svn_client.h: svn_client_copy3():
> /** Copy @a src_path to @a dst_path.
> *
> * @a src_path must be a file or directory under version control, or the
> * URL of a versioned item in the repository. If @a src_path is a
> * URL, @a src_revision is used to choose the revision from which to copy
> * the @a src_path.
... otherwise is src_revision ignored, or does it have some effect anyway?
> @a dst_path must be a file or directory under version
> * control, or a repository URL.
That's not right. dst_path must be a path or URL that is not yet under version
control. (It will become a file or directory, the same kind as src_path.)
> * If @a dst_path already exists, fail.
Now that you have written code that relies on particular error codes being
returned, we need to document them here.
> * Attempt to create any parent directories for @a dst_path that do not exist.
I added that line in r17415, but, by experiment, it does not appear to be true.
I wonder whether it was true even then. I think it was true at least for WC
destinations, before you added the following code to wc_to_wc_copy() and
repos_to_wc_copy():
> /* Make sure the destination parent is a directory and produce a clear
> error message if it is not. */
> SVN_ERR (svn_io_check_path (dst_parent, &dst_parent_kind, pool));
> if (dst_parent_kind != svn_node_none)
> return svn_error_createf (SVN_ERR_WC_NOT_DIRECTORY, NULL,
What do we want? Do create parents, don't create parents, or compatibility
with how it was before (which may have been a mixture)?
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18315 - in trunk/subversion: include libsvn_client
svn tests/cmdline
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Wed, 1 Feb 2006, Garrett Rooney wrote:
> On 2/1/06, lundblad@tigris.org <lu...@tigris.org> wrote:
> > Author: lundblad
> > Date: Wed Feb 1 16:53:34 2006
> > New Revision: 18315
> >
> > Modified:
> > trunk/subversion/include/svn_client.h
> > trunk/subversion/libsvn_client/copy.c
> > trunk/subversion/svn/copy-cmd.c
> > trunk/subversion/svn/move-cmd.c
> > trunk/subversion/tests/cmdline/copy_tests.py
> >
> > Log:
> > Fix issue #2188: svn_client_copy/move should error out if target exists.
> >
> > This change introduces new copy/move public functions with the same argument
> > lists, but with the semantic change that they always fail if the destination
> > exists. The command line client implements the old behaviour of copying the
> > source as a child of an existing target.
>
> Any reason this has the compat glue in the client instead of making
> the behavior optional via a new parameter or something? I'm thinking
> an enum that controls the behavior, leaving us with the option of
> later implementing the ability to overwrite the existing item without
> having to rev the API again...
>
maxb proposed that right before 12 was released; many people wanted to
move the createchildifexist logic out of the function to simplify the API.
Some of the threads are linked to the issue. Note that in the cmdline
client, this is not compatibility code, it is just how the cmdline client
wants to handle a particular failure case.
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org