You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2007/04/06 17:57:37 UTC
[PATCH] follow up to r24468 by not sending depths from client
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)
{
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] follow up to r24468 by not sending depths from client
Posted by Daniel Rall <dl...@collab.net>.
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)
> {