You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "S.Ramaswamy" <ra...@collab.net> on 2005/05/14 08:11:02 UTC
Re: [PATCH] Issue #2188 - [API] svn_client_copy/move should error
>
> The command line client should not be using deprecated functions.
>
ok, will change that in the next version of the patch.
>> --- subversion/libsvn_client/copy.c (revision 14705)
>> +++ subversion/libsvn_client/copy.c (working copy)
>> @@ -67,6 +67,7 @@
>> svn_boolean_t is_move,
>> svn_boolean_t force,
>> svn_client_ctx_t *ctx,
>> + svn_boolean_t dir_override,
>> apr_pool_t *pool)
>> {
>> svn_node_kind_t src_kind, dst_kind;
>> @@ -93,6 +94,12 @@
>> }
>> else if (dst_kind == svn_node_dir)
>> {
>> + if ((!dir_override) && (src_kind == svn_node_dir))
>
> Is that src_kind check correct?
>
> The aim is to make "copy url_or_path1 url_or_path2" predictable,
> i.e. to allow the caller to predict the exact destination URL or path.
> As far as I can see with your patch the caller still doesn't know what
> will get created when src_kind is not a directory.
>
As far as I could make out from the discussions on the lists and the
tracker, the requirement is for the directory creation behavior of svn
copy/move to be configurable, when the dst is an existing directory.
Currently if the destination specified by the user is an existing
directory, and the src is a directory as well, then
a directory with name "dst + basename(src)" is created - this surprises
many users, who would much rather have an error thrown or make this
behavior configurable. That's what this patch allows the non-cmdline svn
clients to do - calling svn_client_copy2() with dir_override=FALSE will
result in an error and dir_override=TRUE maintains the current behaviour.
I imagine that a GUI client like Tortoise will have a have a way of
letting users configure this behaviour. If the src is a directory and
dest_kind is svn_node_none, then the contents of the src dir are copied into
the new dst dir - no directory with name = "dst + basename(src)" is created
in this case.
When the src_kind is svn_node_file, and the dst of a similar kind then an
error is thrown; if the dst_kind is svn_node_dir, the file simply gets
copied into the dst dir.
> I don't know whether it makes sense to have a flag to retain the old
> behaviour in the new function. If the "smart" destination logic was
> relegated to the deprecated functions the new functions might well be
> simpler.
>
Not sure what you mean in the last para. Can you be a little more verbose
:-).
Thanks
Ramaswamy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2188 - [API] svn_client_copy/move should error
Posted by "S.Ramaswamy" <ra...@collab.net>.
On Sat, 2005-05-14 at 22:24 +0100, Philip Martin wrote:
>
> I think the aim should be to provide an interface that produces
> predictable results.
Hmm, that puts the issue in a different light altogether - maybe the
tracker needs to be updated with a crisper defintion of the issue.
I am not going to working on this anymore.
Thanks
Ramaswamy
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue #2188 - [API] svn_client_copy/move should error
Posted by Philip Martin <ph...@codematters.co.uk>.
"S.Ramaswamy" <ra...@collab.net> writes:
>> The aim is to make "copy url_or_path1 url_or_path2" predictable,
>> i.e. to allow the caller to predict the exact destination URL or path.
>> As far as I can see with your patch the caller still doesn't know what
>> will get created when src_kind is not a directory.
>>
>
> As far as I could make out from the discussions on the lists and the
> tracker, the requirement is for the directory creation behavior of svn
> copy/move to be configurable, when the dst is an existing directory.
I think the aim should be to provide an interface that produces
predictable results.
> Currently if the destination specified by the user is an existing
> directory, and the src is a directory as well, then
> a directory with name "dst + basename(src)" is created - this surprises
> many users, who would much rather have an error thrown or make this
> behavior configurable.
Forget about "users", this is an API so think about "applications".
An application wil be just as "surprised" when it runs
svn_client_move foo/bar zig/zag
expecting to create a file zig/zag but getting a file zig/zag/bar.
The application has no reliable way to determine which name will get
created.
>> I don't know whether it makes sense to have a flag to retain the old
>> behaviour in the new function. If the "smart" destination logic was
>> relegated to the deprecated functions the new functions might well be
>> simpler.
>
> Not sure what you mean in the last para. Can you be a little more verbose
> :-).
I'm suggesting that a sensible API would always be predictable, so the
current "magic" that tests whether dst is a directory should be
deprecated. Thus only the deprecated functions would have the
checking code, the new functions would simply assume that the
destination does not exist and attempt to create the copy, returning
an error if the some existing item prevents the copy.
I'm suggesting that svn_client_move should be modelled on the C
library function rename(2) rather than the program 'mv'. Similarly
svn_client_copy should be modelled on link(2) rather than 'cp'.
Things get more complicated if we want to support multiple source
targets, we know approximately how it should behave but nobody has
worked out the details, see:
http://svn.haxx.se/dev/archive-2005-04/0075.shtml
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org