On Tue, Feb 21, 2012 at 5:54 PM, Greg Stein <gstein_at_gmail.com> wrote:
> On Tue, Feb 21, 2012 at 16:44, Â <hwright_at_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/
Received on 2012-02-22 21:45:37 CET