On Fri, Jan 30, 2009 at 17:55, C. Michael Pilato <cmpilato_at_collab.net> wrote:
>...
> +++ branches/http-protocol-v2/subversion/libsvn_ra_serf/serf.c Fri Jan 30 08:55:13 2009 (r35588)
> @@ -397,20 +397,30 @@ svn_ra_serf__rev_proplist(svn_ra_session
> {
> svn_ra_serf__session_t *session = ra_session->priv;
> apr_hash_t *props;
> - const char *vcc_url;
> -
> + const char *propfind_path;
> +
> props = apr_hash_make(pool);
> *ret_props = apr_hash_make(pool);
>
> - SVN_ERR(svn_ra_serf__discover_root(&vcc_url, NULL, TRUE,
> - session, session->conns[0],
> - session->repos_url.path, pool));
> + if (SVN_RA_SERF__HAVE_HTTPV2_SUPPORT(session))
> + {
> + propfind_path = apr_psprintf(pool, "%s/%ld/", session->rev_stub, rev);
> + rev = SVN_INVALID_REVNUM;
There shouldn't be a trailing slash on that URL. The revision resource
is not a collection. There will never be any member resources. (yes,
maybe some other code/parsing needs to change)
What is the rev= for? There is no comment saying why you do that, but
I'm gonna guess that it is to make __retrieve_props() not add a
revision header or somesuch into the PROPFIND? ... or something else?
Too opaque here.
> + }
> + else
> + {
> + /* Use the VCC as the propfind target path. */
> + SVN_ERR(svn_ra_serf__discover_root(&propfind_path, NULL, TRUE,
> + session, session->conns[0],
> + session->repos_url.path, pool));
> + }
>
> SVN_ERR(svn_ra_serf__retrieve_props(props, session, session->conns[0],
> - vcc_url, rev, "0", all_props, pool));
> + propfind_path, rev, "0", all_props,
> + pool));
I don't recall what the "0" is all about, but I get it isn't needed in
v2. And the rev definitely isn't (whatever it is there for) since
you've already got the rev encoded in the URL.
Short answer is: some of these APIs should be simplified, I'd say, for
clarity. The existence of a parameter then leads the reader into
questioning its purpose.
>
> - svn_ra_serf__walk_all_props(props, vcc_url, rev, svn_ra_serf__set_bare_props,
> - *ret_props, pool);
> + svn_ra_serf__walk_all_props(props, propfind_path, rev,
> + svn_ra_serf__set_bare_props, *ret_props, pool);
Again with the rev...
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1075523
Received on 2009-01-30 18:16:39 CET