On Mon, Apr 23, 2012 at 12:52 PM, Greg Stein <gst...@gmail.com> wrote: > > On Apr 23, 2012 1:36 PM, "Hyrum K Wright" <hyrum.wri...@wandisco.com> wrote: >> >> On Mon, Apr 23, 2012 at 12:18 PM, Greg Stein <gst...@gmail.com> wrote: >> > >> > On Apr 23, 2012 1:12 PM, "Hyrum K Wright" <hyrum.wri...@wandisco.com> >> > wrote: >> >> >> >> On Mon, Apr 23, 2012 at 12:08 PM, Greg Stein <gst...@gmail.com> wrote: >> >> > >> >> > On Apr 23, 2012 8:00 AM, "Hyrum K Wright" <hyrum.wri...@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/
