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

Re: svn commit: r26503 - in trunk/subversion: include libsvn_client libsvn_ra libsvn_ra_local libsvn_ra_neon libsvn_ra_serf libsvn_ra_svn libsvn_repos mod_dav_svn/reports svnserve tests/libsvn_repos

From: Hyrum K. Wright <hyrum_wright_at_mail.utexas.edu>
Date: 2007-09-10 14:39:37 CEST

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

sussman@tigris.org wrote:
> Author: sussman
> Date: Sun Sep 9 21:56:55 2007
> New Revision: 26503
>
> Log:
> Send copyfrom-args on updates *only* if the client explicitly requests them.
>
> This keeps the 1.5 servers backward-compatible. Because incoming
> copyfrom args cause old clients to error, they should only be
> requested by new clients that understand them. This means adding a
> new boolean 'send_copyfrom_args' to the main libsvn_repos
> editor-driver, svn_ra_do_update2(), and all four RA implementations.
> Eesh!

I haven't reviewed the whole patch, in form or substance, but the
following modifications to the svn protocol did catch my eye:

> * subversion/libsvn_ra_svn/client.c
> (ra_svn_update): transmit new arg as boolean in update-request tuple.
>
> * subversion/svnserve/serve.c
> (accept_report): take new boolean, pass to svn_repos_begin_report2.
> (update): possibly parse new optional boolean in update-request tuple,
> pass to accept_report.
> (switch): pass FALSE to new accept_report arg. For now.
> (status, diff): pass FALSE to new accept_report arg.
...
> Modified: trunk/subversion/libsvn_ra_svn/client.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_svn/client.c?pathrev=26503&r1=26502&r2=26503
> ==============================================================================
> --- trunk/subversion/libsvn_ra_svn/client.c (original)
> +++ trunk/subversion/libsvn_ra_svn/client.c Sun Sep 9 21:56:55 2007
> @@ -1071,6 +1071,7 @@
> const svn_ra_reporter3_t **reporter,
> void **report_baton, svn_revnum_t rev,
> const char *target, svn_depth_t depth,
> + svn_boolean_t send_copyfrom_args,
> const svn_delta_editor_t *update_editor,
> void *update_baton, apr_pool_t *pool)
> {
> @@ -1079,8 +1080,9 @@
> svn_boolean_t recurse = DEPTH_TO_RECURSE(depth);
>
> /* Tell the server we want to start an update. */
> - SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "update", "(?r)cbw", rev, target,
> - recurse, svn_depth_to_word(depth)));
> + SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "update", "(?r)cbwb", rev, target,
> + recurse, svn_depth_to_word(depth),
> + send_copyfrom_args));

Here, you send a boolean ('b'), which is fine...

> SVN_ERR(handle_auth_request(sess_baton, pool));
>
> /* Fetch a reporter for the caller to drive. The reporter will drive
>
...
> Modified: trunk/subversion/svnserve/serve.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svnserve/serve.c?pathrev=26503&r1=26502&r2=26503
> ==============================================================================
> --- trunk/subversion/svnserve/serve.c (original)
> +++ trunk/subversion/svnserve/serve.c Sun Sep 9 21:56:55 2007
> @@ -609,6 +609,7 @@
> const char *target, const char *tgt_path,
> svn_boolean_t text_deltas,
> svn_depth_t depth,
> + svn_boolean_t send_copyfrom_args,
> svn_boolean_t ignore_ancestry)
> {
> const svn_delta_editor_t *editor;
> @@ -622,6 +623,7 @@
> SVN_CMD_ERR(svn_repos_begin_report2(&report_baton, rev, b->repos,
> b->fs_path->data, target, tgt_path,
> text_deltas, depth, ignore_ancestry,
> + send_copyfrom_args,
> editor, edit_baton,
> authz_check_access_cb_func(b),
> b, pool));
> @@ -1305,13 +1307,14 @@
> svn_revnum_t rev;
> const char *target, *full_path, *depth_word;
> svn_boolean_t recurse;
> + svn_boolean_t send_copyfrom_args = FALSE;
> /* 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;
>
> /* Parse the arguments. */
> - SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cb?w", &rev, &target,
> - &recurse, &depth_word));
> + SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cb?w?b", &rev, &target,
> + &recurse, &depth_word, &send_copyfrom_args));

...but here you parse a boolean ('b') which is verboten. parse_tuple()
has no way of specifying "unspecified" when the boolean value is not
provided by the client. What you probably want to use is 'B', which
gives three values, including SVN_RA_SVN_UNSPECIFIED_NUMBER.
subversion/svnserve/serve.c:log_cmd() has some examples.

> target = svn_path_canonicalize(target, pool);
>
> if (depth_word)
> @@ -1326,7 +1329,8 @@
> if (!SVN_IS_VALID_REVNUM(rev))
> SVN_CMD_ERR(svn_fs_youngest_rev(&rev, b->fs, pool));
>
> - return accept_report(conn, pool, b, rev, target, NULL, TRUE, depth, FALSE);
> + return accept_report(conn, pool, b, rev, target, NULL, TRUE,
> + depth, send_copyfrom_args, FALSE);
> }
>
> static svn_error_t *switch_cmd(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
> @@ -1360,7 +1364,9 @@
> &switch_path));
>
> return accept_report(conn, pool, b, rev, target, switch_path, TRUE,
> - depth, TRUE);
> + depth,
> + FALSE /* TODO(sussman): no copyfrom args for now */,
> + TRUE);
> }
>
> static svn_error_t *status(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
> @@ -1388,7 +1394,8 @@
> if (!SVN_IS_VALID_REVNUM(rev))
> SVN_CMD_ERR(svn_fs_youngest_rev(&rev, b->fs, pool));
>
> - return accept_report(conn, pool, b, rev, target, NULL, FALSE, depth, FALSE);
> + return accept_report(conn, pool, b, rev, target, NULL, FALSE,
> + depth, FALSE, FALSE);
> }
>
> static svn_error_t *diff(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
> @@ -1436,7 +1443,7 @@
> &versus_path));
>
> return accept_report(conn, pool, b, rev, target, versus_path,
> - text_deltas, depth, ignore_ancestry);
> + text_deltas, depth, FALSE, ignore_ancestry);
> }
>
> /* Regardless of whether a client's capabilities indicate an

Also, you may want to update subversion/libsvn_ra_svn/protocol to
reflect the new parameter.

- -Hyrum
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFG5TsJCwOubk4kUXwRAjsTAKCa2qr5Gzr4qg+VRfmUOQ6B4ogawACfYIbG
xX4LEsQ65GaxJkMs7755Rt8=
=ZIOm
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Sep 10 14:36:26 2007

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