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

Fixing depth-handling in the RA interface.

From: Karl Fogel <kfogel_at_red-bean.com>
Date: 2007-03-22 07:37:54 CET

I wrote:
> > Just FYI: now in svnserve/serve.c, accept_report() no longer takes a
> > depth parameter, which means that update|switch|status|diff client
> > protocol should no longer send it and the server should no longer
> > expect it, and libsvn_ra_svn/protocol should be updated... all of
> > which I will do tonight. (One result of r23967.)
>
> Sorry, I phrased that slightly inaccurately. What I meant was,
> accept_report() no longer *uses* its 'depth' parameter, so it should
> go away.

Okay, this change is in theory relatively easy (see partial patch
below), but it raises an interesting question:

Now that we communicate depth via set_path() and link_path(), the
svn_ra_do_{update2,switch2,status2}() functions (which are new in 1.5)
would theoretically lose their 'recurse' parameters (which were slated
to become depths, but now would become nonexistent instead). And
svn_ra_do_diff3(), which is new in 1.5 and for some reason already has
a depth parameter, would lose that parameter.

But... these functions are still transmitting 'recurse' (based on
depth) for older servers. I don't see any way to move that to
set_path()/link_path(), since the protocol requires it in the initial
command.

So in libsvn_ra_svn/client.c, what should we do for ra_svn_update(),
ra_svn_switch(), ra_svn_status(), and ra_svn_diff(), all of which
currently take depth, map it to recurse, and transmit *both* depth and
recurse? In the partial patch below, the server-side change to stop
expecting depth is easy and "obvious"... but on the client side, it
seems odd to me that new 1.5 APIs would take a recurse parameter
instead of depth, even if they're only taking it in order to transmit
something required for compatibility.

I'm not asking this question very well, but hopefully you know what
I'm getting at. Thoughts?

-Karl

[[[
Remove depth from code and protocol paths that no longer use it after r23967.

* subversion/libsvn_ra_svn/protocol
  (3.1.1. Main Command Set): Remove depth from update, switch, status, diff.

* subversion/svnserve/serve.c
  (MAYBE_UNFOLD_TO_DEPTH): Remove this now-unused macro.
  (accept_report): Remove now-unused depth parameter.
  (update, switch_cmd, status, diff): Don't expect a depth word
    anymore, and don't pass depth to accept_report.

### The next changes are being discussed, they are not done: ###

* subversion/libsvn_ra_svn/client.c
  (ra_svn_update, ra_svn_switch, ra_svn_status, ra_svn_diff): These
    should likewise stop transmitting depth, but presumably would have
    to still transmit recurse, because that's not optional. Asking on
    dev@ about this...
]]]

Index: subversion/libsvn_ra_svn/protocol
===================================================================
--- subversion/libsvn_ra_svn/protocol (revision 23998)
+++ subversion/libsvn_ra_svn/protocol (working copy)
@@ -296,7 +296,7 @@
     New in svn 1.2. If path is non-existent, an empty response is returned.
 
   update
- params: ( [ rev:number ] target:string recurse:bool ? depth:word)
+ params: ( [ rev:number ] target:string recurse:bool )
     Client switches to report command set.
     Upon finish-report, server sends auth-request.
     After auth exchange completes, server switches to editor command set.
@@ -304,8 +304,7 @@
     response: ( )
 
   switch
- params: ( [ rev:number ] target:string recurse:bool url:string
- ? depth:word)
+ params: ( [ rev:number ] target:string recurse:bool url:string )
     Client switches to report command set.
     Upon finish-report, server sends auth-request.
     After auth exchange completes, server switches to editor command set.
@@ -313,7 +312,7 @@
     response: ( )
 
   status
- params: ( target:string recurse:bool ? [ rev:number ] ? depth:word )
+ params: ( target:string recurse:bool ? [ rev:number ] )
     Client switches to report command set.
     Upon finish-report, server sends auth-request.
     After auth exchange completes, server switches to editor command set.
@@ -322,7 +321,7 @@
 
   diff
     params: ( [ rev:number ] target:string recurse:bool ignore-ancestry:bool
- url:string ? text-deltas:bool ? depth:word )
+ url:string ? text-deltas:bool )
     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/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c (revision 23998)
+++ subversion/svnserve/serve.c (working copy)
@@ -42,14 +42,6 @@
 
 #include "server.h"
 
-/* Set DEPTH based on boolean RECURSE, but only if DEPTH was unset.
- * This is for old clients that send only recurse, not depth.
- */
-#define MAYBE_UNFOLD_TO_DEPTH(recurse, depth) \
- if ((depth) == svn_depth_unknown) \
- (depth) = ((recurse) ? svn_depth_infinity : svn_depth_empty)
-
-
 typedef struct {
   apr_pool_t *pool;
   svn_revnum_t *new_rev;
@@ -566,7 +558,6 @@
                                   server_baton_t *b, svn_revnum_t rev,
                                   const char *target, const char *tgt_path,
                                   svn_boolean_t text_deltas,
- svn_depth_t depth,
                                   svn_boolean_t ignore_ancestry)
 {
   const svn_delta_editor_t *editor;
@@ -1261,24 +1252,18 @@
 {
   server_baton_t *b = baton;
   svn_revnum_t rev;
- const char *target, *depth_word;
+ const char *target;
   svn_boolean_t recurse;
- /* Default to unknown. Old clients won't send depth, but later
- MAYBE_UNFOLD_TO_DEPTH() will DTRT with that anyway. */
- 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", &rev, &target,
+ &recurse));
   target = svn_path_canonicalize(target, pool);
