Vlad,
Mike Pilato and I wrote a patch last night to restore the distinction
between requested depth and reported depth. It passes 'make check',
but could you give it a review?
Note that we'll still need code to actually use the requested depth
when present (if it's absent, i.e., svn_depth_unknown, then this is
not the "upgrade" case and we just use the reported per-path depths as
our guide). But that's a separate change. This just lays the
groundwork for that.
-Karl
[[[
With cmpilato, restore the distinction between requested depth and
reported depth, as discussed in this mail and the thread around it:
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=1267
From: Vlad Georgescu <vgeorgescu@gmail.com>
To: dev@subversion.tigris.org
Subject: Re: [PATCH] Make sparse-directories checkouts/updates \
work with old servers
Date: Fri, 04 May 2007 08:35:06 +0300
Message-ID: <463AC60A.3040103@gmail.com>
This involved reverting portions of r24493 and r24468, and making some
additional changes:
* subversion/libsvn_ra_serf/update.c
(make_update_reporter): Send depth again.
* subversion/libsvn_ra_svn/client.c
(ra_svn_update, ra_svn_switch, ra_svn_status, ra_svn_diff): Same.
* subversion/libsvn_repos/reporter.c
(report_baton_t): Rename 'default_depth' to 'requested_depth', and
explain very clearly what it is, using small words, so you'll be
sure to understand.
(write_path_info): Just error if depth is svn_depth_unknown.
(svn_repos__begin_report): Rename 'default_depth' argument to 'depth',
assign it to b->requested_depth.
(svn_repos_set_path2, svn_repos_link_path2): Default to
svn_depth_infinity.
* subversion/libsvn_ra_svn/protocol
(update, switch, status, diff): Restore depth parameter.
* subversion/svnserve/serve.c
(set_path, link_path): Default depth to svn_depth_infinity again.
(update, switch_cmd, status, diff): Parse optional depth parameter again.
(accept_report): Change RECURSE parameter back to DEPTH.
* subversion/mod_dav_svn/reports/update.c
(dav_svn__update_report): Expect a "depth" element, possibly falling
back to "recurse".
* subversion/libsvn_ra_dav/fetch.c
(make_reporter): Always send requested depth.
]]]
Index: subversion/libsvn_ra_serf/update.c
===================================================================
--- subversion/libsvn_ra_serf/update.c (revision 24967)
+++ subversion/libsvn_ra_serf/update.c (working copy)
@@ -2402,8 +2402,7 @@
report->sess->bkt_alloc);
}
- /* Old servers still pay attention to "recursive" here (modern
- servers use the "depth" argument to link_path/set_path). */
+ /* Old servers know "recursive" but not "depth"; help them DTRT. */
if (depth == svn_depth_files || depth == svn_depth_empty)
{
svn_ra_serf__add_tag_buckets(report->buckets,
@@ -2411,6 +2410,11 @@
report->sess->bkt_alloc);
}
+ /* ### TODO(sd): Send depth as a word, not a number! */
+ svn_ra_serf__add_tag_buckets(report->buckets,
+ "S:depth", apr_itoa(pool, depth),
+ report->sess->bkt_alloc);
+
return SVN_NO_ERROR;
}
Index: subversion/mod_dav_svn/reports/update.c
===================================================================
--- subversion/mod_dav_svn/reports/update.c (revision 24967)
+++ subversion/mod_dav_svn/reports/update.c (working copy)
@@ -916,7 +916,8 @@
const dav_svn_repos *repos = resource->info->repos;
const char *target = "";
svn_boolean_t text_deltas = TRUE;
- svn_depth_t depth = svn_depth_infinity;
+ svn_depth_t depth = svn_depth_unknown;
+ svn_boolean_t saw_depth = FALSE;
svn_boolean_t resource_walk = FALSE;
svn_boolean_t ignore_ancestry = FALSE;
dav_svn__authz_read_baton arb;
@@ -1016,25 +1017,37 @@
return derr;
target = cdata;
}
- if (child->ns == ns && strcmp(child->name, "recursive") == 0)
+ if (child->ns == ns && strcmp(child->name, "depth") == 0)
{
cdata = dav_xml_get_cdata(child, resource->pool, 1);
if (! *cdata)
return malformed_element_error(child->name, resource->pool);
+ depth = svn_depth_from_word(cdata);
+ saw_depth = TRUE;
+ }
+ if ((child->ns == ns && strcmp(child->name, "recursive") == 0)
+ && (! saw_depth))
+ {
+ cdata = dav_xml_get_cdata(child, resource->pool, 1);
+ if (! *cdata)
+ return malformed_element_error(child->name, resource->pool);
if (strcmp(cdata, "no") == 0)
depth = svn_depth_files;
- /* "yes" is the default, so we do nothing special for it.
-
- Note that even modern, depth-aware clients still transmit
- "no" for "recursive" in the svn_depth_files case and the
+ else
+ depth = svn_depth_infinity;
+ /* Note that even modern, depth-aware clients still transmit
+ "no" for "recursive" (along with "files" for "depth") in
+ the svn_depth_files case, and transmit "no" in the
svn_depth_empty case. This is because they don't know if
they're talking to a depth-aware server or not, and they
- don't need to know -- all they have to do is transmit a
- "recursive" element here, and send depths in their
- set_path() calls, and the server will DTRT either way
- (although in the svn_depth_empty case, the client will
- still have some work to do in ignoring the files that
- come down). */
+ don't need to know -- all they have to do is transmit
+ both, and the server will DTRT either way (although in
+ the svn_depth_empty case, the client will still have some
+ work to do in ignoring the files that come down).
+
+ When both "depth" and "recursive" are sent, we don't
+ bother to check if they're mutually consistent, we just
+ let depth dominate. */
}
if (child->ns == ns && strcmp(child->name, "ignore-ancestry") == 0)
{
@@ -1188,10 +1201,6 @@
entry_counter++;
- /* Pre-1.5 clients specify a report-wide depth via a
- recurse flag when beginning the report. */
- depth = svn_depth_unknown;
-
while (this_attr)
{
if (! strcmp(this_attr->name, "rev"))
Index: subversion/libsvn_repos/reporter.c
===================================================================
--- subversion/libsvn_repos/reporter.c (revision 24967)
+++ subversion/libsvn_repos/reporter.c (working copy)
@@ -89,7 +89,13 @@
svn_revnum_t t_rev; /* Revnum which the edit will bring the wc to */
const char *t_path; /* FS path the edit will bring the wc to */
svn_boolean_t text_deltas; /* Whether to report text deltas */
- svn_depth_t default_depth; /* Default depth for set_path and link_path. */
+
+ /* If the client requested a specific depth, record it here; if the
+ client did not, then this is svn_depth_unknown, and the depth of
+ information transmitted from server to client will be governed
+ strictly by the path-associated depths recorded in the report. */
+ svn_depth_t requested_depth;
+
svn_boolean_t ignore_ancestry;
svn_boolean_t is_switch;
const svn_delta_editor_t *editor;
@@ -1044,7 +1050,7 @@
/* --- COLLECTING THE REPORT INFORMATION --- */
/* Record a report operation into the temporary file. Return an error
- if both DEPTH and B->default_depth is svn_depth_unknown. */
+ if DEPTH is svn_depth_unknown. */
static svn_error_t *
write_path_info(report_baton_t *b, const char *path, const char *lpath,
svn_revnum_t rev, svn_depth_t depth,
@@ -1054,16 +1060,10 @@
const char *lrep, *rrep, *drep, *ltrep, *rep;
if (depth == svn_depth_unknown)
- {
- depth = b->default_depth;
+ return svn_error_createf(SVN_ERR_REPOS_BAD_ARGS, NULL,
+ _("Unsupported report depth '%s'"),
+ svn_depth_to_word(depth));
- if (depth == svn_depth_unknown)
- return svn_error_createf(SVN_ERR_REPOS_BAD_ARGS, NULL,
- _("Unsupported report depth '%s' for "
- "path '%s'"),
- svn_depth_to_word(depth), path);
- }
-
/* Munge the path to be anchor-relative, so that we can use edit paths
as report paths. */
path = svn_path_join(b->s_operand, path, pool);
@@ -1104,8 +1104,7 @@
svn_boolean_t start_empty, const char *lock_token,
apr_pool_t *pool)
{
- return svn_repos_set_path3(baton, path, rev,
- ((report_baton_t *) baton)->default_depth,
+ return svn_repos_set_path3(baton, path, rev, svn_depth_infinity,
start_empty, lock_token, pool);
}
@@ -1131,8 +1130,7 @@
svn_revnum_t rev, svn_boolean_t start_empty,
const char *lock_token, apr_pool_t *pool)
{
- return svn_repos_link_path3(baton, path, link_path, rev,
- ((report_baton_t *) baton)->default_depth,
+ return svn_repos_link_path3(baton, path, link_path, rev, svn_depth_infinity,
start_empty, lock_token, pool);
}
@@ -1186,7 +1184,7 @@
const char *s_operand,
const char *switch_path,
svn_boolean_t text_deltas,
- svn_depth_t default_depth,
+ svn_depth_t depth,
svn_boolean_t ignore_ancestry,
const svn_delta_editor_t *editor,
void *edit_baton,
@@ -1207,7 +1205,7 @@
b->t_path = switch_path ? switch_path
: svn_path_join(fs_base, s_operand, pool);
b->text_deltas = text_deltas;
- b->default_depth = default_depth;
+ b->requested_depth = depth;
b->ignore_ancestry = ignore_ancestry;
b->is_switch = (switch_path != NULL);
b->editor = editor;
Index: subversion/libsvn_ra_svn/client.c
===================================================================
--- subversion/libsvn_ra_svn/client.c (revision 24967)
+++ subversion/libsvn_ra_svn/client.c (working copy)
@@ -1083,8 +1083,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)cb", rev, target,
- recurse));
+ SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "update", "(?r)cbw", rev, target,
+ recurse, svn_depth_to_word(depth)));
SVN_ERR(handle_auth_request(sess_baton, pool));
/* Fetch a reporter for the caller to drive. The reporter will drive
@@ -1107,8 +1107,9 @@
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)cbc", rev,
- target, recurse, switch_url));
+ SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "switch", "(?r)cbcw", rev,
+ target, recurse, switch_url,
+ svn_depth_to_word(depth)));
SVN_ERR(handle_auth_request(sess_baton, pool));
/* Fetch a reporter for the caller to drive. The reporter will drive
@@ -1131,8 +1132,9 @@
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)",
- target, recurse, rev));
+ SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "status", "cb(?r)w",
+ target, recurse, rev,
+ svn_depth_to_word(depth)));
SVN_ERR(handle_auth_request(sess_baton, pool));
/* Fetch a reporter for the caller to drive. The reporter will drive
@@ -1158,9 +1160,10 @@
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)cbbcb", rev,
+ SVN_ERR(svn_ra_svn_write_cmd(conn, pool, "diff", "(?r)cbbcbw", rev,
target, recurse, ignore_ancestry,
- versus_url, text_deltas));
+ versus_url, text_deltas,
+ svn_depth_to_word(depth)));
SVN_ERR(handle_auth_request(sess_baton, pool));
/* Fetch a reporter for the caller to drive. The reporter will drive
Index: subversion/libsvn_ra_svn/protocol
===================================================================
--- subversion/libsvn_ra_svn/protocol (revision 24967)
+++ subversion/libsvn_ra_svn/protocol (working copy)
@@ -302,7 +302,7 @@
returned. If rev is not specified, the youngest revision is used.
update
- params: ( [ rev:number ] target:string recurse:bool )
+ params: ( [ rev:number ] target:string recurse:bool ? depth:word)
Client switches to report command set.
Upon finish-report, server sends auth-request.
After auth exchange completes, server switches to editor command set.
@@ -310,7 +310,8 @@
response: ( )
switch
- params: ( [ rev:number ] target:string recurse:bool url:string )
+ params: ( [ rev:number ] target:string recurse:bool url:string
+ ? depth:word)
Client switches to report command set.
Upon finish-report, server sends auth-request.
After auth exchange completes, server switches to editor command set.
@@ -318,7 +319,7 @@
response: ( )
status
- params: ( target:string recurse:bool ? [ rev:number ] )
+ params: ( target:string recurse:bool ? [ rev:number ] ? depth:word )
Client switches to report command set.
Upon finish-report, server sends auth-request.
After auth exchange completes, server switches to editor command set.
@@ -327,7 +328,7 @@
diff
params: ( [ rev:number ] target:string recurse:bool ignore-ancestry:bool
- url:string ? text-deltas:bool )
+ url:string ? text-deltas:bool ? depth:word )
Client switches to report command set.
Upon finish-report, server sends auth-request.
After auth exchange completes, server switches to editor command set.
Index: subversion/libsvn_ra_dav/fetch.c
===================================================================
--- subversion/libsvn_ra_dav/fetch.c (revision 24967)
+++ subversion/libsvn_ra_dav/fetch.c (working copy)
@@ -3105,8 +3105,7 @@
SVN_ERR(svn_io_file_write_full(rb->tmpfile, s, strlen(s), NULL, pool));
}
- /* Old servers still pay attention to "recursive" here (modern
- servers use the "depth" argument to link_path/set_path). */
+ /* Old servers know "recursive" but not "depth"; help them DTRT. */
if (depth == svn_depth_files || depth == svn_depth_empty)
{
const char *data = "<S:recursive>no</S:recursive>" DEBUG_CR;
@@ -3114,6 +3113,13 @@
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)
{
Index: subversion/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c (revision 24967)
+++ subversion/svnserve/serve.c (working copy)
@@ -45,7 +45,6 @@
#include "server.h"
-
typedef struct {
apr_pool_t *pool;
svn_revnum_t *new_rev;
@@ -517,9 +516,8 @@
report_driver_baton_t *b = baton;
const char *path, *lock_token, *depth_word;
svn_revnum_t rev;
- /* Pre-1.5 clients specify a report-wide depth via a recurse flag
- when beginning the report. */
- svn_depth_t depth = svn_depth_unknown;
+ /* Default to infinity, for old clients that don't send depth. */
+ svn_depth_t depth = svn_depth_infinity;
svn_boolean_t start_empty;
SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "crb?(?c)?w",
@@ -554,9 +552,8 @@
const char *path, *url, *lock_token, *fs_path, *depth_word;
svn_revnum_t rev;
svn_boolean_t start_empty;
- /* Pre-1.5 clients specify a report-wide depth via a recurse flag
- when beginning the report. */
- svn_depth_t depth = svn_depth_unknown;
+ /* Default to infinity, for old clients that don't send depth. */
+ svn_depth_t depth = svn_depth_infinity;
SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "ccrb?(?c)?w",
&path, &url, &rev, &start_empty,
@@ -613,7 +610,7 @@
server_baton_t *b, svn_revnum_t rev,
const char *target, const char *tgt_path,
svn_boolean_t text_deltas,
- svn_boolean_t recurse,
+ svn_depth_t depth,
svn_boolean_t ignore_ancestry)
{
const svn_delta_editor_t *editor;
@@ -626,9 +623,8 @@
svn_ra_svn_get_editor(&editor, &edit_baton, conn, pool, NULL, NULL);
SVN_CMD_ERR(svn_repos__begin_report(&report_baton, rev, b->repos,
b->fs_path->data, target, tgt_path,
- text_deltas,
- SVN_DEPTH_FROM_RECURSE(recurse),
- ignore_ancestry, editor, edit_baton,
+ text_deltas, depth, ignore_ancestry,
+ editor, edit_baton,
authz_check_access_cb_func(b),
b, pool));
@@ -1309,18 +1305,27 @@
{
server_baton_t *b = baton;
svn_revnum_t rev;
- const char *target;
+ const char *target, *depth_word;
svn_boolean_t recurse;
+ /* 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", &rev, &target,
- &recurse));
+ SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cb?w", &rev, &target,
+ &recurse, &depth_word));
target = svn_path_canonicalize(target, pool);
+
+ if (depth_word)
+ depth = svn_depth_from_word(depth_word);
+ else
+ depth = recurse ? svn_depth_infinity : svn_depth_empty;
+
SVN_ERR(trivial_auth_request(conn, pool, b));
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, recurse, FALSE);
+ return accept_report(conn, pool, b, rev, target, NULL, TRUE, depth, FALSE);
}
static svn_error_t *switch_cmd(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
@@ -1328,15 +1333,24 @@
{
server_baton_t *b = baton;
svn_revnum_t rev;
- const char *target;
+ const char *target, *depth_word;
const char *switch_url, *switch_path;
svn_boolean_t recurse;
+ /* 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)cbc", &rev, &target,
- &recurse, &switch_url));
+ SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cbc?w", &rev, &target,
+ &recurse, &switch_url, &depth_word));
target = svn_path_canonicalize(target, pool);
switch_url = svn_path_canonicalize(switch_url, pool);
+
+ if (depth_word)
+ depth = svn_depth_from_word(depth_word);
+ else
+ depth = recurse ? svn_depth_infinity : svn_depth_empty;
+
SVN_ERR(trivial_auth_request(conn, pool, b));
if (!SVN_IS_VALID_REVNUM(rev))
SVN_CMD_ERR(svn_fs_youngest_rev(&rev, b->fs, pool));
@@ -1345,7 +1359,7 @@
&switch_path));
return accept_report(conn, pool, b, rev, target, switch_path, TRUE,
- recurse, TRUE);
+ depth, TRUE);
}
static svn_error_t *status(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
@@ -1353,19 +1367,27 @@
{
server_baton_t *b = baton;
svn_revnum_t rev;
- const char *target;
+ const char *target, *depth_word;
svn_boolean_t recurse;
+ /* 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, "cb?(?r)",
- &target, &recurse, &rev));
+ SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "cb?(?r)?w",
+ &target, &recurse, &rev, &depth_word));
target = svn_path_canonicalize(target, pool);
+
+ if (depth_word)
+ depth = svn_depth_from_word(depth_word);
+ else
+ depth = recurse ? svn_depth_infinity : svn_depth_empty;
+
SVN_ERR(trivial_auth_request(conn, pool, b));
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, recurse,
- FALSE);
+ return accept_report(conn, pool, b, rev, target, NULL, FALSE, depth, FALSE);
}
static svn_error_t *diff(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
@@ -1373,9 +1395,12 @@
{
server_baton_t *b = baton;
svn_revnum_t rev;
- const char *target, *versus_url, *versus_path;
+ const char *target, *versus_url, *versus_path, *depth_word;
svn_boolean_t recurse, ignore_ancestry;
svn_boolean_t text_deltas;
+ /* 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. */
if (params->nelts == 5)
@@ -1384,16 +1409,23 @@
SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cbbc", &rev, &target,
&recurse, &ignore_ancestry, &versus_url));
text_deltas = TRUE;
+ depth_word = NULL;
}
else
{
- SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cbbcb",
+ SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cbbcb?w",
&rev, &target, &recurse,
&ignore_ancestry, &versus_url,
- &text_deltas));
+ &text_deltas, &depth_word));
}
target = svn_path_canonicalize(target, pool);
versus_url = svn_path_canonicalize(versus_url, pool);
+
+ if (depth_word)
+ depth = svn_depth_from_word(depth_word);
+ else
+ depth = recurse ? svn_depth_infinity : svn_depth_empty;
+
SVN_ERR(trivial_auth_request(conn, pool, b));
if (!SVN_IS_VALID_REVNUM(rev))
SVN_CMD_ERR(svn_fs_youngest_rev(&rev, b->fs, pool));
@@ -1402,7 +1434,7 @@
&versus_path));
return accept_report(conn, pool, b, rev, target, versus_path,
- text_deltas, recurse, ignore_ancestry);
+ text_deltas, depth, ignore_ancestry);
}
/* Regardless of whether a client's capabilities indicate an
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue May 8 18:38:57 2007