[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Extending the ra_svn protocol (was: Re: [PATCH] (3rd try) Fix issue #1976: Set arbitrary revision properties (revprops) during commit.)

From: Peter Lundblad <plundblad_at_google.com>
Date: 2007-03-12 19:03:30 CET

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

This is an archived mail posted to the Subversion Dev mailing list.