Karl, that looks great, please commit.
- Dan
On Fri, 06 Apr 2007, Karl Fogel wrote:
> Dan, Vlad, et al -- does this look right to you? I'm running tests on
> it right now, just want to know if you agree this is a needed change.
>
> [[[
> Follow up to r24468: adjust protocol sent from client.
>
> * subversion/libsvn_ra_serf/update.c
> (struct report_baton): Get rid of unused depth field.
> (make_update_reporter): Don't send "depth" anymore, don't set it in baton.
> (set_path, link_path): Unconditionally send depth.
>
> * subversion/libsvn_ra_dav/fetch.c
> (reporter_set_path, reporter_link_path): Unconditionally send depth.
>
> * subversion/libsvn_ra_svn/client.c
> (ra_svn_update, ra_svn_switch, ra_svn_status, ra_svn_diff): Don't
> send depth anymore, just send recurse based on depth.
> ]]]
>
> Index: subversion/libsvn_ra_serf/update.c
> ===================================================================
> --- subversion/libsvn_ra_serf/update.c (revision 24482)
> +++ subversion/libsvn_ra_serf/update.c (working copy)
> @@ -274,9 +274,6 @@
> svn_boolean_t ignore_ancestry;
> svn_boolean_t text_deltas;
>
> - /* What depth was requested? */
> - svn_depth_t depth;
> -
> apr_hash_t *lock_path_tokens;
>
> /* Our master update editor and baton. */
> @@ -1871,23 +1868,20 @@
> serf_bucket_aggregate_append(report->buckets, tmp);
> }
>
> - /* ### TODO(sd): Or should this be 'if (depth != report->depth) ...' ? */
> - if (depth != svn_depth_infinity)
> - {
> - tmp = SERF_BUCKET_SIMPLE_STRING_LEN(" depth=\"",
> - sizeof(" depth=\"")-1,
> - report->sess->bkt_alloc);
> - serf_bucket_aggregate_append(report->buckets, tmp);
> -
> - tmp = SERF_BUCKET_SIMPLE_STRING(svn_depth_to_word(depth),
> + /* Depth. */
> + tmp = SERF_BUCKET_SIMPLE_STRING_LEN(" depth=\"",
> + sizeof(" depth=\"")-1,
> report->sess->bkt_alloc);
> - serf_bucket_aggregate_append(report->buckets, tmp);
> + serf_bucket_aggregate_append(report->buckets, tmp);
> +
> + tmp = SERF_BUCKET_SIMPLE_STRING(svn_depth_to_word(depth),
> + report->sess->bkt_alloc);
> + serf_bucket_aggregate_append(report->buckets, tmp);
> +
> + tmp = SERF_BUCKET_SIMPLE_STRING_LEN("\"", sizeof("\"")-1,
> + report->sess->bkt_alloc);
> + serf_bucket_aggregate_append(report->buckets, tmp);
>
> - tmp = SERF_BUCKET_SIMPLE_STRING_LEN("\"", sizeof("\"")-1,
> - report->sess->bkt_alloc);
> - serf_bucket_aggregate_append(report->buckets, tmp);
> - }
> -
> if (start_empty)
> {
> tmp = SERF_BUCKET_SIMPLE_STRING_LEN(" start-empty=\"true\"",
> @@ -1994,23 +1988,20 @@
> serf_bucket_aggregate_append(report->buckets, tmp);
> }
>
> - /* ### TODO(sd): Or should this be 'if (depth != report->depth) ...' ? */
> - if (depth != svn_depth_infinity)
> - {
> - tmp = SERF_BUCKET_SIMPLE_STRING_LEN(" depth=\"",
> - sizeof(" depth=\"")-1,
> - report->sess->bkt_alloc);
> - serf_bucket_aggregate_append(report->buckets, tmp);
> -
> - tmp = SERF_BUCKET_SIMPLE_STRING(svn_depth_to_word(depth),
> + /* Depth. */
> + tmp = SERF_BUCKET_SIMPLE_STRING_LEN(" depth=\"",
> + sizeof(" depth=\"")-1,
> report->sess->bkt_alloc);
> - serf_bucket_aggregate_append(report->buckets, tmp);
> + serf_bucket_aggregate_append(report->buckets, tmp);
>
> - tmp = SERF_BUCKET_SIMPLE_STRING_LEN("\"", sizeof("\"")-1,
> - report->sess->bkt_alloc);
> - serf_bucket_aggregate_append(report->buckets, tmp);
> - }
> + tmp = SERF_BUCKET_SIMPLE_STRING(svn_depth_to_word(depth),
> + report->sess->bkt_alloc);
> + serf_bucket_aggregate_append(report->buckets, tmp);
>
> + tmp = SERF_BUCKET_SIMPLE_STRING_LEN("\"", sizeof("\"")-1,
> + report->sess->bkt_alloc);
> + serf_bucket_aggregate_append(report->buckets, tmp);
> +
> if (start_empty)
> {
> tmp = SERF_BUCKET_SIMPLE_STRING_LEN(" start-empty=\"true\"",
> @@ -2328,7 +2319,6 @@
> report->sess = ra_session->priv;
> report->conn = report->sess->conns[0];
> report->target_rev = revision;
> - report->depth = depth;
> report->ignore_ancestry = ignore_ancestry;
> report->text_deltas = text_deltas;
> report->lock_path_tokens = apr_hash_make(pool);
> @@ -2412,7 +2402,8 @@
> report->sess->bkt_alloc);
> }
>
> - /* Old servers know "recursive" but not "depth"; help them DTRT. */
> + /* Old servers still pay attention to "recursive" here (modern
> + servers use the "depth" argument to link_path/set_path). */
> if (depth == svn_depth_files || depth == svn_depth_empty)
> {
> svn_ra_serf__add_tag_buckets(report->buckets,
> @@ -2420,10 +2411,6 @@
> report->sess->bkt_alloc);
> }
>
> - svn_ra_serf__add_tag_buckets(report->buckets,
> - "S:depth", apr_itoa(pool, depth),
> - report->sess->bkt_alloc);
> -
> return SVN_NO_ERROR;
> }
>
> Index: subversion/libsvn_ra_svn/client.c
> ===================================================================
> --- subversion/libsvn_ra_svn/client.c (revision 24482)
> +++ subversion/libsvn_ra_svn/client.c (working copy)
> @@ -1082,8 +1082,8 @@
> svn_boolean_t recurse = SVN_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)cb", rev, target,
> + recurse));
> SVN_ERR(handle_auth_request(sess_baton, pool));
>
> /* Fetch a reporter for the caller to drive. The reporter will drive
> @@ -1106,9 +1106,8 @@
> svn_boolean_t recurse = SVN_DEPTH_TO_RECURSE(depth);
>
> /* Tell the server we want to start a switch. */
> - SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "switch", "(?r)cbcw", rev,
> - target, recurse, switch_url,
> - svn_depth_to_word(depth)));
> + SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "switch", "(?r)cbc", rev,
> + target, recurse, switch_url));
> SVN_ERR(handle_auth_request(sess_baton, pool));
>
> /* Fetch a reporter for the caller to drive. The reporter will drive
> @@ -1131,9 +1130,8 @@
> svn_boolean_t recurse = SVN_DEPTH_TO_RECURSE(depth);
>
> /* Tell the server we want to start a status operation. */
> - SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "status", "cb(?r)w",
> - target, recurse, rev,
> - svn_depth_to_word(depth)));
> + SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "status", "cb(?r)",
> + target, recurse, rev));
> SVN_ERR(handle_auth_request(sess_baton, pool));
>
> /* Fetch a reporter for the caller to drive. The reporter will drive
> @@ -1159,10 +1157,9 @@
> svn_boolean_t recurse = SVN_DEPTH_TO_RECURSE(depth);
>
> /* Tell the server we want to start a diff. */
> - SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "diff", "(?r)cbbcbw", rev,
> + SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "diff", "(?r)cbbcb", rev,
> target, recurse, ignore_ancestry,
> - versus_url, text_deltas,
> - svn_depth_to_word(depth)));
> + versus_url, text_deltas));
> SVN_ERR(handle_auth_request(sess_baton, pool));
>
> /* Fetch a reporter for the caller to drive. The reporter will drive
> Index: subversion/libsvn_ra_dav/fetch.c
> ===================================================================
> --- subversion/libsvn_ra_dav/fetch.c (revision 24482)
> +++ subversion/libsvn_ra_dav/fetch.c (working copy)
> @@ -2788,11 +2788,9 @@
> report_baton_t *rb = report_baton;
> const char *entry;
> svn_stringbuf_t *qpath = NULL;
> - const char *depthstring = "";
> const char *tokenstring = "";
> -
> - if (depth != svn_depth_infinity)
> - depthstring = apr_psprintf(pool, "depth=\"%s\"", svn_depth_to_word(depth));
> + const char *depthstring = apr_psprintf(pool, "depth=\"%s\"",
> + svn_depth_to_word(depth));
>
> if (lock_token)
> tokenstring = apr_psprintf(pool, "lock-token=\"%s\"", lock_token);
> @@ -2826,11 +2824,9 @@
> const char *entry;
> svn_stringbuf_t *qpath = NULL, *qlinkpath = NULL;
> svn_string_t bc_relative;
> - const char *depthstring = "";
> const char *tokenstring = "";
> -
> - if (depth != svn_depth_infinity)
> - depthstring = apr_psprintf(pool, "depth=\"%s\"", svn_depth_to_word(depth));
> + const char *depthstring = apr_psprintf(pool, "depth=\"%s\"",
> + svn_depth_to_word(depth));
>
> if (lock_token)
> tokenstring = apr_psprintf(pool, "lock-token=\"%s\"", lock_token);
> @@ -3105,7 +3101,8 @@
> SVN_ERR(svn_io_file_write_full(rb->tmpfile, s, strlen(s), NULL, pool));
> }
>
> - /* Old servers know "recursive" but not "depth"; help them DTRT. */
> + /* Old servers still pay attention to "recursive" here (modern
> + servers use the "depth" argument to link_path/set_path). */
> if (depth == svn_depth_files || depth == svn_depth_empty)
> {
> const char *data = "<S:recursive>no</S:recursive>" DEBUG_CR;
> @@ -3113,13 +3110,6 @@
> NULL, pool));
> }
>
> - /* mod_dav_svn defaults to svn_depth_infinity, but we always send anyway. */
> - {
> - s = apr_psprintf(pool, "<S:depth>%s</S:depth>" DEBUG_CR,
> - svn_depth_to_word(depth));
> - SVN_ERR(svn_io_file_write_full(rb->tmpfile, s, strlen(s), NULL, pool));
> - }
> -
> /* mod_dav_svn will use ancestry in diffs unless it finds this element. */
> if (ignore_ancestry)
> {
- application/pgp-signature attachment: stored
Received on Sat Apr 7 01:49:19 2007