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

Re: svn commit: r23760 - in branches/sparse-directories/subversion: libsvn_ra_svn svnserve

From: Karl Fogel <kfogel_at_red-bean.com>
Date: 2007-03-12 04:28:12 CET

lundblad@tigris.org writes:
> Log:
> On the sparse-directories branch: make the depth
> parameter to the set-path and link-path ra_svn protocol commands non-optional.
>
> NOTE: In a sense, this parameter is still "optional" because the parameter
> list may end before it. However, if a client wants to put parameters after
> depth (which might be added in the future), they have to specify a depth.

Peter, please correct me if I'm misunderstanding the protocol here,
but:

What I had intended to do was make transmission of an actual depth
value optional, by placing it inside a tuple and allowing the tuple to
be empty.

That's why the svn_ra_svn_parse_tuple() calls had "(?w)", while the
svn_ra_svn_write_cmd() calls had "(w)". (The existing code already
had "(?c)" in the svn_ra_svn_write_cmd() calls, which makes sense for
that parameter because the lock_token might be NULL.) But the client
definitely has the depth, so we transmit unconditionally -- at least
today. The reason to make it a possibly-empty tuple is to reserve a
future right to transmit no depth at all, not even svn_depth_unknown.

Why might we need that ability? Well, if someday depth itself becomes
obsolete, the ideal thing would be for clients to simply transmit an
empty tuple. That would keep the client code simple, and make the
wire protocol less confusing for debugging, and yet it would still be
compatible with older servers (i.e., the servers we're writing right
now).

I don't see any reason why we'd be likely to abandon depth, it just
seems good to be gratuitously forward-compatible. Thoughts?

-Karl

> * subversion/libsvn_ra_svn/protocol (3.1.3): Make depth non-optional
> for the commands that take a depth.
>
> * subversion/libsvn_ra_svn/client.c
> (ra_svn_set_path): Remove parens around depth parameter. (And don't
> flush stdout, BTW;)
> (ra_svn_link_path): Remove parens around depth parameter.
>
> * svnserve/serve.c (set_path, link_path): Make depth non-optional.
>
>
> Modified:
> branches/sparse-directories/subversion/libsvn_ra_svn/client.c
> branches/sparse-directories/subversion/libsvn_ra_svn/protocol
> branches/sparse-directories/subversion/svnserve/serve.c
>
> Modified: branches/sparse-directories/subversion/libsvn_ra_svn/client.c
> URL: http://svn.collab.net/viewvc/svn/branches/sparse-directories/subversion/libsvn_ra_svn/client.c?pathrev=23760&r1=23759&r2=23760
> ==============================================================================
> --- branches/sparse-directories/subversion/libsvn_ra_svn/client.c (original)
> +++ branches/sparse-directories/subversion/libsvn_ra_svn/client.c Fri Mar 9 15:59:36 2007
> @@ -259,10 +259,9 @@
> {
> ra_svn_reporter_baton_t *b = baton;
>
> - SVN_ERR(svn_ra_svn_write_cmd(b->conn, pool, "set-path", "crb(?c)(w)",
> + SVN_ERR(svn_ra_svn_write_cmd(b->conn, pool, "set-path", "crb(?c)w",
> path, rev, start_empty, lock_token,
> svn_depth_to_word(depth)));
> - fflush(stdout);
> return SVN_NO_ERROR;
> }
>
> @@ -285,7 +284,7 @@
> {
> ra_svn_reporter_baton_t *b = baton;
>
> - SVN_ERR(svn_ra_svn_write_cmd(b->conn, pool, "link-path", "ccrb(?c)(w)",
> + SVN_ERR(svn_ra_svn_write_cmd(b->conn, pool, "link-path", "ccrb(?c)w",
> path, url, rev, start_empty, lock_token,
> svn_depth_to_word(depth)));
> return SVN_NO_ERROR;
>
> Modified: branches/sparse-directories/subversion/libsvn_ra_svn/protocol
> URL: http://svn.collab.net/viewvc/svn/branches/sparse-directories/subversion/libsvn_ra_svn/protocol?pathrev=23760&r1=23759&r2=23760
> ==============================================================================
> --- branches/sparse-directories/subversion/libsvn_ra_svn/protocol (original)
> +++ branches/sparse-directories/subversion/libsvn_ra_svn/protocol Fri Mar 9 15:59:36 2007
> @@ -484,14 +484,14 @@
>
> set-path:
> params: ( path:string rev:number start-empty:bool
> - ? [ lock-token:string ] ? [ depth:word ] )
> + ? [ lock-token:string ] ? depth:word )
>
> delete-path:
> params: ( path:string )
>
> link-path:
> params: ( path:string url:string rev:number start-empty:bool
> - ? [ lock-token:string ] ? [ depth:word ] )
> + ? [ lock-token:string ] ? depth:word )
>
> finish-report:
> params: ( )
>
> Modified: branches/sparse-directories/subversion/svnserve/serve.c
> URL: http://svn.collab.net/viewvc/svn/branches/sparse-directories/subversion/svnserve/serve.c?pathrev=23760&r1=23759&r2=23760
> ==============================================================================
> --- branches/sparse-directories/subversion/svnserve/serve.c (original)
> +++ branches/sparse-directories/subversion/svnserve/serve.c Fri Mar 9 15:59:36 2007
> @@ -476,7 +476,7 @@
> svn_depth_t depth = svn_depth_infinity;
> svn_boolean_t start_empty;
>
> - SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "crb?(?c)?(?w)",
> + SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "crb?(?c)?w",
> &path, &rev, &start_empty, &lock_token,
> &depth_word));
> if (depth_word)
> @@ -511,7 +511,7 @@
> /* 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)",
> + SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "ccrb?(?c)?w",
> &path, &url, &rev, &start_empty,
> &lock_token, &depth_word));
> path = svn_path_canonicalize(path, pool);
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Mar 12 04:28:26 2007

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