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

Re: svn commit: r1329015 - in /subversion/trunk/subversion: include/private/svn_ra_private.h libsvn_ra/editor.c libsvn_ra/ra_loader.c libsvn_ra/ra_loader.h

From: Hyrum K Wright <hyrum.wright_at_wandisco.com>
Date: Mon, 23 Apr 2012 15:21:36 -0500

On Mon, Apr 23, 2012 at 12:52 PM, Greg Stein <gstein_at_gmail.com> wrote:
>
> On Apr 23, 2012 1:36 PM, "Hyrum K Wright" <hyrum.wright_at_wandisco.com> wrote:
>>
>> On Mon, Apr 23, 2012 at 12:18 PM, Greg Stein <gstein_at_gmail.com> wrote:
>> >
>> > On Apr 23, 2012 1:12 PM, "Hyrum K Wright" <hyrum.wright_at_wandisco.com>
>> > wrote:
>> >>
>> >> On Mon, Apr 23, 2012 at 12:08 PM, Greg Stein <gstein_at_gmail.com> wrote:
>> >> >
>> >> > On Apr 23, 2012 8:00 AM, "Hyrum K Wright" <hyrum.wright_at_wandisco.com>
>> >> > wrote:
>> >> >>...
>> >> >
>> >> >
>> >> >> I added an Ev2 capability to svn_ra.h on the ev2-export branch,
>> >> >> which
>> >> >> we could use to check the ability of the client and/or server to
>> >> >> receiver Ev2-encoded deltas.  Feel free to poach the one-line patch.
>> >> >
>> >> > I'm not sure it is needed. How would the client use it?
>> >>
>> >> To determine if the server is Ev2 capable?  We've got to have *some*
>> >> method of doing so, and the existing capabilities mechanism was built
>> >> just for that purpose.
>> >
>> > But is that an RA constant, or something under the covers? ie. does that
>> > need to be in svn_ra.h for client consumption, or can RA determine that
>> > privately and compensate?
>>
>> The string is globally defined, but each server (and client?)
>> advertises the capabilities it supports as part of the initial
>> connection set up.  These are then cached by the client (and server?)
>> for other libraries to consume.  In practice, these means that
>> libsvn_ra can fall back to alternative implementations of
>> functionality if talking to older servers.
>>
>> > For example, ra_dav can do everything but the symlink operations and
>> > rotate(). Those will need to query the server to determine whether to
>> > use
>> > new/old marshalling. But to the svn_ra user, it is always available.
>> >
>> > For ra_local, it is always available at both internal/client levels.
>>
>> Both correct, and the capabilities mechanism is the way for the ra
>> layers to determine if the other side supports the new marshalling
>> format.
>
> Alright. It sounds like svn_ra.h has two purposes: stuff for libsvn_client,
> and stuff for all RA layers to use internally. Ugh.

But the capability strings are part of the public API.
svn_ra_has_capability() expects them as input, and it makes sense to
define them in a public place. Consumers external to the ra libs can
realistically be expected to ask this question, so I don't see a
problem with them being in the public space.

> I'd rather see internal stuff moved to a private header, since it is not
> part of the public API...
>
> IOW, if/when that capability hits trunk, can we put it into svn_ra_private?
> (and do so, going forward)

Given the above, I think they should stay in svn_ra.h.

-Hyrum

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/
Received on 2012-04-23 22:22:11 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.