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