On Tue, Feb 21, 2012 at 5:54 PM, Greg Stein <gst...@gmail.com> wrote: > On Tue, Feb 21, 2012 at 16:44, <hwri...@apache.org> wrote: >> Author: hwright >> Date: Tue Feb 21 21:44:13 2012 >> New Revision: 1292047 >> >> URL: http://svn.apache.org/viewvc?rev=1292047&view=rev >> Log: >> Ev2 shims: Register the appropriate callbacks for use with the Ev2 shims >> every >> time we fetch a commit editor from within the client. > > I think you've leaked the "hidden shims" out too far with this change. > Can't you put it back where you just registered the shims for every RA > session. It certainly doesn't hurt to put them into the session, and > let them be unused. > > On the baton, you're passing NULL for all of these, so it would seem > that you could do the same when creating the RA session and installing > shims. If you *do* need the baton sometimes, then pass the baton into > the client-internal RA session creation. > > ... something. It just seems that we shouldn't be seeing this > registration everywhere.
I agree in principle that keeping the shims and their corresponding "magic" as hidden as possible is a good thing. But in this case, I can't quite work out how to do what you are suggesting, for the following reason. When fetching the base/props/kind of a node, we need the local_abspath of that node. Since the only thing the base/props/kind fetching functions know about is the path relative to the edit root, the baton for those functions needs a prefix abspath to create the complete abspath for the node in question. However, we don't know what edit root abspath will be when creating the ra session. (Indeed, in the case of "pure" ra drives, without a corresponding working copy, we don't have one, but in that case we just use the empty stream and props as our base and everything works correctly.) To complicate matters further, even if a root abspath is know when the ra session is opened--say because the ra session is opened to a certain location--it could potentially change by reparenting the session or through some other means. While I don't know if such problems are practical or simply academic, I think they are import to consider. In short: if there is way to improve the abstraction, I'm all for it, but I'm just not seeing a way around the need for the root abspath in the callback baton. -Hyrum -- uberSVN: Apache Subversion Made Easy http://www.uberSVN.com/
