On Mon, Apr 6, 2009 at 08:47, Philipp Marek <philipp.marek_at_emerion.com> wrote:
>...
>> > @@ -0,0 +1,100 @@
>> > +NOTES on a revamped editor interface
>> > +
>> > +-- all paths are relpaths
>> > + -- could be: relative to wcroot, or repos, or <what else?>
> I'd hope that they're relative to the object the "parent" baton belongs to -
> ie. for ->delete_entry the directory of the given dir_baton.
There are no parent/directory batons in this design. All the paths are
relative to the root of the tree: the wcroot with canonical, relative
filesystem paths, or the repository root with relative repository
paths. I don't believe that we have any other roots in our
editor-related systems.
>> > +-- expectations around *what* is being changed
> ...
>> > + 5. move REV: source will be like a delete, so could have been
>> > edited/deleted +
> I'd expect that a MOVE will have to give a "base" revision, and if that's no
> longer current there'd be an "out of date".
Correct. Both delete() and move() will say "doing OP to PATH in REV."
If there are changes to PATH between REV and HEAD, then you'll get an
out-of-date error.
>> > +API will be like svn_stream_t, RA, and FS
> ...
>> > + -- no return values, so scratch_pool everywhere
> What does that mean? I'm a bit worried about APR's behaviour to close the file
> on pool cleanup, because it's not that easy anymore to check for errors.
> Or do you think about something completely different here, ie. return "values"
> as in "results" (like a path, struct, ...)?
Every single editor callback gets a scratch_pool passed to it. You can
open a file within that pool and either close it manually or wait for
pool cleanup. Generally, the pool will be cleared "soon".
But if you look at all the callback signatures, there are no OUT
parameters. So we don't need a pool for allocating that data.
>> > +add_directory(path, children, props)
>> > + -- name all children. receiver can put on "pending" list
>> > + -- MUST be followed (at some point) by add_*() calls for each child
> Why should children be given, if they're to be done explicitly later anyway?
The children should be given so that the directory knows its children
at the given revision. Once it has that list and its properties, then
the directory is fully-specified. An interruption after this point
will not require re-sending the directory information. When the
children are passed to this function, the receiver can make a note of
all the children, and note they are incomplete. As each child arrives,
then it is completed and will not have to be resent if an interruption
arrives.
>> > +set_props(path, against_rev, props, complete)
>> > + -- 'complete' indicates whether this completes the change to this node
>> > + -- MUST be TRUE for directories
>> > + -- if TRUE, then a set_text() MUST follow if node is a file
>> > + -- if TRUE, then a set_target() MUST follow if node is a symlink
> Maybe a flag to say "append/replace these" vs. "replace all properties with
> that hash"?
The caller always has the full, desired hash of properties. The
receiver can "diff" them if it was set/replace/delete sub-operations.
The editors which send information over the wire will probably create
a diff to keep the wire data smaller.
>> > +set_text(path, against_rev, checksum, stream)
>> > + -- completes the change for this file (ie. if set_props was called)
>> > + -- stream can be ignored by recipient if it knows the checksum
>> > + -- include a mime type to help recipient manage stream processing?
> might checksum be a hash? Eg. "I know the MD5 and the SHA1 - take what you
> want"
This is a good point.
Hyrum: any thoughts on expanding the checksum type to contain multiple
algorithms?
>> > +set_target(path, against_rev, target)
>> > + -- completes the change for this symlink (ie. if set_props was called)
> For the three above: why a against_rev? Is the number given to open_root()
> et.al. not enough?
There is no open_root() call in this design. The against_rev parameter
says "I'm expecting to change the target of PATH in REV", and we can
signal out-of-date if there have been changes between REV and HEAD.
>> > +callback:
>> > +get_info(path, *kind, *revision, *checksum, *children, *target,
>> > + *props, *stream)
> Again: Post a wish regarding the checksum returned? Maybe the repository
> stores more than one.
Yah.
>> > +new interface: svn_editor.h
>> > + -- don't try to rev svn_delta_editor_t. creates a huge mess
>> > + -- svn_editor_* prefix
>> > + -- svn_editor_setcb_* to set the various callbacks
>> > + -- svn_editor_setcb_many(vtable) to copy funcs in
> Could use a some other parameter (size of table) or a hash instead, to be
> completely future-proof.
The vtable passed is a structure of a given type. If we
add/modify/remove callbacks, then we can just pass a different
structure type.
The big change from editor v1 is that the driver does not dereference
the vtable directory, but uses functions to perform the call (kind of
like callers using svn_stream_read() rather than the read-callback).
This gives us a lot of flexibility in future-proofing the code: we can
automatically convert across different function signatures for the
callback. Other debug features like control flow verification,
tracing, etc are possible with this model, along with some standard
features like cancellation support.
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1560148
Received on 2009-04-06 11:22:07 CEST