Hi again,
first of all, a different question to gstein -- tried to get hold of you on
IRC, but here it is via email instead:
in editor v2, how are text deltas communicated? There is a set_text() and a
set_props() ... Is it like this: if REVISION == SVN_INVALID_REVNUM then
CONTENTS is the pure text, otherwise CONTENTS is a delta relative to
REVISION? (same with set_props()?)
Which leads me to another consideration ...
copy A to B
edit B/file
How would this communicate across the editor? We want to edit the new
B/file, i.e. send a text delta relative to A/file_at_BASE. The set_text() only
gets a RELPATH of B/file, which is a path that technically has no revision
yet -- see my problem?
I hear that the current editor always sends a delta, against an empty file
if needed. But then the function should be called apply_text_delta()...
So, gstein, what were your intentions?
...
Now other replies:
Greg Stein wrote:
>> Furthermore, if we are to add a debug/validation layer that enforces the
>
> hehe... "When", not "if" :-)
heh
let's hold a lottery draw for the great privilege and fun of writing that
validator.
>> constraints outlined by the API description, we might want to have some
>> checks against the paths passed, e.g. whether all children of an added
>> directory are indeed added later. In that case, it would also be good to
>> enforce a very specific path format, as in "this/is/a/path". Agreed?
>> (What's that called, "Subversion's internal path format"?)
>
> We don't really have a name for pure, relative paths. Bert and I
> discussed "repos path" or "relative path", and having functions for
> those alongside the dirent and uri "path" functions. Neither of us
> have been gung ho about writing those funcs ... no immediate need.
>
> But yes: they are internal style, relative paths. (meaning "/" separators)
>
>> () In the same line, add_directory()'s CHILDREN arg (apr_array_header_t) is
>> not defined yet. The simplest would be to define it as a list of mere names
>> of immediate children (basenames) that are to be added subsequently.
>
> My intent was a set of basenames.
>
>> My intuition says this list should also pass the node kind or something.
>> What does the WC actually need to create an 'incomplete' node?
>
> Just the name.
So, say a merge tries to add a dir A/alpha, but A/alpha happens to be a file
in the target. When the callbacks send the ... meh, I keep forgetting that
it is *add*_directory(). Obviously, if A already exists, then it's already a
tree-conflict on A and A/alpha isn't of much interest.
Ok, cool. ;)
[...]
>> I would say yes, require
>> paths to be in the Subversion-internal format (separator always '/'),
>> not starting with '/', and relative to an externally-agreed root.
>
> Agreed.
Agreed likewise.
>
>> This raises a question in my head. In editor v1, each child node is
>> referred to simply by its basename, implicitly relative to "the current
>> parent" in the call nesting sequence. When you make a copy of a parent
>> directory, and then start issuing edit instructions within the copied
>> subtree, that all makes sense. If in editor v2 every node will be
>> referred to by its full path (relative to an agreed root), we need to be
>> very careful to define whether the path is the source path or the
>> destination path when the operation might be referring to a node inside
>> a copied/moved subtree. It might sound trivial but do you see what I
>> mean? It's easiest to think about when considering a "move".
>
> The path is *always* relative to the root. Thus, the root must
> encompass all copy/move sources and destinations.
The driver must be aware of whether it first moved the parent or not. I
think it's not exactly trivial, but inherent to the driver's chosen
algorithm/calling order, no general restrictions needed.
>
> Typically, the root will be the working copy root, or the repository
> root. There really aren't any other interesting roots.
Maybe any subdir of the working copy root, e.g. the outermost dir of a commit.
>
> Note that an update/checkout/switch will never invoke the copy/move
> functions.
whoa, wait a minute. I agree with checkout/switch, since there's no
underlying common history involved. But update?
If user A does an 'svn copy' and commits,
then user B does an 'svn update',
surely the copy() callback should be invoked during 'update'?
Remember that copy() is the new "add_with_history" callback with a different
name. But also, IMHO 'update' should invoke move() instead of { copy();
delete(); }.
> Nodes are always added. A pristine working copy has no
> knowledge of historical copies/moves, so that information does not
> pass through the editor interface. The copy/move semantics of the
> interface will really only occur during a commit, where all paths will
> be relative to the repository root (regardless of how you may have
> jumbled up your working copy; a wc does not have to match the repo
> layout).
>
>> move A/B to A/C
>> edit A/C/file1 # looks logical
>>
>> edit Z/C/file1
>> move Z/B to Z/C # is this sequence meaningful? allowed?
>>
>> Is there an ordering restriction, perhaps that operations on a parent
>> must come before operations on any children?
>
> This is allowed by the interface, as I originally conceived it. My
> hope was to remove as many ordering constraints as possible.
>
> Now, will any code drive it this way? Probably not, so the issue may
> be moot. If some code *does* drive it this way, then is it a problem?
> I think not.
Yes, the problem that might arise is receivers having to cache things
because they can't make sane assumptions on the order of incoming data.
That's the same reason why I wanted the 'replace' stuff.
So I want to impose the restriction as below, that if an edit comes in, the
receiver doesn't ever need to cache that edit and wait until the target of
the edit arises out of a subsequent editor call:
[...]
>> "The producer must issue the editor calls in a logical sequence, so that
>> calls do not manipulate a path before that path is created (using add, copy
>> or move) nor after such a path was obsoleted (e.g. using delete or move)."
>
> Yup. I think my notes covered this. Basically, the node must exist at
> the time of the change.
so we've already agreed on it :)
>
>> We could also add:
>>
>> "The editor calls must not manipulate a path _before_ it is replaced or
>> deleted." (Why produce edits when the path is going to be removed anyway)
>
> This kind of change might not be optimal, but I see no reason to
> actively prevent it.
Neither do I, so I kept it separate. Julian, do you agree with this?
I'd like to follow this guideline:
> Until we can show a *problem*
> caused by funny ordering, then let's just leave it open.
BTW, what is the problem arising when we take away the restriction "calls
(except for the three exceptions) must complete their node before returning"?
And I'm still waiting on a 1:1 review of the API in svn_editor.h thus far.
Maybe I'll go hide a few typos to see if anyone catches them ;)
heh, damn, version control would give them away
~Neels
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2395155
Received on 2009-09-15 18:41:26 CEST