Daniel Näslund wrote on Mon, Aug 16, 2010 at 15:16:33 +0200:
> And this time with a patch attached.
>
> On Mon, Aug 16, 2010 at 03:09:49PM +0200, Daniel Näslund wrote:
> > I'm touching a lot of code here. Reviews would be much apprecieated.
>
> >
> > [[[
> > Make the diff editor able to receive copyfrom information. Involves
> > passing down a 'send_copyfrom_args' to all RA implemtations.
> >
> > Note that this commit merely allows the copyfrom args to be passed to
> > the client. They copyfrom information is not yet stored and used.
This commit changes the ra_svn protocols, but not the other protocols.
Okay.
> > ]]]
> >
> > Thanks,
> > Daniel
> Index: subversion/libsvn_ra/deprecated.c
> ===================================================================
> --- subversion/libsvn_ra/deprecated.c (revision 985813)
> +++ subversion/libsvn_ra/deprecated.c (arbetskopia)
> @@ -239,6 +239,30 @@ svn_error_t *svn_ra_get_commit_editor(svn_ra_sessi
> keep_locks, pool);
> }
>
> +svn_error_t * svn_ra_do_diff3(svn_ra_session_t *session,
Style nit: no space after '*'.
> + apr_pool_t *pool)
> Index: subversion/libsvn_ra_svn/protocol
> ===================================================================
> --- subversion/libsvn_ra_svn/protocol (revision 985813)
> +++ subversion/libsvn_ra_svn/protocol (arbetskopia)
> @@ -345,7 +345,8 @@ second place for auth-request point as noted below
>
> diff
> params: ( [ rev:number ] target:string recurse:bool
> ignore-ancestry:bool
> - url:string ? text-deltas:bool ? depth:word )
> + url:string ? text-deltas:bool ? depth:word
> + send_copyfrom_param:bool )
Err, shouldn't this be
+ url:string ? text-deltas:bool ? depth:word
+ ? send_copyfrom_param:bool )
since clients before-your-change don't send the send_copyfrom_args param?
> Client switches to report command set.
> Upon finish-report, server sends auth-request.
> After auth exchange completes, server switches to editor command set.
> Index: subversion/include/svn_ra.h
> ===================================================================
> --- subversion/include/svn_ra.h (revision 985813)
> +++ subversion/include/svn_ra.h (arbetskopia)
> @@ -1291,9 +1291,30 @@ svn_ra_do_status(svn_ra_session_t *session,
> * needed, and sending too much data back, a pre-1.5 'recurse'
> * directive may be sent to the server, based on @a depth.
> *
> - * @since New in 1.5.
> + * @since New in 1.7.
> */
> svn_error_t *
> +svn_ra_do_diff4(svn_ra_session_t *session,
> + const svn_ra_reporter3_t **reporter,
> + void **report_baton,
> + svn_revnum_t revision,
> + const char *diff_target,
> + svn_depth_t depth,
> + svn_boolean_t send_copyfrom_args,
> + svn_boolean_t ignore_ancestry,
> + svn_boolean_t text_deltas,
> + const char *versus_url,
> + const svn_delta_editor_t *diff_editor,
> + void *diff_baton,
> + apr_pool_t *pool);
Need an empty line right here (as I "said" in a previous mail).
> +/**
> + * Similar to svn_ra_do_diff4(), but with @c send_copyfrom_args set to
> + * FALSE.
> + *
> + * @deprecated Provided for compatibility with the 1.5 API.
> + */
> +SVN_DEPRECATED
> +svn_error_t *
> svn_ra_do_diff3(svn_ra_session_t *session,
> const svn_ra_reporter3_t **reporter,
> void **report_baton,
> @@ -1306,7 +1327,6 @@ svn_ra_do_diff3(svn_ra_session_t *session,
> const svn_delta_editor_t *diff_editor,
> void *diff_baton,
> apr_pool_t *pool);
> -
And here you removed an empty line? Can we have it back please?
(that way one can use paragraph-motion commands (e.g., '}') when reading
the source.)
> /**
> * Similar to svn_ra_do_diff3(), but taking @c svn_ra_reporter2_t
> * instead of @c svn_ra_reporter3_t, and therefore only able to report
> Index: subversion/libsvn_client/repos_diff.c
> ===================================================================
> --- subversion/libsvn_client/repos_diff.c (revision 985813)
> +++ subversion/libsvn_client/repos_diff.c (arbetskopia)
> @@ -552,6 +552,8 @@ delete_entry(const char *path,
> svn_wc_notify_action_t action = svn_wc_notify_skip;
> svn_boolean_t tree_conflicted = FALSE;
>
> + SVN_DBG(("delete_entry: %s\n", path));
> +
Shouldn't be committed. (There are a few others in the diff.)
> Index: subversion/svnserve/serve.c
> ===================================================================
> --- subversion/svnserve/serve.c (revision 985813)
> +++ subversion/svnserve/serve.c (arbetskopia)
> @@ -1670,6 +1670,8 @@ static svn_error_t *diff(svn_ra_svn_conn_t *conn,
> /* Default to unknown. Old clients won't send depth, but we'll
> handle that by converting recurse if necessary. */
> svn_depth_t depth = svn_depth_unknown;
> + apr_uint64_t send_copyfrom_param;
> + svn_boolean_t send_copyfrom_args;
>
> /* Parse the arguments. */
> if (params->nelts == 5)
> @@ -1682,10 +1684,14 @@ static svn_error_t *diff(svn_ra_svn_conn_t *conn,
> }
> else
> {
> - SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cbbcb?w",
> + /* ### I'm only duplicating the logic in update() for parsing
> + * ### the send_copyfrom_param. Find out how the svn protocol works.
> + */
Is this ### still outstanding?
> + SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cbbcb?wB",
Should this be
+ SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cbbcb?w?B",
?
Have you tested a server with this patch against a client before the patch?
(and vice-versa)
> &rev, &target, &recurse,
> &ignore_ancestry, &versus_url,
> - &text_deltas, &depth_word));
> + &text_deltas, &depth_word,
> + &send_copyfrom_param));
> }
> target = svn_uri_canonicalize(target, pool);
> versus_url = svn_uri_canonicalize(versus_url, pool);
