Gerco Ballintijn writes:
> Index: subversion/libsvn_ra_svn/client.c
> ===================================================================
> --- subversion/libsvn_ra_svn/client.c (revision 23757)
> +++ subversion/libsvn_ra_svn/client.c (working copy)
> @@ -837,7 +816,9 @@
> }
> svn_pool_destroy(iterpool);
> }
> - SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!)b)", keep_locks));
> + SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!)b(!", keep_locks));
> + SVN_ERR(svn_ra_svn_write_proplist(conn, pool, revprop_table));
> + SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!))"));
> SVN_ERR(handle_auth_request(sess_baton, pool));
> SVN_ERR(svn_ra_svn_read_cmd_response(conn, pool, ""));
>
> Index: subversion/libsvn_ra_svn/protocol
> ===================================================================
> --- subversion/libsvn_ra_svn/protocol (revision 23757)
> +++ subversion/libsvn_ra_svn/protocol (working copy)
> @@ -252,7 +252,7 @@
>
> commit
> params: ( logmsg:string ? ( ( lock-path:string lock-token:string ) ... )
> - keep-locks:bool )
> + keep-locks:bool ? rev-props:proplist )
> response: ( )
> Upon receiving response, client switches to editor command set.
> Upon successful completion of edit, server sends auth-request.
> Index: subversion/svnserve/serve.c
> ===================================================================
> --- subversion/svnserve/serve.c (revision 23757)
> +++ subversion/svnserve/serve.c (working copy)
> @@ -935,14 +912,22 @@
>
> if (params->nelts == 1)
> {
> - /* Clients before 1.2 don't send lock-tokens and keep-locks fields. */
> + /* Clients before 1.2 don't send lock-tokens, keep-locks,
> + and rev-props fields. */
> SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "c", &log_msg));
> - lock_tokens = NULL;
> - keep_locks = TRUE;
> }
> + else if (params->nelts == 3)
> + {
> + /* Clients before 1.5 don't send the rev-props field. */
> + SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "clb", &log_msg,
> + &lock_tokens, &keep_locks));
> + }
> else
> - SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "clb", &log_msg,
> - &lock_tokens, &keep_locks));
> + {
> + SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "clbl", &log_msg,
> + &lock_tokens, &keep_locks,
> + &revprop_list));
> + }
>
> /* The handling for locks is a little problematic, because the
> protocol won't let us send several auth requests once one has
The above snippet of a patch (see subject) is fine (except that the
second and third case in the svnserve code could be combined, but that's
not relevant here), except for the fact that the client can't detect
whether the server ignored the rev props or took them into account.
Normally, we want the other end to ignore additional elements in a tuple for
forwards compatibility, but not this time.
So, does anyone have an opinion on the cleanest way to extend
the protocol? Some alternatives spring to my mind:
- We could add a capability that gets negotiated in the initiating phase
of the connection. The client can just error out if a caller wants
to set revprops. This works, but seems a bit ugly for such a minor thing
as a new parameter.
- We could add a new command (commit2) which have all arguments mandatory.
New clients invoke this command and fall back on the old one if
needed. That also works, and is actually what was done when lock switched
to do multiple paths in one roundtrip. But I think it can get a bit
messy over time and the "2" isn't exactly descriptive.
- We can add something to the response tuple, which is currently emtpy.
That could be a flag telling the client that revprops were taken
into account. Also a bit ugly, 'cause we don't do that in general.
Any other ideas? Opinions?
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Mar 12 19:03:56 2007