[ note: this is a single reply to all three messages on this thread; I
felt this was easier for future replies/discussion than separate
emails ]
On Tue, Sep 1, 2009 at 16:58, Neels J Hofmeyr <neels_at_elego.de> wrote:
>...
> () The new callbacks use a REL_PATH argument consistently. But the editor
> interface really makes no restriction on the path whatsoever (yet). It seems
> that it serves no purpose to call this REL_PATH instead of just PATH. But:
>
> The usual case will be that producer and consumer agree on a base path, and
> the paths passed to the callbacks are indeed relative. In that case, it
> serves the consumer code readability to call it REL_PATH.
Right. I used the name RELPATH for readability purposes. Historically,
we used PATH to mean all kinds of things, and it has burned us. The
new WC code uses ABSPATH and RELPATH names to help the reader identify
what is expected within those variables.
> Furthermore, if we are to add a debug/validation layer that enforces the
hehe... "When", not "if" :-)
> 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.
> Sending this list "only" serves recovery in case the operation is
> interrupted, right? Is it really necessary to do this list thing at all?
> What's the use of 'incomplete' child nodes for recovery anyway?
The directory is "complete" at the end of that function call. If you
don't pass the names, then the directory is left open until the end of
the editor drive. There is no "done" operation to signal that the
directory is complete. All of the editor functions complete their
change in that single call, except for the three unique, documented
pairs of calls.
On Mon, Sep 14, 2009 at 10:56, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> On Tue, 2009-09-01 at 22:58 +0200, Neels Janosch Hofmeyr wrote:
>...
>> Furthermore, if we are to add a debug/validation layer that enforces the
>> 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"?)
>
> Thinking about how the editor is supposed to communicate changes to "a
> tree" (examples: a versioned tree in the same repo, or an unversioned
> tree on disk or represented in a patch file), 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.
> 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.
Typically, the root will be the working copy root, or the repository
root. There really aren't any other interesting roots.
Note that an update/checkout/switch will never invoke the copy/move
functions. 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.
On Mon, Sep 14, 2009 at 18:47, Neels J Hofmeyr <neels_at_elego.de> wrote:
> Julian Foad wrote:
>...
> There are a number of ordering restrictions, but it's more on the order in
> which an individual path must be manipulated. We should forbid the second
> case above. Now is the time to formulate the proper API definition.
I think it should be fine to just let it be. I don't see that anybody
will drive the editor that way, nor do I see a problem resulting from
such a drive.
> Imposing "parents first" seems too restrictive to me. For example, it's
> perfectly ok to edit a file and change its parent dir's props later.
Totally agreed.
I don't see any of our potential receivers having a problem with this
kind of ordering.
> I feel like writing "The order of API calls must make sense" :P
>
> I'm saying that we could even allow:
> edit Z/B/file1 # edit file in first position
> move Z/B to Z/C
>
> (But with our current code base, this case probably won't happen anyway, as
> far as I know at least.)
Agreed. I believe most drivers will issue the move() call before any
changes are applied to file1.
> How's this, does it contain enough restriction? :
>
> "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.
> 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.
> But it forbids sequences like this:
> copy A to B
> edit B
> copy B to C
> atomically replace B with A
> It's an ambitious sequence, I know, and it is equivalent to
> copy A to C
> edit C
> copy A to B
> which I'm wildly guessing is what our current code will always automagically
> collapse to anyway.
>
> Still, with an eye ahead towards a possible dawning of operational merges in
> a foggy, hypothetical future, it might pay off to just leave it open for
> interpretation by driver/receiver implementations.
Operational merges, or anything else. Until we can show a *problem*
caused by funny ordering, then let's just leave it open.
> BTW, thanks for picking this thread up again, Julian :)
Agreed! I had starred this message, but it fell off my recent stack :-P
Cheers,
-g
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2394883
Received on 2009-09-15 07:57:04 CEST