You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kamesh Jayachandran <ka...@collab.net> on 2007/03/20 15:03:30 UTC
[PATCH][merge-tracking]copy_tests 11 fails over ra_dav
Hi All,
Find the attached patch and log.
With regards
Kamesh Jayachandran
Re: [PATCH][merge-tracking]copy_tests 11 fails over ra_dav
Posted by Daniel Rall <dl...@collab.net>.
On Tue, 27 Mar 2007, Daniel Rall wrote:
> On Tue, 27 Mar 2007, Kamesh Jayachandran wrote:
>
> > Daniel Rall wrote:
> > >Kamesh, this works great! I noticed that it doesn't use the RA
> > >session reparenting suggested by Peter Lundblad -- any particular
> > >reason for that?
> > Yes. After sending the patch last week, I thought of trying reparent
> > api, but could not do so.
> >
> > Yes, reparenting is the way to go.
> >
> > Find the attached patch.
>
> Thanks Kamesh, I'll take a look.
Committed to trunk in r24201.
Re: [PATCH][merge-tracking]copy_tests 11 fails over ra_dav
Posted by Kamesh Jayachandran <ka...@collab.net>.
>> Yes we still need this.
>> In this case ra_dav layer returns 'SVN_ERR_RA_DAV_PATH_NOT_FOUND' not
>> 'SVN_ERR_RA_DAV_REQUEST_FAILED'.
>>
>>
>>> that there's no special-casing for ra_svn's protocol.
>>>
>>>
>> ra_svn returns SVN_ERR_FS_NOT_FOUND like ra_local. So no need to special
>> case.
>>
>
> I wonder if there's a way to smuggle that same error code back to the
> client over DAV?
>
I am posting a separate patch for that.
With regards
Kamesh Jayachandran
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH][merge-tracking]copy_tests 11 fails over ra_dav
Posted by Daniel Rall <dl...@collab.net>.
On Tue, 27 Mar 2007, Kamesh Jayachandran wrote:
> Daniel Rall wrote:
> >Kamesh, this works great! I noticed that it doesn't use the RA
> >session reparenting suggested by Peter Lundblad -- any particular
> >reason for that?
> Yes. After sending the patch last week, I thought of trying reparent
> api, but could not do so.
>
> Yes, reparenting is the way to go.
>
> Find the attached patch.
Thanks Kamesh, I'll take a look.
> >Also, there's some error handling in
> >get_implied_merge_info() to account for locally-added but uncommitted
> >versioned resources; is it still correct as written (e.g. do we still
> >need the test for SVN_ERR_RA_DAV_REQUEST_FAILED)? Seems a little odd
> >
> Yes we still need this.
> In this case ra_dav layer returns 'SVN_ERR_RA_DAV_PATH_NOT_FOUND' not
> 'SVN_ERR_RA_DAV_REQUEST_FAILED'.
>
> >that there's no special-casing for ra_svn's protocol.
> >
> ra_svn returns SVN_ERR_FS_NOT_FOUND like ra_local. So no need to special
> case.
I wonder if there's a way to smuggle that same error code back to the
client over DAV?
Re: [PATCH][merge-tracking]copy_tests 11 fails over ra_dav
Posted by Kamesh Jayachandran <ka...@collab.net>.
Daniel Rall wrote:
> Kamesh, this works great! I noticed that it doesn't use the RA
> session reparenting suggested by Peter Lundblad -- any particular
> reason for that?
Yes. After sending the patch last week, I thought of trying reparent
api, but could not do so.
Yes, reparenting is the way to go.
Find the attached patch.
> Also, there's some error handling in
> get_implied_merge_info() to account for locally-added but uncommitted
> versioned resources; is it still correct as written (e.g. do we still
> need the test for SVN_ERR_RA_DAV_REQUEST_FAILED)? Seems a little odd
>
Yes we still need this.
In this case ra_dav layer returns 'SVN_ERR_RA_DAV_PATH_NOT_FOUND' not
'SVN_ERR_RA_DAV_REQUEST_FAILED'.
> that there's no special-casing for ra_svn's protocol.
>
ra_svn returns SVN_ERR_FS_NOT_FOUND like ra_local. So no need to special
case.
With regards
Kamesh Jayachandran
Re: [PATCH][merge-tracking]copy_tests 11 fails over ra_dav
Posted by Daniel Rall <dl...@collab.net>.
Kamesh, this works great! I noticed that it doesn't use the RA
session reparenting suggested by Peter Lundblad -- any particular
reason for that? Also, there's some error handling in
get_implied_merge_info() to account for locally-added but uncommitted
versioned resources; is it still correct as written (e.g. do we still
need the test for SVN_ERR_RA_DAV_REQUEST_FAILED)? Seems a little odd
that there's no special-casing for ra_svn's protocol.
- Dan
On Tue, 20 Mar 2007, Kamesh Jayachandran wrote:
> [[[
> On the merge-tracking branch: Fix 11th copy_test failure.
>
> * subversion/libsvn_client/copy.c
> (get_implied_merge_info, calculate_target_merge_info):
> Remove FIXME marker.
> (wc_to_repos_copy):
> Remove FIXME marker.
> Call 'calculate_target_merge_info' with ra_session of repos_root.
>
> Patch by: kameshj
> ]]]
> Index: subversion/libsvn_client/copy.c
> ===================================================================
> --- subversion/libsvn_client/copy.c (revision 23922)
> +++ subversion/libsvn_client/copy.c (working copy)
> @@ -390,9 +390,6 @@
> was created (copied or added). */
> err = svn_ra_get_log(ra_session, rel_paths, 1, rev, 1, FALSE, TRUE,
> revnum_receiver, &oldest_rev, pool);
> - /* ### FIXME: ra_dav may fail with SVN_ERR_RA_DAV_PATH_NOT_FOUND.
> - ### This is possibly related to path construction mentioned in
> - ### calculate_target_merge_info()... */
> if (err)
> {
> if (err->apr_err == SVN_ERR_FS_NOT_FOUND ||
> @@ -441,10 +438,6 @@
> pool));
>
> /* Obtain any implied and/or existing (explicit) merge info. */
> - /* ### FIXME: May fail with SVN_ERR_RA_DAV_PATH_NOT_FOUND over
> - ### ra_dav, because we're providing a path relative to the
> - ### repository root instead of the ra_session (which may've been
> - ### opened to a path somewhere under the root). */
> SVN_ERR(get_implied_merge_info(ra_session, target_mergeinfo,
> src_rel_path, src_path, src_revnum, pool));
> SVN_ERR(svn_client__get_merge_info_for_path(ra_session, &src_mergeinfo,
> @@ -956,7 +949,7 @@
> apr_pool_t *pool)
> {
> const char *message;
> - const char *top_src_path, *top_dst_url;
> + const char *top_src_path, *top_dst_url, *repos_root;
> svn_ra_session_t *ra_session;
> const svn_delta_editor_t *editor;
> void *edit_baton;
> @@ -1013,8 +1006,6 @@
> svn_client__copy_pair_t *pair = APR_ARRAY_IDX(copy_pairs, i,
> svn_client__copy_pair_t *);
>
> - /* ### FIXME: I want the path relative to the ra_session
> - ### instead. */
> SVN_ERR(svn_client__path_relative_to_root(&pair->src_rel, pair->src,
> NULL, ra_session, adm_access,
> pool));
> @@ -1086,6 +1077,11 @@
> APR_HASH_KEY_STRING)))
> goto cleanup;
>
> + /* Change the ra_session to repos_root. So that 'svn_ra_get_log'
> + on paths relative to repos_root would work fine. */
> + SVN_ERR(svn_ra_get_repos_root(ra_session, &repos_root, pool));
> + SVN_ERR(svn_client_open_ra_session(&ra_session, repos_root, ctx, pool));
> +
> /* ### TODO: This extra loop would be unnecessary if this code lived
> ### in svn_client__get_copy_committables(), which is incidentally
> ### only used above (so should really be in this source file). */