Re: svn_delta_path_driver(), its purpose and future

On Wed, Nov 30, 2011 at 8:12 AM, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> On 11/30/2011 12:59 AM, Hyrum K Wright wrote:
>> Let me offer a concrete example, in the hopes that I can make some
>> sense.  I use the term "sender" to mean "the thing that is invoking
>> the editor callbacks" and "receiver" to mean "the thing who is
>> providing callbacks to be invoked".  I believe these are the
>> traditional uses of the terms; if not, I've got some more reeducation
>> to go through.
>
> (I've always used "editor driver" and "editor implementation", but whatever.)

Yep, and after I sent that mail I realized that after writing that
paragraph, I didn't use any of those terms the rest of the mail.
Whatever indeed. :)

>> In libsvn_client/mergeinfo.c there is a function named
>> svn_client__elide_mergeinfo_catalog() (or SCEMC for short).  SCEMC
>> uses SDPD to elider mergeinfo and uses a default Ev1 editor with only
>> custom open_root() and open_directory() handlers[1].  It provides this
>> editor to SDPD.
>>
>> Without the shims, everything works well, but when the delayed-run
>> action of the shims is inserted, we get crashes.  The reason for this
>> is that with the shims, the SDPD callback is invoked prior to any
>> open_directory() handlers of the editor are invoked, and the SDPD
>> callback depends upon state which the open_directory() handler sets.
>>
>> Basically, the delayed processing of the Ev1 open_directory() handler
>> is combined with the *immediate* handling of the SDPD callback, which
>> results in out-of-order execution of the SDPD callback, and from there
>> badness ensues.  My thought is to move the shim insertion inside SDPD
>> and make it clever enough to delay calling the SDPD callback until the
>> Ev1 queue is played back, thus ensuring the ordering is then correct.
>
> I guess this is the part where I'm confused.  You can't "delay" an
> open_directory() call -- it must execute, it must return, and when it
> returns it must provide a new directory baton or throw an error.

Actually, such calls can be delayed. Quoth the docs:

 * At least one implementation of the editor interface is
 * asynchronous; an error from one operation may be detected some
 * number of operations later. As a result, an editor driver must not
 * assume that an error from an editing function resulted from the
 * particular operation being detected. Moreover, once an editing
 * function returns an error, the edit is dead; the only further
 * operation which may be called on the editor is abort_edit.

Now, that doesn't mean that calls to the SDPD callback shouldn't be
similarly delayed as well, just that calling an editor handler
function does not mean that the effects will happen *right now*.

> I'm starting to wonder if these shims fit into a different junction than I'd
> assumed.  In fact, in this case, there should be no shims in use at all --
> it's perfectly okay for SCEMC to use SDPD and an Ev1 editor now and forever.
>  We should only be using shims where they are *required* to fix up interface
> mismatches.  No such mismatch exists at this call site which, as you say,
> owns both the sender and the receiver.

True, and I've got a very simple patch which removes the use of the
shims from the above example site. I'll need to look elsewhere to see
if other places need a similar adjustment.

Just brain storming here, but since the users of SDPD are already
operating in a way which is similar to Ev2, I'm wondering if they
would be primary candidates for converting to Ev2 initially. Of
course, that'd mean other work would need to be done to give them
something to interact with, but it may be a possible way forward.

> Ah...  I see what's going on now.  You're trying to perform a double
> transformation (delta->editor->delta) here.  Now I understand what you mean
> by delaying the open_directory().  It's the open_directory() of that
> tail-end Ev1 that's delayed.  So again, in this instance, the use of shims
> is completely unnecessary.  You must know that, so may I assume that you're
> just trying to do this stuff for the sake of working out problems with the
> shims themselves?

Yes! Instead of writing individual tests and guessing at all the
different scenarios the shims would encounter, the easiest solution
was to insert the matching pairs of shims in places where editors are
used or created, and then just fix the bugs. While that may seem like
something of a shotgun approach, it guarantees that the shims will
handle what we need them to in the transition period from Ev1 to Ev2.
As some point, we'll be able to decouple the Ev1->Ev2->Ev1 path, and
replace individual consumers or producers at our leisure. That's the
theory, at least.

-Hyrum

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/