You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@lyra.org> on 2006/01/25 04:02:05 UTC
Re: svn commit: r18214 - in trunk: . build/ac-macros subversion/include subversion/libsvn_ra subversion/libsvn_ra_serf
On Tue, Jan 24, 2006 at 08:57:27PM -0600, jerenkrantz@tigris.org wrote:
>...
> +++ trunk/subversion/include/svn_ra.h Tue Jan 24 20:57:24 2006
> @@ -1545,6 +1545,12 @@
> apr_pool_t *pool,
> apr_hash_t *hash);
>
> +/** Initialize libsvn_ra_serf.
> + *
> + * @deprecated Provided for backward compatibility with the 1.1 API. */
> +svn_error_t * svn_ra_serf_init (int abi_version,
> + apr_pool_t *pool,
> + apr_hash_t *hash);
No need for this. There are no programs built against the 1.1 API that
will be calling this function.
>...
> +++ trunk/subversion/libsvn_ra_serf/serf.c Tue Jan 24 20:57:24 2006
>...
> +static svn_error_t *
> +svn_ra_serf__open (svn_ra_session_t *session,
> + const char *repos_URL,
> + const svn_ra_callbacks2_t *callbacks,
> + void *callback_baton,
> + apr_hash_t *config,
> + apr_pool_t *pool)
> +{
> + return NULL;
> +}
I would suggest that all of these stubs to an abort(). Thus, while
you're developing, if one is called unexpectedly, then you'll see it
right away.
>...
> +static const svn_ra__vtable_t serf_vtable = {
> + ra_serf_version,
> + ra_serf_get_description,
> + ra_serf_get_schemes,
> + svn_ra_serf__open,
You could also replace these with NULL, until they're written.
>...
> +/* Compatibility wrapper for pre-1.2 subversions. Needed? */
I'm not sure how this wrapper works, but yah: it might be needed. This
would allow a 1.0 or 1.1 client to use the 1.x SVN libraries to talk
to a repository via serf.
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
Re: svn commit: r18214 - in trunk: . build/ac-macros subversion/include subversion/libsvn_ra subversion/libsvn_ra_serf
Posted by Greg Stein <gs...@lyra.org>.
On Tue, Jan 24, 2006 at 08:31:32PM -0800, Justin Erenkrantz wrote:
>...
> I thought so too, but there's this nice todo in ra_loader.c:
>
> * ### todo: Any RA libraries implemented from this point forward
> * ### don't really need an svn_ra_NAME_init compatibility function.
> * ### Currently, load_ra_module() will error if no such function is
> * ### found, but it might be more friendly to simply set *COMPAT_FUNC
> * ### to null (assuming COMPAT_FUNC itself is non-null).
>
> So, I'm not sure setting compat_init field to NULL would actually
> work. There are some files in trunk that still use
> svn_ra_get_ra_library. If they were to request the 'http' or 'https'
> scheme when ra_serf was loaded instead of ra_dav, an error would be
> returned. If a program worked with svn 1.0 that expected 'http', it'd
> fail if serf were included unless the compat init was present...
Ah. Right. The "look up the entry point in the DSO" thing. Shoulda
remembered that, as I wrote that code :-P
I was looking at it from the angle of somebody directly calling it
from some client code.
Thx,
-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
Re: svn commit: r18214 - in trunk: . build/ac-macros subversion/include subversion/libsvn_ra subversion/libsvn_ra_serf
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On 1/24/06, Greg Stein <gs...@lyra.org> wrote:
> On Tue, Jan 24, 2006 at 08:57:27PM -0600, jerenkrantz@tigris.org wrote:
> >...
> > +++ trunk/subversion/include/svn_ra.h Tue Jan 24 20:57:24 2006
> > @@ -1545,6 +1545,12 @@
> > apr_pool_t *pool,
> > apr_hash_t *hash);
> >
> > +/** Initialize libsvn_ra_serf.
> > + *
> > + * @deprecated Provided for backward compatibility with the 1.1 API. */
> > +svn_error_t * svn_ra_serf_init (int abi_version,
> > + apr_pool_t *pool,
> > + apr_hash_t *hash);
>
> No need for this. There are no programs built against the 1.1 API that
> will be calling this function.
I thought so too, but there's this nice todo in ra_loader.c:
* ### todo: Any RA libraries implemented from this point forward
* ### don't really need an svn_ra_NAME_init compatibility function.
* ### Currently, load_ra_module() will error if no such function is
* ### found, but it might be more friendly to simply set *COMPAT_FUNC
* ### to null (assuming COMPAT_FUNC itself is non-null).
So, I'm not sure setting compat_init field to NULL would actually
work. There are some files in trunk that still use
svn_ra_get_ra_library. If they were to request the 'http' or 'https'
scheme when ra_serf was loaded instead of ra_dav, an error would be
returned. If a program worked with svn 1.0 that expected 'http', it'd
fail if serf were included unless the compat init was present...
> I would suggest that all of these stubs to an abort(). Thus, while
> you're developing, if one is called unexpectedly, then you'll see it
> right away.
Fair enough.
> You could also replace these with NULL, until they're written.
Yes, but I wanted to walk through the functions myself and get the
prototypes in there now so that I know what functions I need to
implement.
> > +/* Compatibility wrapper for pre-1.2 subversions. Needed? */
>
> I'm not sure how this wrapper works, but yah: it might be needed. This
> would allow a 1.0 or 1.1 client to use the 1.x SVN libraries to talk
> to a repository via serf.
I tried to not include these, but the implementation for the new init
function is also in that wrapper template. Modulo the reasons above
to keep the compat_init function in there, without the compatibility
wrapper, we'd have to 'roll' our own init function for the new API as
well.
Thanks. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org