Here is a patch to the problem in question to review .
[[[
Fix double occurrence of the same commit path
in commit items array which takes place in
'svn cp --parent url/src url/dst' command
where src exists and dst does not.
* subversion/libsvn_client/copy.c
(repos_to_repos_copy): when collecting new_dirs in case
make_parents is true proceed only when dir (the result of
svn_path_is_child(top_url, svn_path_dirname(pair->dst, pool),..))
is not null. If dir is null do not bother collecting anything
for new_dirs.
]]]
----
Alexander Sinyushkin,
TMate Software,
http://svnkit.com/ - Java [Sub]Versioning Library!
Karl Fogel wrote:
> Alexander Sinyushkin <Alexander.Sinyushkin_at_svnkit.com> writes:
>> If I try to copy an existing directory to nonexistent one with
>> --parents, like this:
>>
>> svn copy --parents svn://localhost/repo/x svn://localhost/repo/y
>>
>> where x exists and y does not, an editor for commit message is
>> launched twice (if no -m or -F is provided). Commit succeeds,
>> i.e. afterwards I see /y/x in my repo but isn't it a bug that the
>> editor is launched twice?
>
> Yes, that's a bug.
>
>> Debugging this command I see that first call to setup_copy() fails
>> with the following error:
>>
>> "File already exists: filesystem '/home/alex/workspace/tmp/repo/db',
>> transaction '9-f', path '/y'"
>>
>> The problem here is in that commit paths array (which is passed to
>> svn_delta_path_driver()) contain two identical strings "y" what causes
>> the aforementioned error.
>>
>> But then the second call to setup_copy() is made in the following if
>> clause and commit ends well.
>>
>> if (copy_as_child && err && (sources->nelts == 1)
>> && (err->apr_err == SVN_ERR_ENTRY_EXISTS
>> || err->apr_err == SVN_ERR_FS_ALREADY_EXISTS))
>>
>> Anyway, I'm reporting this because you may decide that this behavior
>> should be changed. Thank you.
>
> I doubt anyone would disagree that it's bug... Since you're now familiar
> with the code path, do you have time to make a patch?
>
> Best,
> -Karl
>
>
Index: subversion/libsvn_client/copy.c
===================================================================
--- subversion/libsvn_client/copy.c (revision 31880)
+++ subversion/libsvn_client/copy.c (working copy)
@@ -801,36 +801,32 @@
dir = svn_path_is_child(top_url, svn_path_dirname(pair->dst, pool),
pool);
- /* Imagine a situation where the user passes --parents
- unnecessarily (that is, the destination directory already
- exists). Suppose they're copying a file when they do this.
- There are two syntaxes they might use:
-
- 1. svn copy --parents URL/src/foo.txt URL/dst/foo.txt
- 2. svn copy --parents URL/src/foo.txt URL/dst
-
- Assuming src/ and dst/ exist already, then both ways should
- create dst/foo.txt. However, the svn_path_dirname() call
+ /* Imagine a situation where the user tries to copy an existing source
+ directory to nonexistent directory with --parents options specified:
+
+ svn copy --parents URL/src URL/dst
+
+ where src exists and dst does not. the svn_path_dirname() call
above will produce a string equivalent to top_url, which
- means the svn_path_is_child() will return NULL (quirkily,
- that's what it does when the two paths are the same). We
- must compensate for that behavior, while making sure not to
- add dst/ to the list of new_dirs to be created. */
-
- if (! dir)
- dir = svn_path_is_child(top_url, pair->dst, pool);
+ means the svn_path_is_child() will return NULL. In this case proceed
+ only if dir is not NULL. Do not try to add dst to the new_dirs list
+ since it will be added further in this function to commit items array
+ what can lead to double appearance of the same path in commit paths array */
- SVN_ERR(svn_ra_check_path(ra_session, dir, SVN_INVALID_REVNUM, &kind,
- iterpool));
-
- while (kind == svn_node_none)
+ if (dir)
{
- svn_pool_clear(iterpool);
- APR_ARRAY_PUSH(new_dirs, const char *) = dir;
+ SVN_ERR(svn_ra_check_path(ra_session, dir, SVN_INVALID_REVNUM, &kind,
+ iterpool));
- svn_path_split(dir, &dir, NULL, pool);
- SVN_ERR(svn_ra_check_path(ra_session, dir, SVN_INVALID_REVNUM, &kind,
- iterpool));
+ while (kind == svn_node_none)
+ {
+ svn_pool_clear(iterpool);
+ APR_ARRAY_PUSH(new_dirs, const char *) = dir;
+
+ svn_path_split(dir, &dir, NULL, pool);
+ SVN_ERR(svn_ra_check_path(ra_session, dir, SVN_INVALID_REVNUM, &kind,
+ iterpool));
+ }
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-06-25 14:02:58 CEST