- if (depth_word)
- depth = svn_depth_from_word(depth_word);
   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));
 
- MAYBE_UNFOLD_TO_DEPTH(recurse, depth);
- return accept_report(conn, pool, b, rev, target, NULL, TRUE, depth, FALSE);
+ return accept_report(conn, pool, b, rev, target, NULL, TRUE, FALSE);
 }
 
 static svn_error_t *switch_cmd(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
@@ -1286,20 +1271,15 @@
 {
   server_baton_t *b = baton;
   svn_revnum_t rev;
- const char *target, *depth_word;
+ const char *target;
   const char *switch_url, *switch_path;
   svn_boolean_t recurse;
- /* Default to unknown. Old clients won't send depth, but later
- MAYBE_UNFOLD_TO_DEPTH() will DTRT with that anyway. */
- svn_depth_t depth = svn_depth_unknown;
 
   /* Parse the arguments. */
- SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cbc?w", &rev, &target,
- &recurse, &switch_url, &depth_word));
+ SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cbc", &rev, &target,
+ &recurse, &switch_url));
   target = svn_path_canonicalize(target, pool);
   switch_url = svn_path_canonicalize(switch_url, pool);
- if (depth_word)
- depth = svn_depth_from_word(depth_word);
   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));
@@ -1307,9 +1287,7 @@
                           svn_path_uri_decode(switch_url, pool),
                           &switch_path));
 
- MAYBE_UNFOLD_TO_DEPTH(recurse, depth);
- return accept_report(conn, pool, b, rev, target, switch_path, TRUE,
- depth, TRUE);
+ return accept_report(conn, pool, b, rev, target, switch_path, TRUE, TRUE);
 }
 
 static svn_error_t *status(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
@@ -1317,24 +1295,18 @@
 {
   server_baton_t *b = baton;
   svn_revnum_t rev;
- const char *target, *depth_word;
+ const char *target;
   svn_boolean_t recurse;
- /* Default to unknown. Old clients won't send depth, but later
- MAYBE_UNFOLD_TO_DEPTH() will DTRT with that anyway. */
- svn_depth_t depth = svn_depth_unknown;
 
   /* Parse the arguments. */
- SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "cb?(?r)?w",
- &target, &recurse, &rev, &depth_word));
+ SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "cb?(?r)",
+ &target, &recurse, &rev));
   target = svn_path_canonicalize(target, pool);
- if (depth_word)
- depth = svn_depth_from_word(depth_word);
   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));
 
- MAYBE_UNFOLD_TO_DEPTH(recurse, depth);
- return accept_report(conn, pool, b, rev, target, NULL, FALSE, depth, FALSE);
+ return accept_report(conn, pool, b, rev, target, NULL, FALSE, FALSE);
 }
 
 static svn_error_t *diff(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
@@ -1342,33 +1314,27 @@
 {
   server_baton_t *b = baton;
   svn_revnum_t rev;
- const char *target, *versus_url, *versus_path, *depth_word;
+ const char *target, *versus_url, *versus_path;
   svn_boolean_t recurse, ignore_ancestry;
   svn_boolean_t text_deltas;
- /* Default to unknown. Old clients won't send depth, but later
- MAYBE_UNFOLD_TO_DEPTH() will DTRT with that anyway. */
- svn_depth_t depth = svn_depth_unknown;
 
   /* Parse the arguments. */
   if (params->nelts == 5)
     {
- /* Clients before 1.4 don't send the text_deltas boolean or depth. */
+ /* Clients before 1.4 don't send the text_deltas boolean. */
       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?w",
+ SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "(?r)cbbcb",
                                      &rev, &target, &recurse,
                                      &ignore_ancestry, &versus_url,
- &text_deltas, &depth_word));
+ &text_deltas));
     }
   target = svn_path_canonicalize(target, pool);
   versus_url = svn_path_canonicalize(versus_url, pool);
- if (depth_word)
- depth = svn_depth_from_word(depth_word);
   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));
@@ -1376,9 +1342,8 @@
                           svn_path_uri_decode(versus_url, pool),
                           &versus_path));
 
- MAYBE_UNFOLD_TO_DEPTH(recurse, depth);
   return accept_report(conn, pool, b, rev, target, versus_path,
- text_deltas, depth, ignore_ancestry);
+ text_deltas, ignore_ancestry);
 }
 
 /* Send a log entry to the client. */

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Mar 22 07:38:14 2007

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