Re: svn commit: r18258 - trunk/subversion/libsvn_ra_serf

["Peter N. Lundblad" <pe...@famlundblad.se>]

On Thu, 26 Jan 2006 jerenkrantz@tigris.org wrote:

>
>  static svn_error_t *
> -svn_ra_serf__reparent (svn_ra_session_t *session,
> +svn_ra_serf__reparent (svn_ra_session_t *ra_session,
>                         const char *url,
>                         apr_pool_t *pool)
>  {
> -  abort();
> +  serf_session_t *session = ra_session->priv;
> +  apr_uri_t new_url;
> +
> +  /* If it's the URL we already have, wave our hands and do nothing. */
> +  if (strcmp(session->repos_url_str, url) == 0)
> +    {
> +      return SVN_NO_ERROR;
> +    }
> +
> +  /* Do we need to check that it's the same host and port? */

I'd say no, since the caller svn_ra_reparent will guarantee that the
repository root part is the same on the old and new URL.

> +  apr_uri_parse(session->pool, url, &new_url);
> +
Minor leak, since the session pool is used.  I solved this with a
stringbuf in the other RA layers when I implemented svn_ra_reparent.

> +  session->repos_url.path = new_url.path;
> +  session->repos_url_str = apr_pstrdup(pool, url);

Woops, you use the temporary pool for a "session variable".

> +
>  static svn_error_t *
> -svn_ra_serf__get_repos_root (svn_ra_session_t *session,
> +svn_ra_serf__get_repos_root (svn_ra_session_t *ra_session,
>                               const char **url,
>                               apr_pool_t *pool)
>  {
> -  abort();
> +  serf_session_t *session = ra_session->priv;
> +
> +  if (!session->repos_root_str)
> +    {
> +      const char *baseline_url, *root_path;
> +      svn_stringbuf_t *url_buf;
> +      apr_hash_t *props;
> +
> +      SVN_ERR(fetch_props(&props, session, session->repos_url.path, "0",
> +                          repos_root_props, pool));
> +      baseline_url = fetch_prop(props, session->repos_url.path,
> +                                SVN_DAV_PROP_NS_DAV, "baseline-relative-path");
> +
> +      if (!baseline_url)
> +        {
> +          abort();
> +        }
> +
> +      /* If we see baseline_url as "", we're the root.  Otherwise... */
> +      if (*baseline_url == '\0')
> +        {
> +          root_path = session->repos_url.path;
> +          session->repos_root = session->repos_url;
> +          session->repos_root_str = session->repos_url_str;
> +        }
> +      else
> +        {
> +          url_buf = svn_stringbuf_create(session->repos_url.path, pool);
> +          svn_path_remove_components(url_buf,
> +                                     svn_path_component_count(baseline_url));
> +          root_path = apr_pstrdup(session->pool, url_buf->data);
> +
> +          /* Now that we have the root_path, recreate the root_url. */
> +          session->repos_root = session->repos_url;
> +          session->repos_root.path = (char*)root_path;
> +          session->repos_root_str = apr_uri_unparse(session->pool,
> +                                                    &session->repos_root, 0);

Does this *guarantee* that the string returned will be a prefix of the URL
used to open the session, as required by the API?  I couldn't read that
out from the APR documentation.. I just want to make sure this is the
case.

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r18258 - trunk/subversion/libsvn_ra_serf

["Peter N. Lundblad" <pe...@famlundblad.se>]

On Fri, 27 Jan 2006, Greg Stein wrote:

> On Fri, Jan 27, 2006 at 10:24:14PM +0100, Peter N. Lundblad wrote:
> > On Thu, 26 Jan 2006 jerenkrantz@tigris.org wrote:
> >...
> > > +  /* Do we need to check that it's the same host and port? */
> >
> > I'd say no, since the caller svn_ra_reparent will guarantee that the
> > repository root part is the same on the old and new URL.
>
> Agreed. Somebody might be changing the hostname in order to use a
> different virthost on the server. Or maybe moving to a private port.
> The UUIDs are checked, so any kind of change in the URL should be just
> fine.
>
I think you misunderstood me.  Currently, svn_ra_reparent will enforce the
rule that the repo root part of the URL must not change.  That's mostly
for simplicity (avoiding having to reconnect and that).  The point of
reparent is to change the path inside the repository.

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r18258 - trunk/subversion/libsvn_ra_serf

[Greg Stein <gs...@lyra.org>]

On Fri, Jan 27, 2006 at 10:24:14PM +0100, Peter N. Lundblad wrote:
> On Thu, 26 Jan 2006 jerenkrantz@tigris.org wrote:
>...
> > +  /* Do we need to check that it's the same host and port? */
> 
> I'd say no, since the caller svn_ra_reparent will guarantee that the
> repository root part is the same on the old and new URL.

Agreed. Somebody might be changing the hostname in order to use a
different virthost on the server. Or maybe moving to a private port.
The UUIDs are checked, so any kind of change in the URL should be just
fine.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org