"Rui, Guo" <timmyguo_at_mail.ustc.edu.cn> writes:
>> Of course, svn_io_copy_dir_recursively() should document the specific
>> error it will return. Right now it doesn't; but since callers will
>> depending the exact error from now on (whether SVN_ERR_ENTRY_EXISTS or
>> SVN_ERR_FS_ALREADY_EXISTS), we need to document how that error can be
>> produced.
> Well, please see the definition of the two error codes quoted below.
>
> SVN_ERRDEF(SVN_ERR_ENTRY_EXISTS,
> SVN_ERR_ENTRY_CATEGORY_START + 2,
> "Entry already exists")
>
> SVN_ERRDEF(SVN_ERR_FS_ALREADY_EXISTS,
> SVN_ERR_FS_CATEGORY_START + 20,
> "Item already exists in filesystem")
Ah, it's just a misunderstanding. The "FS" in "SVN_ERR_FS_*" refers to
the Subversion repository filesystem -- that is, the versioned tree with
nodes and representations and DAGs and and all that. It's not the
client's local disk filesystem.
Meanwhile, with "SVN_ERR_ENTRY_EXISTS", it's not clear whether "ENTRY"
refers to a Subversion working copy (.svn) entry, or to a directory
entry in the client's local filesystem, or perhaps both. I agree this
is confusing. In my opinion, you could use SVN_ERR_ENTRY_EXISTS, or
create a new error SVN_ERR_WC_PATH_EXISTS.
>> > @@ -1997,8 +1997,7 @@
>> > /* If the destination exists, try to copy the sources as children of the
>> > destination. */
>> > if (copy_as_child && err && (sources->nelts == 1)
>> > - && (err->apr_err == SVN_ERR_ENTRY_EXISTS
>> > - || err->apr_err == SVN_ERR_FS_ALREADY_EXISTS))
>> > + && (err->apr_err == SVN_ERR_FS_ALREADY_EXISTS))
>> > {
>> > const char *src_path = APR_ARRAY_IDX(sources, 0,
>> > svn_client_copy_source_t *)->path;
>>
>> Okay, I see how changing the error codes simplified the above
>> conditional. But was that the only purpose, or was there some other
>> purpose?
>
> This is not only simplify the condition. Please see the comment in the code.
> This branch will try to copy the source as an sub-entry of the destination if
> the destination exists. However, its only meaningful when the destination
> exists in the FS. An hidden entry will show an obvious exception to this. In
> such a situation, only the entry exists.
>
> PS: The same logic also apply to the move command. You may have to add a
> similar modification as above.
Oops. All this depends on the misunderstanding of what "FS" means. I'm
sorry; I wish I had realized earlier!
Traditionally, in Subversion, "FS" refers to the repository filesystem.
You'll see that convention elsewhere in the code, too.
Best,
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-08-10 16:48:35 CEST