You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ph...@apache.org on 2010/03/16 18:11:15 UTC
svn commit: r923875 - /subversion/trunk/subversion/libsvn_client/copy.c
Author: philip
Date: Tue Mar 16 17:11:15 2010
New Revision: 923875
URL: http://svn.apache.org/viewvc?rev=923875&view=rev
Log:
* subversion/libsvn_client/copy.c (wc_to_repos_copy): Remove access baton.
Modified:
subversion/trunk/subversion/libsvn_client/copy.c
Modified: subversion/trunk/subversion/libsvn_client/copy.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/copy.c?rev=923875&r1=923874&r2=923875&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/copy.c (original)
+++ subversion/trunk/subversion/libsvn_client/copy.c Tue Mar 16 17:11:15 2010
@@ -1142,7 +1142,6 @@ wc_to_repos_copy(svn_commit_info_t **com
void *edit_baton;
void *commit_baton;
apr_hash_t *committables;
- svn_wc_adm_access_t *adm_access;
apr_array_header_t *commit_items;
const svn_wc_entry_t *entry;
apr_pool_t *iterpool;
@@ -1150,15 +1149,13 @@ wc_to_repos_copy(svn_commit_info_t **com
apr_hash_t *commit_revprops;
int i;
- /* Find the common root of all the source paths, and probe the wc. */
+ /* Find the common root of all the source paths */
get_copy_pair_ancestors(copy_pairs, &top_src_path, NULL, NULL, pool);
- SVN_ERR(svn_wc__adm_probe_in_context(&adm_access, ctx->wc_ctx, top_src_path,
- FALSE, -1, ctx->cancel_func,
- ctx->cancel_baton, pool));
-
- /* The commit process uses absolute paths, so we need to open the access
- baton using absolute paths, and so we really need to use absolute
- paths everywhere. */
+
+ /* Do we need to lock the working copy? 1.6 didn't take a write
+ lock, but what happens if the working copy changes during the copy
+ operation? */
+
iterpool = svn_pool_create(pool);
for (i = 0; i < copy_pairs->nelts; i++)
@@ -1193,8 +1190,7 @@ wc_to_repos_copy(svn_commit_info_t **com
}
SVN_ERR(svn_client__open_ra_session_internal(&ra_session, top_dst_url,
- svn_wc_adm_access_path(
- adm_access),
+ top_src_path,
NULL, TRUE, TRUE, ctx, pool));
/* If requested, determine the nearest existing parent of the destination,
@@ -1272,7 +1268,7 @@ wc_to_repos_copy(svn_commit_info_t **com
if (! message)
{
svn_pool_destroy(iterpool);
- return svn_error_return(svn_wc_adm_close2(adm_access, pool));
+ return SVN_NO_ERROR;
}
}
else
@@ -1295,7 +1291,7 @@ wc_to_repos_copy(svn_commit_info_t **com
SVN_CLIENT__SINGLE_REPOS_NAME,
APR_HASH_KEY_STRING)))
{
- return svn_error_return(svn_wc_adm_close2(adm_access, pool));
+ return SVN_NO_ERROR;
}
/* If we are creating intermediate directories, tack them onto the list
@@ -1397,8 +1393,7 @@ wc_to_repos_copy(svn_commit_info_t **com
svn_pool_destroy(iterpool);
- /* It's only a read lock, so unlocking is harmless. */
- return svn_error_return(svn_wc_adm_close2(adm_access, pool));
+ return SVN_NO_ERROR;
}
/* Peform each individual copy operation for a repos -> wc copy. A
Re: svn commit: r923875 - /subversion/trunk/subversion/libsvn_client/copy.c
Posted by Greg Stein <gs...@gmail.com>.
On Tue, Mar 16, 2010 at 15:59, Philip Martin <ph...@wandisco.com> wrote:
> Greg Stein <gs...@gmail.com> writes:
>
>> On Tue, Mar 16, 2010 at 13:11, <ph...@apache.org> wrote:
>>>...
>>> +++ subversion/trunk/subversion/libsvn_client/copy.c Tue Mar 16 17:11:15 2010
>>>...
>>> @@ -1150,15 +1149,13 @@ wc_to_repos_copy(svn_commit_info_t **com
>>> apr_hash_t *commit_revprops;
>>> int i;
>>>
>>> - /* Find the common root of all the source paths, and probe the wc. */
>>> + /* Find the common root of all the source paths */
>>> get_copy_pair_ancestors(copy_pairs, &top_src_path, NULL, NULL, pool);
>>> - SVN_ERR(svn_wc__adm_probe_in_context(&adm_access, ctx->wc_ctx, top_src_path,
>>> - FALSE, -1, ctx->cancel_func,
>>> - ctx->cancel_baton, pool));
>>> -
>>> - /* The commit process uses absolute paths, so we need to open the access
>>> - baton using absolute paths, and so we really need to use absolute
>>> - paths everywhere. */
>>> +
>>> + /* Do we need to lock the working copy? 1.6 didn't take a write
>>> + lock, but what happens if the working copy changes during the copy
>>> + operation? */
>>
>> I'd switch this to a ### comment saying "we should lock the working
>> copy to prevent changes while we perform the copy to the repository."
>>
>> But when we do that... aren't we starting a commit? and doesn't the
>> commit lock the working copy?
>
> No, it calls the lower level function svn_client__do_commit that does
> no locking. I think I'll change it to take locks, assuming that doing
> so doesn't cause regression tests failures. I don't suppose anybody
> relies on wc-to-repo copy "working" when the wc is already locked :)
Sounds good. Again, I would suggest the call_with_write_lock()
function. I would like to eventually remove the acquire/release
variants, as they are more prone to leaving locks around.
Cheers,
-g
Re: svn commit: r923875 - /subversion/trunk/subversion/libsvn_client/copy.c
Posted by Philip Martin <ph...@wandisco.com>.
Greg Stein <gs...@gmail.com> writes:
> On Tue, Mar 16, 2010 at 13:11, <ph...@apache.org> wrote:
>>...
>> +++ subversion/trunk/subversion/libsvn_client/copy.c Tue Mar 16 17:11:15 2010
>>...
>> @@ -1150,15 +1149,13 @@ wc_to_repos_copy(svn_commit_info_t **com
>> apr_hash_t *commit_revprops;
>> int i;
>>
>> - /* Find the common root of all the source paths, and probe the wc. */
>> + /* Find the common root of all the source paths */
>> get_copy_pair_ancestors(copy_pairs, &top_src_path, NULL, NULL, pool);
>> - SVN_ERR(svn_wc__adm_probe_in_context(&adm_access, ctx->wc_ctx, top_src_path,
>> - FALSE, -1, ctx->cancel_func,
>> - ctx->cancel_baton, pool));
>> -
>> - /* The commit process uses absolute paths, so we need to open the access
>> - baton using absolute paths, and so we really need to use absolute
>> - paths everywhere. */
>> +
>> + /* Do we need to lock the working copy? 1.6 didn't take a write
>> + lock, but what happens if the working copy changes during the copy
>> + operation? */
>
> I'd switch this to a ### comment saying "we should lock the working
> copy to prevent changes while we perform the copy to the repository."
>
> But when we do that... aren't we starting a commit? and doesn't the
> commit lock the working copy?
No, it calls the lower level function svn_client__do_commit that does
no locking. I think I'll change it to take locks, assuming that doing
so doesn't cause regression tests failures. I don't suppose anybody
relies on wc-to-repo copy "working" when the wc is already locked :)
--
Philip
Re: svn commit: r923875 - /subversion/trunk/subversion/libsvn_client/copy.c
Posted by Greg Stein <gs...@gmail.com>.
On Tue, Mar 16, 2010 at 13:11, <ph...@apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_client/copy.c Tue Mar 16 17:11:15 2010
>...
> @@ -1150,15 +1149,13 @@ wc_to_repos_copy(svn_commit_info_t **com
> apr_hash_t *commit_revprops;
> int i;
>
> - /* Find the common root of all the source paths, and probe the wc. */
> + /* Find the common root of all the source paths */
> get_copy_pair_ancestors(copy_pairs, &top_src_path, NULL, NULL, pool);
> - SVN_ERR(svn_wc__adm_probe_in_context(&adm_access, ctx->wc_ctx, top_src_path,
> - FALSE, -1, ctx->cancel_func,
> - ctx->cancel_baton, pool));
> -
> - /* The commit process uses absolute paths, so we need to open the access
> - baton using absolute paths, and so we really need to use absolute
> - paths everywhere. */
> +
> + /* Do we need to lock the working copy? 1.6 didn't take a write
> + lock, but what happens if the working copy changes during the copy
> + operation? */
I'd switch this to a ### comment saying "we should lock the working
copy to prevent changes while we perform the copy to the repository."
But when we do that... aren't we starting a commit? and doesn't the
commit lock the working copy?
>...
Cheers,
-g